Lysenko/ocd 3171 om installation#33
Conversation
…t/spot-oceancd-cli into lysenko/OCD-3171-om-installation
konstantin-lysenko-netapp
force-pushed
the
lysenko/OCD-3171-om-installation
branch
from
604f086 to
49497ff
Compare
| return rolloutDefinition, nil | ||
| } | ||
|
|
||
| func InstallOperator(_ context.Context, payload operator.InstallationPayload) (*operator.InstallationOutput, error) { |
There was a problem hiding this comment.
change to getOMInstallationManifests
removed operator upgrade validation
…t/spot-oceancd-cli into lysenko/OCD-3171-om-installation
| type Config struct { | ||
| Filename string | ||
| Filename string | ||
| SingleOnly bool |
| applyHandler := cluster.BaseApplyHandler{} | ||
| for _, resource := range resources { | ||
| if err := applyHandler.Apply(resource); err != nil { | ||
| return fmt.Errorf("failed to apply operator manager ConfigMap: %w", err) |
There was a problem hiding this comment.
the error message has typo, you can apply other resources and not only ConfigMap
| "spot-oceancd-operator-commons/component_configs" | ||
| ) | ||
|
|
||
| type InstallationConfig struct { |
There was a problem hiding this comment.
change to OperatorManagerConfig
There was a problem hiding this comment.
we already have OperatorManagerConfigthat stores argoRollouts and operator's configs that then converted into ConfigMap
the InstallationConfig also contains info about the operator manager, the config is used only for the installation of OM
I suggest leave it as it is
| ArgoRolloutsConfig component_configs.ArgoRolloutsConfig `json:"argo"` | ||
| } | ||
|
|
||
| func (c *InstallationConfig) GetData() map[string]interface{} { |
There was a problem hiding this comment.
No one is using this method, please remove
| return data | ||
| } | ||
|
|
||
| func (c *InstallationConfig) GetOperatorManagerConfig() *component_configs.OperatorManagerConfig { |
There was a problem hiding this comment.
No one is using this method, please remove
There was a problem hiding this comment.
it is used by buildOperatorManagerConfigMap method
| ImagePullSecrets []corev1.LocalObjectReference `json:"imagePullSecrets"` | ||
| } | ||
|
|
||
| func NewInstallationConfig(data map[string]interface{}) (*InstallationConfig, error) { |
There was a problem hiding this comment.
change to NewOperatorManagerConfig
There was a problem hiding this comment.
we already have OperatorManagerConfigthat stores argoRollouts and operator's configs that then converted into ConfigMap
the InstallationConfig also contains info about the operator manager, the config is used only for the installation of OM
I suggest leave it as it is
| return &config, nil | ||
| } | ||
|
|
||
| func DefaultInstallationConfig() InstallationConfig { |
There was a problem hiding this comment.
Change to DefaultOperatorManagerConfig
There was a problem hiding this comment.
we already have OperatorManagerConfigthat stores argoRollouts and operator's configs that then converted into ConfigMap
the InstallationConfig also contains info about the operator manager, the config is used only for the installation of OM
I suggest leave it as it is
| Manager ManagerConfig `json:"manager"` | ||
| } | ||
|
|
||
| func NewInstallationPayload(config *InstallationConfig) OMManifestsRequest { |
There was a problem hiding this comment.
change to NewOMManfiestsRequest
| OM OM `json:"om"` | ||
| } | ||
|
|
||
| type OM struct { |
There was a problem hiding this comment.
change to OMManifestsItemsResponse
There was a problem hiding this comment.
but this structure will be inside the items field
the om field is redundant IMHO
we’re trying to put it on crutches
2 participants
This is the default pull request template. You can customize it by adding a
pull_request_template.mdat the root of your repo or inside the.githubfolder.Jira Ticket
Include a link to your Jira Ticket
Example: JIRAISS-1234
Demo
Please add a recording of the feature/bug fix in work. if you added new routes, the recording should show the request and response for each new/changed route
Checklist: