From 67a2930245b9c30194f943cd7fe26f10a6b81c8c Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Wed, 8 May 2024 15:41:37 -0400 Subject: [PATCH 01/10] ROX-24127: tenant resources via gitops --- .secrets.baseline | 6 +- e2e/e2e_canary_upgrade_test.go | 141 ++++++++++++++---- .../pkg/central/reconciler/reconciler.go | 8 + .../dinosaur/pkg/api/private/api/openapi.yaml | 2 + .../model_managed_central_all_of_spec.go | 1 + internal/dinosaur/pkg/gitops/config.go | 64 ++++++++ internal/dinosaur/pkg/gitops/service.go | 56 ++++++- internal/dinosaur/pkg/gitops/service_test.go | 140 +++++++++++++++++ .../dinosaur/pkg/presenters/managedcentral.go | 52 +++++-- .../pkg/presenters/managedcentral_test.go | 24 +-- openapi/fleet-manager-private.yaml | 4 +- 11 files changed, 434 insertions(+), 64 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 8e2f97f249..2154fd0e55 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -304,14 +304,14 @@ "filename": "internal/dinosaur/pkg/presenters/managedcentral.go", "hashed_secret": "f4ac636d63edfd5477df8f25e4f4794c73e91d51", "is_verified": false, - "line_number": 207 + "line_number": 208 }, { "type": "Secret Keyword", "filename": "internal/dinosaur/pkg/presenters/managedcentral.go", "hashed_secret": "e26735ec1cbf8ad15cb7d1eea4893035f61297aa", "is_verified": false, - "line_number": 213 + "line_number": 214 } ], "internal/dinosaur/pkg/services/dinosaurservice_moq.go": [ @@ -463,5 +463,5 @@ } ] }, - "generated_at": "2024-04-12T16:55:50Z" + "generated_at": "2024-05-08T19:37:20Z" } diff --git a/e2e/e2e_canary_upgrade_test.go b/e2e/e2e_canary_upgrade_test.go index 7868a1e52c..73c50a2f2e 100644 --- a/e2e/e2e_canary_upgrade_test.go +++ b/e2e/e2e_canary_upgrade_test.go @@ -153,19 +153,15 @@ var _ = Describe("Fleetshard-sync Targeted Upgrade", Ordered, func() { var centralNamespace string It("run only one operator with version: "+operatorVersion1, func() { - config := gitops.Config{ - RHACSOperators: operator.OperatorConfigs{ - CRDURLs: defaultCRDUrls, - Configs: []operator.OperatorConfig{operatorConfig1}, - }, - Centrals: gitops.CentralsConfig{ - Overrides: []gitops.CentralOverride{ - overrideAllCentralsToBeReconciledByOperator(operatorConfig1), - overrideAllCentralsToUseMinimalResources(), - }, - }, - } - Expect(putGitopsConfig(ctx, config)).To(Succeed()) + Expect(updateGitopsConfig(ctx, func(config gitops.Config) gitops.Config { + config = defaultGitopsConfig() + config.RHACSOperators.Configs = []operator.OperatorConfig{operatorConfig1} + config.Centrals.Overrides = []gitops.CentralOverride{ + overrideAllCentralsToBeReconciledByOperator(operatorConfig1), + overrideAllCentralsToUseMinimalResources(), + } + return config + })).To(Succeed()) Eventually(assertDeployedOperators(ctx, operator1DeploymentName)). WithTimeout(waitTimeout). WithPolling(defaultPolling). @@ -194,25 +190,74 @@ var _ = Describe("Fleetshard-sync Targeted Upgrade", Ordered, func() { }) It("upgrade central", func() { - config := gitops.Config{ - RHACSOperators: operator.OperatorConfigs{ - CRDURLs: defaultCRDUrls, - Configs: []operator.OperatorConfig{operatorConfig1, operatorConfig2}, - }, - Centrals: gitops.CentralsConfig{ - Overrides: []gitops.CentralOverride{ - overrideAllCentralsToBeReconciledByOperator(operatorConfig2), - overrideAllCentralsToUseMinimalResources(), - }, - }, - } - Expect(putGitopsConfig(ctx, config)).To(Succeed()) + Expect(updateGitopsConfig(ctx, func(config gitops.Config) gitops.Config { + config = defaultGitopsConfig() + config.RHACSOperators.Configs = []operator.OperatorConfig{operatorConfig1, operatorConfig2} + config.Centrals.Overrides = []gitops.CentralOverride{ + overrideAllCentralsToBeReconciledByOperator(operatorConfig2), + overrideAllCentralsToUseMinimalResources(), + } + return config + })).To(Succeed()) Eventually(assertCentralLabelSelectorPresent(ctx, createdCentral, centralNamespace, operatorVersion2)). WithTimeout(waitTimeout). WithPolling(defaultPolling). Should(Succeed()) }) + It("changes tenant resources", func() { + egressProxy, err := getDeployment(ctx, centralNamespace, "egress-proxy") + Expect(err).ToNot(HaveOccurred()) + Expect(egressProxy.Spec.Template.Spec.Containers).To(HaveLen(1)) + Expect(egressProxy.Spec.Template.Spec.Containers[0].Resources.Requests.Cpu().String()).To(Equal("100m")) + Expect(egressProxy.Spec.Template.Spec.Containers[0].Resources.Requests.Memory().String()).To(Equal("275Mi")) + Expect(egressProxy.Spec.Template.Spec.Containers[0].Resources.Limits.Memory().String()).To(Equal("275Mi")) + Expect(updateGitopsConfig(ctx, func(config gitops.Config) gitops.Config { + config.TenantResources.Default = ` +labels: + app.kubernetes.io/managed-by: rhacs-fleetshard + app.kubernetes.io/instance: {{ .Name }} + rhacs.redhat.com/org-id: {{ .OrganizationID }} + rhacs.redhat.com/tenant: {{ .ID }} + rhacs.redhat.com/instance-type: {{ .InstanceType }} +annotations: + rhacs.redhat.com/org-name: {{ .OrganizationName }} +secureTenantNetwork: false +centralRdsCidrBlock: "10.1.0.0/16" +egressProxy: + image: "registry.redhat.io/openshift4/ose-egress-http-proxy:v4.14" + replicas: 2 + resources: + requests: + cpu: 100m + memory: 200Mi + limits: + memory: 200Mi +` + return config + })).To(Succeed()) + + Eventually(func() error { + egressProxy, err := getDeployment(ctx, centralNamespace, "egress-proxy") + if err != nil { + return err + } + if egressProxy.Spec.Template.Spec.Containers[0].Resources.Requests.Memory().String() != "200Mi" { + return fmt.Errorf("egress proxy memory request not updated") + } + if egressProxy.Spec.Template.Spec.Containers[0].Resources.Limits.Memory().String() != "200Mi" { + return fmt.Errorf("egress proxy memory limit not updated") + } + if egressProxy.Spec.Template.Spec.Containers[0].Resources.Requests.Cpu().String() != "100m" { + return fmt.Errorf("egress proxy cpu request not updated") + } + return nil + }). + WithTimeout(waitTimeout). + WithPolling(defaultPolling). + Should(Succeed()) + }) + It("delete central", func() { Expect(deleteCentralByID(ctx, client, createdCentral.Id)). To(Succeed()) @@ -289,6 +334,24 @@ func putGitopsConfig(ctx context.Context, config gitops.Config) error { return k8sClient.Create(ctx, configMap) } +func updateGitopsConfig(ctx context.Context, updateFn func(config gitops.Config) gitops.Config) error { + var configMap v1.ConfigMap + if err := k8sClient.Get(ctx, ctrlClient.ObjectKey{Namespace: namespace, Name: gitopsConfigmapName}, &configMap); err != nil { + return err + } + var config gitops.Config + if err := yaml.Unmarshal([]byte(configMap.Data[gitopsConfigmapDataKey]), &config); err != nil { + return err + } + updated := updateFn(config) + configYAML, err := yaml.Marshal(updated) + if err != nil { + return err + } + configMap.Data[gitopsConfigmapDataKey] = string(configYAML) + return k8sClient.Update(ctx, &configMap) +} + func operatorConfigForVersion(version string) operator.OperatorConfig { return operator.OperatorConfig{ "deploymentName": getDeploymentName(version), @@ -481,8 +544,34 @@ metadata: ` + key + `: "` + value + `"`) } +func defaultTenantResourceValues() string { + return ` +labels: + app.kubernetes.io/managed-by: rhacs-fleetshard + app.kubernetes.io/instance: {{ .Name }} + rhacs.redhat.com/org-id: {{ .OrganizationID }} + rhacs.redhat.com/tenant: {{ .ID }} + rhacs.redhat.com/instance-type: {{ .InstanceType }} +annotations: + rhacs.redhat.com/org-name: {{ .OrganizationName }} +secureTenantNetwork: false +centralRdsCidrBlock: "10.1.0.0/16" +egressProxy: + image: "registry.redhat.io/openshift4/ose-egress-http-proxy:v4.14" + replicas: 2 + resources: + requests: + cpu: 100m + memory: 275Mi + limits: + memory: 275Mi` +} + func defaultGitopsConfig() gitops.Config { return gitops.Config{ + TenantResources: gitops.TenantResourceConfig{ + Default: defaultTenantResourceValues(), + }, RHACSOperators: operator.OperatorConfigs{ CRDURLs: defaultCRDUrls, Configs: []operator.OperatorConfig{ diff --git a/fleetshard/pkg/central/reconciler/reconciler.go b/fleetshard/pkg/central/reconciler/reconciler.go index 6cc27d19ac..36cc47a138 100644 --- a/fleetshard/pkg/central/reconciler/reconciler.go +++ b/fleetshard/pkg/central/reconciler/reconciler.go @@ -1732,6 +1732,14 @@ func (r *CentralReconciler) chartValues(c private.ManagedCentral) (chartutil.Val return nil, errors.New("resources chart is not set") } src := r.resourcesChart.Values + + // We are introducing the passing of helm values from fleetManager (and gitops). If the managed central + // includes the tenant resource values, we will use them. Otherwise, defaults to the previous + // implementation. + if len(c.Spec.TenantResourcesValues) > 0 { + return chartutil.CoalesceTables(c.Spec.TenantResourcesValues, src), nil + } + dst := map[string]interface{}{ "labels": stringMapToMapInterface(getTenantLabels(c)), "annotations": stringMapToMapInterface(getTenantAnnotations(c)), diff --git a/internal/dinosaur/pkg/api/private/api/openapi.yaml b/internal/dinosaur/pkg/api/private/api/openapi.yaml index 84ef0c307c..87301baf16 100644 --- a/internal/dinosaur/pkg/api/private/api/openapi.yaml +++ b/internal/dinosaur/pkg/api/private/api/openapi.yaml @@ -537,6 +537,8 @@ components: - eval - standard type: string + tenantResourcesValues: + type: object centralCRYAML: type: string owners: diff --git a/internal/dinosaur/pkg/api/private/model_managed_central_all_of_spec.go b/internal/dinosaur/pkg/api/private/model_managed_central_all_of_spec.go index 004012f973..16e3357f6f 100644 --- a/internal/dinosaur/pkg/api/private/model_managed_central_all_of_spec.go +++ b/internal/dinosaur/pkg/api/private/model_managed_central_all_of_spec.go @@ -13,6 +13,7 @@ package private // ManagedCentralAllOfSpec struct for ManagedCentralAllOfSpec type ManagedCentralAllOfSpec struct { InstanceType string `json:"instanceType,omitempty"` + TenantResourcesValues map[string]interface{} `json:"tenantResourcesValues,omitempty"` CentralCRYAML string `json:"centralCRYAML,omitempty"` Owners []string `json:"owners,omitempty"` Auth ManagedCentralAllOfSpecAuth `json:"auth,omitempty"` diff --git a/internal/dinosaur/pkg/gitops/config.go b/internal/dinosaur/pkg/gitops/config.go index cb788f319f..b598298c6c 100644 --- a/internal/dinosaur/pkg/gitops/config.go +++ b/internal/dinosaur/pkg/gitops/config.go @@ -10,6 +10,7 @@ import ( // Config represents the gitops configuration type Config struct { + TenantResources TenantResourceConfig `json:"tenantResources"` Centrals CentralsConfig `json:"centrals"` RHACSOperators operator.OperatorConfigs `json:"rhacsOperators"` DataPlaneClusters []DataPlaneClusterConfig `json:"dataPlaneClusters"` @@ -92,10 +93,23 @@ type AddonConfig struct { Parameters map[string]string `json:"parameters"` } +// TenantResourceConfig represents the declarative configuration for tenant resource values defaults and overrides. +type TenantResourceConfig struct { + Default string `json:"default"` + Overrides []TenantResourceOverride `json:"overrides"` +} + +// TenantResourceOverride represents the configuration for a tenant resource override. The override +type TenantResourceOverride struct { + InstanceIDs []string `json:"instanceIds"` + Values string `json:"values"` +} + // ValidateConfig validates the GitOps configuration. func ValidateConfig(config Config) field.ErrorList { var errs field.ErrorList errs = append(errs, validateCentralsConfig(field.NewPath("centrals"), config.Centrals)...) + errs = append(errs, validateTenantResourcesConfig(field.NewPath("tenantResources"), config.TenantResources)...) errs = append(errs, operator.Validate(field.NewPath("rhacsOperators"), config.RHACSOperators)...) errs = append(errs, validateDataPlaneClusterConfigs(field.NewPath("dataPlaneClusters"), config.DataPlaneClusters)...) return errs @@ -108,6 +122,38 @@ func validateCentralsConfig(path *field.Path, config CentralsConfig) field.Error return errs } +func validateTenantResourcesConfig(path *field.Path, config TenantResourceConfig) field.ErrorList { + var errs field.ErrorList + errs = append(errs, validateTenantResourcesDefault(path.Child("default"), config.Default)...) + errs = append(errs, validateTenantResourceOverrides(path.Child("overrides"), config.Overrides)...) + return errs +} + +func validateTenantResourcesDefault(path *field.Path, defaultValues string) field.ErrorList { + var errs field.ErrorList + if err := tryRenderDummyValuesWithPatch(defaultValues); err != nil { + errs = append(errs, field.Invalid(path, defaultValues, "invalid default values: "+err.Error())) + } + return errs +} + +func validateTenantResourceOverrides(path *field.Path, overrides []TenantResourceOverride) field.ErrorList { + var errs field.ErrorList + for i, override := range overrides { + errs = append(errs, validateTenantResourceOverride(path.Index(i), override)...) + } + return errs +} + +func validateTenantResourceOverride(path *field.Path, override TenantResourceOverride) field.ErrorList { + var errs field.ErrorList + errs = append(errs, validateInstanceIDs(path.Child("instanceIds"), override.InstanceIDs)...) + if err := tryRenderDummyValuesWithPatch(override.Values); err != nil { + errs = append(errs, field.Invalid(path.Child("values"), override.Values, "invalid values: "+err.Error())) + } + return errs +} + func validateAdditionalAuthProviders(path *field.Path, providers []AuthProviderAddition) field.ErrorList { var errs field.ErrorList for i, additionalProvider := range providers { @@ -281,6 +327,24 @@ func tryRenderDummyCentralWithPatch(patch string) error { return nil } +func tryRenderDummyValuesWithPatch(patch string) error { + var dummyParams = getDummyCentralParams() + dummyConfig := Config{ + TenantResources: TenantResourceConfig{ + Overrides: []TenantResourceOverride{ + { + Values: patch, + InstanceIDs: []string{"*"}, + }, + }, + }, + } + if _, err := RenderTenantResourceValues(dummyParams, dummyConfig); err != nil { + return err + } + return nil +} + func getDummyCentralParams() CentralParams { return CentralParams{ ID: "id", diff --git a/internal/dinosaur/pkg/gitops/service.go b/internal/dinosaur/pkg/gitops/service.go index e6b2fb7dbc..9a56e22383 100644 --- a/internal/dinosaur/pkg/gitops/service.go +++ b/internal/dinosaur/pkg/gitops/service.go @@ -2,6 +2,7 @@ package gitops import ( "encoding/json" + "helm.sh/helm/v3/pkg/chartutil" "strings" "text/template" @@ -20,6 +21,36 @@ func RenderCentral(params CentralParams, config Config) (v1alpha1.Central, error return applyConfigToCentral(config, central, params) } +// RenderTenantResourceValues renders the values for tenant resources helm chart for the given GitOps configuration and parameters. +func RenderTenantResourceValues(params CentralParams, config Config) (map[string]interface{}, error) { + values := map[string]interface{}{} + if len(config.TenantResources.Default) > 0 { + renderedDefault, err := renderPatchTemplate(config.TenantResources.Default, params) + if err != nil { + return nil, err + } + if err := yaml.Unmarshal([]byte(renderedDefault), &values); err != nil { + return nil, errors.Wrap(err, "failed to unmarshal default tenant resource values") + } + } + + for _, override := range config.TenantResources.Overrides { + if !shouldApplyOverride(override.InstanceIDs, params) { + continue + } + rendered, err := renderPatchTemplate(override.Values, params) + if err != nil { + return nil, err + } + patchValues := map[string]interface{}{} + if err := yaml.Unmarshal([]byte(rendered), &patchValues); err != nil { + return nil, errors.Wrap(err, "failed to unmarshal override patch") + } + values = chartutil.CoalesceTables(patchValues, values) + } + return values, nil +} + func renderDefaultCentral(params CentralParams) (v1alpha1.Central, error) { wr := new(strings.Builder) if err := defaultTemplate.Execute(wr, params); err != nil { @@ -72,7 +103,7 @@ type CentralParams struct { func applyConfigToCentral(config Config, central v1alpha1.Central, ctx CentralParams) (v1alpha1.Central, error) { var overrides []CentralOverride for _, override := range config.Centrals.Overrides { - if !shouldApplyCentralOverride(override, ctx) { + if !shouldApplyOverride(override.InstanceIDs, ctx) { continue } overrides = append(overrides, override) @@ -106,9 +137,9 @@ func applyConfigToCentral(config Config, central v1alpha1.Central, ctx CentralPa return result, nil } -// shouldApplyCentralOverride returns true if the given Central override should be applied to the given Central instance. -func shouldApplyCentralOverride(override CentralOverride, ctx CentralParams) bool { - for _, d := range override.InstanceIDs { +// shouldApplyOverride returns true if the given Central override should be applied to the given Central instance. +func shouldApplyOverride(instanceIDs []string, ctx CentralParams) bool { + for _, d := range instanceIDs { if d == "*" { return true } @@ -122,12 +153,12 @@ func shouldApplyCentralOverride(override CentralOverride, ctx CentralParams) boo // applyPatchToCentral will apply the given patch to the given Central instance. func applyPatchToCentral(centralBytes, patch []byte) ([]byte, error) { // convert patch from yaml to json - jsonPath, err := yaml.YAMLToJSON(patch) + patchJson, err := yaml.YAMLToJSON(patch) if err != nil { return []byte{}, errors.Wrap(err, "failed to convert override patch from yaml to json") } // apply patch - patchedBytes, err := strategicpatch.StrategicMergePatch(centralBytes, jsonPath, v1alpha1.Central{}) + patchedBytes, err := strategicpatch.StrategicMergePatch(centralBytes, patchJson, v1alpha1.Central{}) if err != nil { return []byte{}, errors.Wrap(err, "failed to apply override to Central instance") } @@ -148,3 +179,16 @@ func renderPatchTemplate(patchTemplate string, ctx CentralParams) (string, error // defaultTemplate is the default template for Central instances. var defaultTemplate = template.Must(template.New("default").Parse(string(defaultCentralTemplate))) + +func copyMap(m map[string]interface{}) map[string]interface{} { + result := make(map[string]interface{}) + for k, v := range m { + switch v := v.(type) { + case map[string]interface{}: + result[k] = copyMap(v) + default: + result[k] = v + } + } + return result +} diff --git a/internal/dinosaur/pkg/gitops/service_test.go b/internal/dinosaur/pkg/gitops/service_test.go index f45fb3b5c5..89293704aa 100644 --- a/internal/dinosaur/pkg/gitops/service_test.go +++ b/internal/dinosaur/pkg/gitops/service_test.go @@ -123,6 +123,146 @@ func TestRenderCentral(t *testing.T) { } } +func TestRenderTenantResourceValues(t *testing.T) { + tests := []struct { + name string + config Config + params CentralParams + assert func(t *testing.T, got map[string]interface{}, err error) + }{ + { + name: "no overrides", + params: CentralParams{ + ID: "central-1", + }, + config: Config{}, + assert: func(t *testing.T, got map[string]interface{}, err error) { + assert.NoError(t, err) + assert.Empty(t, got) + }, + }, { + name: "default without overrides", + params: CentralParams{ + ID: "central-1", + }, + config: Config{ + TenantResources: TenantResourceConfig{ + Default: `{"foo": "bar"}`, + }, + }, + assert: func(t *testing.T, got map[string]interface{}, err error) { + assert.NoError(t, err) + assert.Equal(t, map[string]interface{}{"foo": "bar"}, got) + }, + }, { + name: "default with override", + params: CentralParams{ + ID: "central-1", + }, + config: Config{ + TenantResources: TenantResourceConfig{ + Default: `{"foo": "bar"}`, + Overrides: []TenantResourceOverride{ + { + InstanceIDs: []string{"central-1"}, + Values: `{"foo": "baz"}`, + }, + }, + }, + }, + assert: func(t *testing.T, got map[string]interface{}, err error) { + require.NoError(t, err) + assert.Equal(t, map[string]interface{}{"foo": "baz"}, got) + }, + }, + { + name: "default with multiple overrides", + params: CentralParams{ + ID: "central-1", + }, + config: Config{ + TenantResources: TenantResourceConfig{ + Default: `{"foo": "bar"}`, + Overrides: []TenantResourceOverride{ + { + InstanceIDs: []string{"central-1"}, + Values: `{"foo": "baz"}`, + }, + { + InstanceIDs: []string{"central-1"}, + Values: `{"foo": "qux"}`, + }, + }, + }, + }, + assert: func(t *testing.T, got map[string]interface{}, err error) { + require.NoError(t, err) + assert.Equal(t, map[string]interface{}{"foo": "qux"}, got) + }, + }, { + name: "complex valued patch", + params: CentralParams{ + ID: "central-1", + }, + config: Config{ + TenantResources: TenantResourceConfig{ + Default: `{"buzz":"snap", "foo": "snafu"}`, + Overrides: []TenantResourceOverride{ + { + InstanceIDs: []string{"central-1"}, + Values: `{"foo":{ "bar": "qux" }}`, + }, + }, + }, + }, + assert: func(t *testing.T, got map[string]interface{}, err error) { + require.NoError(t, err) + assert.Equal(t, map[string]interface{}{"buzz": "snap", "foo": map[string]interface{}{"bar": "qux"}}, got) + }, + }, { + name: "default with templating", + params: CentralParams{ + ID: "central-1", + }, + config: Config{ + TenantResources: TenantResourceConfig{ + Default: `{"foo": "{{ .ID }}"}`, + }, + }, + assert: func(t *testing.T, got map[string]interface{}, err error) { + require.NoError(t, err) + assert.Equal(t, map[string]interface{}{"foo": "central-1"}, got) + }, + }, { + name: "overrides with templating", + params: CentralParams{ + ID: "central-1", + }, + config: Config{ + TenantResources: TenantResourceConfig{ + Default: `{"foo": "bar"}`, + Overrides: []TenantResourceOverride{ + { + InstanceIDs: []string{"central-1"}, + Values: `{"foo": "{{ .ID }}"}`, + }, + }, + }, + }, + assert: func(t *testing.T, got map[string]interface{}, err error) { + require.NoError(t, err) + assert.Equal(t, map[string]interface{}{"foo": "central-1"}, got) + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := RenderTenantResourceValues(tt.params, tt.config) + tt.assert(t, got, err) + }) + } +} + // TestDefaultTemplateIsValid tests that the default template is valid and // can be unmarshaled to a functional v1alpha1.Central object. func Test_defaultTemplate_isValid(t *testing.T) { diff --git a/internal/dinosaur/pkg/presenters/managedcentral.go b/internal/dinosaur/pkg/presenters/managedcentral.go index ee3be562cb..916786e3bf 100644 --- a/internal/dinosaur/pkg/presenters/managedcentral.go +++ b/internal/dinosaur/pkg/presenters/managedcentral.go @@ -102,7 +102,7 @@ func (c *ManagedCentralPresenter) PresentManagedCentralWithSecrets(from *dbapi.C func (c *ManagedCentralPresenter) presentManagedCentral(gitopsConfig gitops.Config, from *dbapi.CentralRequest) (private.ManagedCentral, error) { centralParams := centralParamsFromRequest(from) - centralYaml, err := c.renderer.getCentralYaml(gitopsConfig, centralParams) + renderedCentral, err := c.renderer.render(gitopsConfig, centralParams) if err != nil { return private.ManagedCentral{}, errors.Wrap(err, "failed to get Central YAML") } @@ -147,8 +147,9 @@ func (c *ManagedCentralPresenter) presentManagedCentral(gitopsConfig gitops.Conf DataEndpoint: private.ManagedCentralAllOfSpecDataEndpoint{ Host: from.GetDataHost(), }, - CentralCRYAML: string(centralYaml), - InstanceType: from.InstanceType, + CentralCRYAML: renderedCentral.CentralCRYaml, + TenantResourcesValues: renderedCentral.Values, + InstanceType: from.InstanceType, }, RequestStatus: from.Status, } @@ -267,23 +268,31 @@ func centralParamsFromRequest(centralRequest *dbapi.CentralRequest) gitops.Centr } } -type renderFn func(gitops.CentralParams, gitops.Config) (v1alpha1.Central, error) +type renderCentralFn func(gitops.CentralParams, gitops.Config) (v1alpha1.Central, error) +type renderValuesFn func(gitops.CentralParams, gitops.Config) (map[string]interface{}, error) type cachedCentralRenderer struct { - renderFn renderFn - locks *keyedMutex - cache *centralYamlCache + renderCentralFn renderCentralFn + renderValuesFn renderValuesFn + locks *keyedMutex + cache *centralYamlCache } func newCachedCentralRenderer() *cachedCentralRenderer { return &cachedCentralRenderer{ - renderFn: gitops.RenderCentral, - locks: newKeyedMutex(), - cache: newCentralYamlCache(), + renderCentralFn: gitops.RenderCentral, + renderValuesFn: gitops.RenderTenantResourceValues, + locks: newKeyedMutex(), + cache: newCentralYamlCache(), } } -func (r *cachedCentralRenderer) getCentralYaml(gitopsConfig gitops.Config, centralParams gitops.CentralParams) ([]byte, error) { +type RenderedCentral struct { + CentralCRYaml string + Values map[string]interface{} +} + +func (r *cachedCentralRenderer) render(gitopsConfig gitops.Config, centralParams gitops.CentralParams) (RenderedCentral, error) { centralID := centralParams.ID // We obtain a lock for the central ID so that no other goroutine can render the central yaml for the same central @@ -296,10 +305,11 @@ func (r *cachedCentralRenderer) getCentralYaml(gitopsConfig gitops.Config, centr // In other words, the central YAML will always be the same for the same gitops config and central params. hashes, err := newHashes(gitopsConfig, centralParams) if err != nil { - return nil, errors.Wrap(err, "failed to get hash for Central") + return RenderedCentral{}, errors.Wrap(err, "failed to get hash for Central") } var centralYaml []byte + var values map[string]interface{} var shouldRender = true // Check if the central yaml is in the cache and if it is, check if the hash matches @@ -311,19 +321,24 @@ func (r *cachedCentralRenderer) getCentralYaml(gitopsConfig gitops.Config, centr if entry.hashes.equals(hashes) { // The hash matches, we can use the cached central yaml centralYaml = entry.centralYaml + values = entry.values shouldRender = false } } if shouldRender { // There was no matching cache entry, we need to render the central yaml. - centralCR, err := r.renderFn(centralParams, gitopsConfig) + centralCR, err := r.renderCentralFn(centralParams, gitopsConfig) if err != nil { - return nil, errors.Wrap(err, "failed to apply GitOps overrides to Central") + return RenderedCentral{}, errors.Wrap(err, "failed to apply GitOps overrides to Central") } centralYaml, err = yaml.Marshal(centralCR) if err != nil { - return nil, errors.Wrap(err, "failed to marshal Central CR") + return RenderedCentral{}, errors.Wrap(err, "failed to marshal Central CR") + } + values, err := r.renderValuesFn(centralParams, gitopsConfig) + if err != nil { + return RenderedCentral{}, errors.Wrap(err, "failed to render tenant resource values") } // Locking the whole cache to add the new entry r.cache.Lock() @@ -332,10 +347,14 @@ func (r *cachedCentralRenderer) getCentralYaml(gitopsConfig gitops.Config, centr id: centralID, hashes: hashes, centralYaml: centralYaml, + values: values, } } - return centralYaml, nil + return RenderedCentral{ + CentralCRYaml: string(centralYaml), + Values: values, + }, nil } type centralHashes struct { @@ -414,6 +433,7 @@ type cacheEntry struct { id string hashes centralHashes centralYaml []byte + values map[string]interface{} } type centralYamlCache struct { diff --git a/internal/dinosaur/pkg/presenters/managedcentral_test.go b/internal/dinosaur/pkg/presenters/managedcentral_test.go index 5db1f0f496..64ef883cc1 100644 --- a/internal/dinosaur/pkg/presenters/managedcentral_test.go +++ b/internal/dinosaur/pkg/presenters/managedcentral_test.go @@ -13,7 +13,7 @@ func TestShouldNotRenderTwiceForSameParams(t *testing.T) { var params = gitops.CentralParams{} var renderCount = 0 r := newCachedCentralRenderer() - r.renderFn = func(params gitops.CentralParams, config gitops.Config) (v1alpha1.Central, error) { + r.renderCentralFn = func(params gitops.CentralParams, config gitops.Config) (v1alpha1.Central, error) { renderCount++ return v1alpha1.Central{}, nil } @@ -21,29 +21,29 @@ func TestShouldNotRenderTwiceForSameParams(t *testing.T) { assert.Equal(t, 0, renderCount) // first call should render once - r.getCentralYaml(gitopsConfig, params) + r.render(gitopsConfig, params) assert.Equal(t, 1, renderCount) // second call should not render again - r.getCentralYaml(gitopsConfig, params) + r.render(gitopsConfig, params) assert.Equal(t, 1, renderCount) // third call with different params should render again params = gitops.CentralParams{ID: "foo"} - r.getCentralYaml(gitopsConfig, params) + r.render(gitopsConfig, params) assert.Equal(t, 2, renderCount) // fourth call with same params should not render again - r.getCentralYaml(gitopsConfig, params) + r.render(gitopsConfig, params) assert.Equal(t, 2, renderCount) // fifth call with different params should render again gitopsConfig = gitops.Config{Centrals: gitops.CentralsConfig{Overrides: []gitops.CentralOverride{{InstanceIDs: []string{"foo"}}}}} - r.getCentralYaml(gitopsConfig, params) + r.render(gitopsConfig, params) assert.Equal(t, 3, renderCount) // sixth call with same params should not render again - r.getCentralYaml(gitopsConfig, params) + r.render(gitopsConfig, params) assert.Equal(t, 3, renderCount) } @@ -54,7 +54,7 @@ func TestShouldNotCacheOnError(t *testing.T) { var shouldThrow = false r := newCachedCentralRenderer() - r.renderFn = func(params gitops.CentralParams, config gitops.Config) (v1alpha1.Central, error) { + r.renderCentralFn = func(params gitops.CentralParams, config gitops.Config) (v1alpha1.Central, error) { renderCount++ if shouldThrow { return v1alpha1.Central{}, assert.AnError @@ -65,17 +65,17 @@ func TestShouldNotCacheOnError(t *testing.T) { assert.Equal(t, 0, renderCount) shouldThrow = true - r.getCentralYaml(gitopsConfig, params) + r.render(gitopsConfig, params) assert.Equal(t, 1, renderCount) - r.getCentralYaml(gitopsConfig, params) + r.render(gitopsConfig, params) assert.Equal(t, 2, renderCount) shouldThrow = false - r.getCentralYaml(gitopsConfig, params) + r.render(gitopsConfig, params) assert.Equal(t, 3, renderCount) - r.getCentralYaml(gitopsConfig, params) + r.render(gitopsConfig, params) assert.Equal(t, 3, renderCount) } diff --git a/openapi/fleet-manager-private.yaml b/openapi/fleet-manager-private.yaml index caa54853cd..38f686b2c8 100644 --- a/openapi/fleet-manager-private.yaml +++ b/openapi/fleet-manager-private.yaml @@ -279,6 +279,8 @@ components: instanceType: type: string enum: [ eval, standard ] + tenantResourcesValues: + type: object centralCRYAML: type: string owners: @@ -392,7 +394,7 @@ components: allOf: - $ref: "#/components/schemas/ManagedCentral" rhacs_operators: - $ref: RHACSOperatorConfigs + $ref: "#/components/schemas/RHACSOperatorConfigs" RHACSOperatorConfigs: properties: From c3e66e6c19114c4022fdeafa8547f65227a1e1eb Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Wed, 8 May 2024 19:55:05 -0400 Subject: [PATCH 02/10] ROX-24127: tenant resources via gitops --- e2e/e2e_canary_upgrade_test.go | 46 +++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/e2e/e2e_canary_upgrade_test.go b/e2e/e2e_canary_upgrade_test.go index 73c50a2f2e..e0724a8107 100644 --- a/e2e/e2e_canary_upgrade_test.go +++ b/e2e/e2e_canary_upgrade_test.go @@ -162,6 +162,7 @@ var _ = Describe("Fleetshard-sync Targeted Upgrade", Ordered, func() { } return config })).To(Succeed()) + debugGitopsConfig(ctx) Eventually(assertDeployedOperators(ctx, operator1DeploymentName)). WithTimeout(waitTimeout). WithPolling(defaultPolling). @@ -182,7 +183,7 @@ var _ = Describe("Fleetshard-sync Targeted Upgrade", Ordered, func() { Expect(err).To(Not(HaveOccurred())) Expect(constants.CentralRequestStatusAccepted.String()).To(Equal(createdCentral.Status)) centralNamespace, err = services.FormatNamespace(createdCentral.Id) - + debugGitopsConfig(ctx) Eventually(assertCentralLabelSelectorPresent(ctx, createdCentral, centralNamespace, operatorVersion1)). WithTimeout(waitTimeout). WithPolling(defaultPolling). @@ -199,6 +200,7 @@ var _ = Describe("Fleetshard-sync Targeted Upgrade", Ordered, func() { } return config })).To(Succeed()) + debugGitopsConfig(ctx) Eventually(assertCentralLabelSelectorPresent(ctx, createdCentral, centralNamespace, operatorVersion2)). WithTimeout(waitTimeout). WithPolling(defaultPolling). @@ -236,7 +238,7 @@ egressProxy: ` return config })).To(Succeed()) - + debugGitopsConfig(ctx) Eventually(func() error { egressProxy, err := getDeployment(ctx, centralNamespace, "egress-proxy") if err != nil { @@ -334,22 +336,54 @@ func putGitopsConfig(ctx context.Context, config gitops.Config) error { return k8sClient.Create(ctx, configMap) } +func debugGitopsConfig(ctx context.Context) { + var configMap v1.ConfigMap + if err := k8sClient.Get(ctx, ctrlClient.ObjectKey{Namespace: namespace, Name: gitopsConfigmapName}, &configMap); err != nil { + if errors2.IsNotFound(err) { + println("configmap not found") + return + } + println("error getting configmap", err.Error()) + return + } + var config gitops.Config + if err := yaml.Unmarshal([]byte(configMap.Data[gitopsConfigmapDataKey]), &config); err != nil { + println("error unmarshalling configmap data", err.Error()) + return + } + println("configmap data", configMap.Data[gitopsConfigmapDataKey]) +} + func updateGitopsConfig(ctx context.Context, updateFn func(config gitops.Config) gitops.Config) error { var configMap v1.ConfigMap if err := k8sClient.Get(ctx, ctrlClient.ObjectKey{Namespace: namespace, Name: gitopsConfigmapName}, &configMap); err != nil { - return err + if !errors2.IsNotFound(err) { + return err + } + configMap = v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: gitopsConfigmapName, + }, + Data: map[string]string{}, + } } var config gitops.Config if err := yaml.Unmarshal([]byte(configMap.Data[gitopsConfigmapDataKey]), &config); err != nil { return err } updated := updateFn(config) - configYAML, err := yaml.Marshal(updated) + updatedYaml, err := yaml.Marshal(updated) if err != nil { return err } - configMap.Data[gitopsConfigmapDataKey] = string(configYAML) - return k8sClient.Update(ctx, &configMap) + configMap.Data[gitopsConfigmapDataKey] = string(updatedYaml) + if err := k8sClient.Update(ctx, &configMap); err != nil { + if !errors2.IsNotFound(err) { + return err + } + } + return k8sClient.Create(ctx, &configMap) } func operatorConfigForVersion(version string) operator.OperatorConfig { From 831733c5e31e331900b4500660b59b0910dd35e0 Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Fri, 10 May 2024 09:02:50 -0400 Subject: [PATCH 03/10] ROX-24127: tenant resources via gitops --- e2e/e2e_canary_upgrade_test.go | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/e2e/e2e_canary_upgrade_test.go b/e2e/e2e_canary_upgrade_test.go index e0724a8107..5759505bf0 100644 --- a/e2e/e2e_canary_upgrade_test.go +++ b/e2e/e2e_canary_upgrade_test.go @@ -340,26 +340,29 @@ func debugGitopsConfig(ctx context.Context) { var configMap v1.ConfigMap if err := k8sClient.Get(ctx, ctrlClient.ObjectKey{Namespace: namespace, Name: gitopsConfigmapName}, &configMap); err != nil { if errors2.IsNotFound(err) { - println("configmap not found") + GinkgoLogr.Info("configmap not found") return } - println("error getting configmap", err.Error()) + GinkgoLogr.Error(err, "error getting configmap") return } var config gitops.Config if err := yaml.Unmarshal([]byte(configMap.Data[gitopsConfigmapDataKey]), &config); err != nil { - println("error unmarshalling configmap data", err.Error()) + GinkgoLogr.Error(err, "error unmarshalling configmap data") return } - println("configmap data", configMap.Data[gitopsConfigmapDataKey]) + GinkgoLogr.Info("configmap data", "config", config) } func updateGitopsConfig(ctx context.Context, updateFn func(config gitops.Config) gitops.Config) error { + exists := true var configMap v1.ConfigMap + var config gitops.Config if err := k8sClient.Get(ctx, ctrlClient.ObjectKey{Namespace: namespace, Name: gitopsConfigmapName}, &configMap); err != nil { if !errors2.IsNotFound(err) { return err } + exists = false configMap = v1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: namespace, @@ -367,23 +370,24 @@ func updateGitopsConfig(ctx context.Context, updateFn func(config gitops.Config) }, Data: map[string]string{}, } + } else { + if err := yaml.Unmarshal([]byte(configMap.Data[gitopsConfigmapDataKey]), &config); err != nil { + return err + } } - var config gitops.Config - if err := yaml.Unmarshal([]byte(configMap.Data[gitopsConfigmapDataKey]), &config); err != nil { - return err - } + updated := updateFn(config) updatedYaml, err := yaml.Marshal(updated) if err != nil { return err } configMap.Data[gitopsConfigmapDataKey] = string(updatedYaml) - if err := k8sClient.Update(ctx, &configMap); err != nil { - if !errors2.IsNotFound(err) { - return err - } + if exists { + return k8sClient.Update(ctx, &configMap) + } else { + return k8sClient.Create(ctx, &configMap) } - return k8sClient.Create(ctx, &configMap) + } func operatorConfigForVersion(version string) operator.OperatorConfig { From c8409ae10ba7fb605d93bcfe6863ff338001867e Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Fri, 10 May 2024 09:25:03 -0400 Subject: [PATCH 04/10] ROX-24127: tenant resources via gitops --- dev/config/gitops-config.yaml | 22 +++++++++++++++++++++- e2e/e2e_canary_upgrade_test.go | 20 ++++++++++---------- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/dev/config/gitops-config.yaml b/dev/config/gitops-config.yaml index f9a607419e..6531e952dc 100644 --- a/dev/config/gitops-config.yaml +++ b/dev/config/gitops-config.yaml @@ -7,7 +7,27 @@ rhacsOperators: image: "quay.io/rhacs-eng/stackrox-operator:4.3.4" centralLabelSelector: "rhacs.redhat.com/version-selector=4.3.4" securedClusterReconcilerEnabled: false - +tenantResources: + default: | + labels: + app.kubernetes.io/managed-by: rhacs-fleetshard + app.kubernetes.io/instance: { { .Name } } + rhacs.redhat.com/org-id: { { .OrganizationID } } + rhacs.redhat.com/tenant: { { .ID } } + rhacs.redhat.com/instance-type: { { .InstanceType } } + annotations: + rhacs.redhat.com/org-name: { { .OrganizationName } } + secureTenantNetwork: false + centralRdsCidrBlock: "10.1.0.0/16" + egressProxy: + image: "registry.redhat.io/openshift4/ose-egress-http-proxy:v4.14" + replicas: 2 + resources: + requests: + cpu: 100m + memory: 275Mi + limits: + memory: 275Mi centrals: overrides: - instanceIds: diff --git a/e2e/e2e_canary_upgrade_test.go b/e2e/e2e_canary_upgrade_test.go index 5759505bf0..104c438708 100644 --- a/e2e/e2e_canary_upgrade_test.go +++ b/e2e/e2e_canary_upgrade_test.go @@ -230,11 +230,11 @@ egressProxy: image: "registry.redhat.io/openshift4/ose-egress-http-proxy:v4.14" replicas: 2 resources: - requests: - cpu: 100m - memory: 200Mi - limits: - memory: 200Mi + requests: + cpu: 100m + memory: 200Mi + limits: + memory: 200Mi ` return config })).To(Succeed()) @@ -598,11 +598,11 @@ egressProxy: image: "registry.redhat.io/openshift4/ose-egress-http-proxy:v4.14" replicas: 2 resources: - requests: - cpu: 100m - memory: 275Mi - limits: - memory: 275Mi` + requests: + cpu: 100m + memory: 275Mi + limits: + memory: 275Mi` } func defaultGitopsConfig() gitops.Config { From f4c09212ba4d8880212114efce33a4a073bab5b1 Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Fri, 10 May 2024 09:26:27 -0400 Subject: [PATCH 05/10] ROX-24127: tenant resources via gitops --- dev/config/gitops-config.yaml | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/dev/config/gitops-config.yaml b/dev/config/gitops-config.yaml index 6531e952dc..7153a74d6e 100644 --- a/dev/config/gitops-config.yaml +++ b/dev/config/gitops-config.yaml @@ -11,23 +11,23 @@ tenantResources: default: | labels: app.kubernetes.io/managed-by: rhacs-fleetshard - app.kubernetes.io/instance: { { .Name } } - rhacs.redhat.com/org-id: { { .OrganizationID } } - rhacs.redhat.com/tenant: { { .ID } } - rhacs.redhat.com/instance-type: { { .InstanceType } } + app.kubernetes.io/instance: {{ .Name }} + rhacs.redhat.com/org-id: {{ .OrganizationID }} + rhacs.redhat.com/tenant: {{ .ID }} + rhacs.redhat.com/instance-type: {{ .InstanceType }} annotations: - rhacs.redhat.com/org-name: { { .OrganizationName } } + rhacs.redhat.com/org-name: {{ .OrganizationName }} secureTenantNetwork: false centralRdsCidrBlock: "10.1.0.0/16" egressProxy: image: "registry.redhat.io/openshift4/ose-egress-http-proxy:v4.14" replicas: 2 resources: - requests: - cpu: 100m - memory: 275Mi - limits: - memory: 275Mi + requests: + cpu: 100m + memory: 275Mi + limits: + memory: 275Mi centrals: overrides: - instanceIds: From 5daef34f1d4a7ae5341646648c8e68fa5fca88cf Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Fri, 10 May 2024 10:00:44 -0400 Subject: [PATCH 06/10] ROX-24127: tenant resources via gitops --- dev/config/gitops-config.yaml | 10 +++++----- e2e/e2e_canary_upgrade_test.go | 24 ++++++++++++------------ 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/dev/config/gitops-config.yaml b/dev/config/gitops-config.yaml index 7153a74d6e..99fcd780e4 100644 --- a/dev/config/gitops-config.yaml +++ b/dev/config/gitops-config.yaml @@ -11,12 +11,12 @@ tenantResources: default: | labels: app.kubernetes.io/managed-by: rhacs-fleetshard - app.kubernetes.io/instance: {{ .Name }} - rhacs.redhat.com/org-id: {{ .OrganizationID }} - rhacs.redhat.com/tenant: {{ .ID }} - rhacs.redhat.com/instance-type: {{ .InstanceType }} + app.kubernetes.io/instance: "{{ .Name }}" + rhacs.redhat.com/org-id: "{{ .OrganizationID }}" + rhacs.redhat.com/tenant: "{{ .ID }}" + rhacs.redhat.com/instance-type: "{{ .InstanceType }}" annotations: - rhacs.redhat.com/org-name: {{ .OrganizationName }} + rhacs.redhat.com/org-name: "{{ .OrganizationName }}" secureTenantNetwork: false centralRdsCidrBlock: "10.1.0.0/16" egressProxy: diff --git a/e2e/e2e_canary_upgrade_test.go b/e2e/e2e_canary_upgrade_test.go index 104c438708..999d8d813d 100644 --- a/e2e/e2e_canary_upgrade_test.go +++ b/e2e/e2e_canary_upgrade_test.go @@ -217,13 +217,13 @@ var _ = Describe("Fleetshard-sync Targeted Upgrade", Ordered, func() { Expect(updateGitopsConfig(ctx, func(config gitops.Config) gitops.Config { config.TenantResources.Default = ` labels: - app.kubernetes.io/managed-by: rhacs-fleetshard - app.kubernetes.io/instance: {{ .Name }} - rhacs.redhat.com/org-id: {{ .OrganizationID }} - rhacs.redhat.com/tenant: {{ .ID }} - rhacs.redhat.com/instance-type: {{ .InstanceType }} + app.kubernetes.io/managed-by: "rhacs-fleetshard" + app.kubernetes.io/instance: "{{ .Name }}" + rhacs.redhat.com/org-id: "{{ .OrganizationID }}" + rhacs.redhat.com/tenant: "{{ .ID }}" + rhacs.redhat.com/instance-type: "{{ .InstanceType }}" annotations: - rhacs.redhat.com/org-name: {{ .OrganizationName }} + rhacs.redhat.com/org-name: "{{ .OrganizationName }}" secureTenantNetwork: false centralRdsCidrBlock: "10.1.0.0/16" egressProxy: @@ -585,13 +585,13 @@ metadata: func defaultTenantResourceValues() string { return ` labels: - app.kubernetes.io/managed-by: rhacs-fleetshard - app.kubernetes.io/instance: {{ .Name }} - rhacs.redhat.com/org-id: {{ .OrganizationID }} - rhacs.redhat.com/tenant: {{ .ID }} - rhacs.redhat.com/instance-type: {{ .InstanceType }} + app.kubernetes.io/managed-by: "rhacs-fleetshard" + app.kubernetes.io/instance: "{{ .Name }}" + rhacs.redhat.com/org-id: "{{ .OrganizationID }}" + rhacs.redhat.com/tenant: "{{ .ID }}" + rhacs.redhat.com/instance-type: "{{ .InstanceType }}" annotations: - rhacs.redhat.com/org-name: {{ .OrganizationName }} + rhacs.redhat.com/org-name: "{{ .OrganizationName }}" secureTenantNetwork: false centralRdsCidrBlock: "10.1.0.0/16" egressProxy: From 2cb27ba9c91eaecb699e23273e48a4ce3363608d Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Fri, 10 May 2024 10:42:12 -0400 Subject: [PATCH 07/10] ROX-24127: tenant resources via gitops --- e2e/e2e_canary_upgrade_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/e2e/e2e_canary_upgrade_test.go b/e2e/e2e_canary_upgrade_test.go index 999d8d813d..c94ae9e380 100644 --- a/e2e/e2e_canary_upgrade_test.go +++ b/e2e/e2e_canary_upgrade_test.go @@ -215,7 +215,8 @@ var _ = Describe("Fleetshard-sync Targeted Upgrade", Ordered, func() { Expect(egressProxy.Spec.Template.Spec.Containers[0].Resources.Requests.Memory().String()).To(Equal("275Mi")) Expect(egressProxy.Spec.Template.Spec.Containers[0].Resources.Limits.Memory().String()).To(Equal("275Mi")) Expect(updateGitopsConfig(ctx, func(config gitops.Config) gitops.Config { - config.TenantResources.Default = ` + tenantResources := config.TenantResources + tenantResources.Default = ` labels: app.kubernetes.io/managed-by: "rhacs-fleetshard" app.kubernetes.io/instance: "{{ .Name }}" @@ -236,6 +237,7 @@ egressProxy: limits: memory: 200Mi ` + config.TenantResources = tenantResources return config })).To(Succeed()) debugGitopsConfig(ctx) From 52eff2bda273fcff1e0a19131dc434d066be3c43 Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Fri, 10 May 2024 10:42:45 -0400 Subject: [PATCH 08/10] ROX-24127: tenant resources via gitops --- e2e/e2e_canary_upgrade_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/e2e/e2e_canary_upgrade_test.go b/e2e/e2e_canary_upgrade_test.go index c94ae9e380..6d1072e68e 100644 --- a/e2e/e2e_canary_upgrade_test.go +++ b/e2e/e2e_canary_upgrade_test.go @@ -231,11 +231,11 @@ egressProxy: image: "registry.redhat.io/openshift4/ose-egress-http-proxy:v4.14" replicas: 2 resources: - requests: - cpu: 100m - memory: 200Mi - limits: - memory: 200Mi + requests: + cpu: 100m + memory: 200Mi + limits: + memory: 200Mi ` config.TenantResources = tenantResources return config From 3e674d8bf5dc260c25d782617589083e96501f33 Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Mon, 13 May 2024 11:36:02 -0400 Subject: [PATCH 09/10] ROX-24127: PR Comments --- internal/dinosaur/pkg/gitops/config.go | 16 +++++++++------- internal/dinosaur/pkg/gitops/service.go | 13 ------------- 2 files changed, 9 insertions(+), 20 deletions(-) diff --git a/internal/dinosaur/pkg/gitops/config.go b/internal/dinosaur/pkg/gitops/config.go index b598298c6c..06e2d203fb 100644 --- a/internal/dinosaur/pkg/gitops/config.go +++ b/internal/dinosaur/pkg/gitops/config.go @@ -131,7 +131,7 @@ func validateTenantResourcesConfig(path *field.Path, config TenantResourceConfig func validateTenantResourcesDefault(path *field.Path, defaultValues string) field.ErrorList { var errs field.ErrorList - if err := tryRenderDummyValuesWithPatch(defaultValues); err != nil { + if err := renderDummyValuesWithPatchForValidation(defaultValues); err != nil { errs = append(errs, field.Invalid(path, defaultValues, "invalid default values: "+err.Error())) } return errs @@ -148,7 +148,7 @@ func validateTenantResourceOverrides(path *field.Path, overrides []TenantResourc func validateTenantResourceOverride(path *field.Path, override TenantResourceOverride) field.ErrorList { var errs field.ErrorList errs = append(errs, validateInstanceIDs(path.Child("instanceIds"), override.InstanceIDs)...) - if err := tryRenderDummyValuesWithPatch(override.Values); err != nil { + if err := renderDummyValuesWithPatchForValidation(override.Values); err != nil { errs = append(errs, field.Invalid(path.Child("values"), override.Values, "invalid values: "+err.Error())) } return errs @@ -301,15 +301,15 @@ func validatePatch(path *field.Path, patch string) field.ErrorList { errs = append(errs, field.Required(path, "patch is required")) return errs } - if err := tryRenderDummyCentralWithPatch(patch); err != nil { + if err := renderDummyCentralWithPatchForValidation(patch); err != nil { errs = append(errs, field.Invalid(path, patch, "invalid patch: "+err.Error())) } return errs } -// tryRenderDummyCentralWithPatch renders a dummy Central instance with the given patch. -// useful to test that a patch is valid. -func tryRenderDummyCentralWithPatch(patch string) error { +// renderDummyCentralWithPatchForValidation renders a dummy Central instance with the given patch. +// useful to test that a Central patch is valid. +func renderDummyCentralWithPatchForValidation(patch string) error { var dummyParams = getDummyCentralParams() dummyConfig := Config{ Centrals: CentralsConfig{ @@ -327,7 +327,9 @@ func tryRenderDummyCentralWithPatch(patch string) error { return nil } -func tryRenderDummyValuesWithPatch(patch string) error { +// renderDummyValuesWithPatchForValidation renders a dummy tenant resource values with the given patch. +// useful to test that a values patch is valid. +func renderDummyValuesWithPatchForValidation(patch string) error { var dummyParams = getDummyCentralParams() dummyConfig := Config{ TenantResources: TenantResourceConfig{ diff --git a/internal/dinosaur/pkg/gitops/service.go b/internal/dinosaur/pkg/gitops/service.go index 9a56e22383..95a8b197c1 100644 --- a/internal/dinosaur/pkg/gitops/service.go +++ b/internal/dinosaur/pkg/gitops/service.go @@ -179,16 +179,3 @@ func renderPatchTemplate(patchTemplate string, ctx CentralParams) (string, error // defaultTemplate is the default template for Central instances. var defaultTemplate = template.Must(template.New("default").Parse(string(defaultCentralTemplate))) - -func copyMap(m map[string]interface{}) map[string]interface{} { - result := make(map[string]interface{}) - for k, v := range m { - switch v := v.(type) { - case map[string]interface{}: - result[k] = copyMap(v) - default: - result[k] = v - } - } - return result -} From 7849fea60598b7d99c5b54a3ba46dd2c24369404 Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Mon, 13 May 2024 11:37:13 -0400 Subject: [PATCH 10/10] ROX-24127: PR Comments --- internal/dinosaur/pkg/gitops/config.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/dinosaur/pkg/gitops/config.go b/internal/dinosaur/pkg/gitops/config.go index 06e2d203fb..6107300791 100644 --- a/internal/dinosaur/pkg/gitops/config.go +++ b/internal/dinosaur/pkg/gitops/config.go @@ -100,6 +100,7 @@ type TenantResourceConfig struct { } // TenantResourceOverride represents the configuration for a tenant resource override. The override +// will be applied on top of the default tenant resource values configuration. type TenantResourceOverride struct { InstanceIDs []string `json:"instanceIds"` Values string `json:"values"`