diff --git a/docs/configuration.md b/docs/configuration.md index e774c5b53..284a50a3a 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -837,6 +837,62 @@ spec: claimName: dynamic-plugins-root ``` +##### List ordering + +By default, new list items in the patch (containers, init containers, volumes, etc.) are **appended** after existing items. Items that match an existing entry by name are merged in-place and retain their original position — their placement in the patch does not reorder them in the result. + +If the ordering of list items matters (e.g., init containers, where execution order is [significant](https://kubernetes.io/docs/concepts/workloads/pods/init-containers/#detailed-behavior)), you can opt in to **prepend** mode by setting the `rhdh.redhat.com/deployment-patch-list-merge-mode` annotation on the Backstage CR. In prepend mode, you can control the exact ordering by listing all items (including existing ones) explicitly in the patch in the desired order. + +> **Note:** The `$setElementOrder`, `$deleteFromPrimitiveList`, and `$retainKeys` directives from the Kubernetes Strategic Merge Patch specification are **not supported** by the [kyaml](https://github.com/kubernetes-sigs/kustomize/tree/master/kyaml) (kustomize) library used by this operator. Listing all items explicitly in the patch is the recommended way to control ordering. + +For example, to run a custom init container **before** `install-dynamic-plugins`: + +```yaml +apiVersion: rhdh.redhat.com/v1alpha5 +kind: Backstage +metadata: + name: my-rhdh + annotations: + rhdh.redhat.com/deployment-patch-list-merge-mode: prepend +spec: + deployment: + patch: + spec: + template: + spec: + initContainers: + - name: my-init + image: busybox + command: ["sh", "-c", "echo preparing"] + - name: install-dynamic-plugins +``` + +The resulting init container order will be: `my-init`, then `install-dynamic-plugins`. + +To run a custom init container **after** `install-dynamic-plugins`: + +```yaml +apiVersion: rhdh.redhat.com/v1alpha5 +kind: Backstage +metadata: + name: my-rhdh + annotations: + rhdh.redhat.com/deployment-patch-list-merge-mode: prepend +spec: + deployment: + patch: + spec: + template: + spec: + initContainers: + - name: install-dynamic-plugins + - name: my-init + image: busybox + command: ["sh", "-c", "echo preparing"] +``` + +The resulting init container order will be: `install-dynamic-plugins`, then `my-init`. Listing `install-dynamic-plugins` by name (without any other fields) anchors it in position, and new items listed after it are placed accordingly. + ##### Handling Discriminated Unions When patching Kubernetes resources that contain **discriminated unions** (fields where one field determines which other fields are valid), you may need to use the `$patch: delete` directive to remove conflicting fields. diff --git a/pkg/model/deployment.go b/pkg/model/deployment.go index 0989a1bce..dcf378220 100644 --- a/pkg/model/deployment.go +++ b/pkg/model/deployment.go @@ -31,6 +31,7 @@ const ( const BackstageImageEnvVar = "RELATED_IMAGE_backstage" const DefaultMountDir = "/opt/app-root/src" const ExtConfigHashAnnotation = "rhdh.redhat.com/ext-config-hash" +const ListMergeAnnotation = "rhdh.redhat.com/deployment-patch-list-merge-mode" type BackstageDeploymentFactory struct{} @@ -227,11 +228,28 @@ func (b *BackstageDeployment) setDeployment(backstage api.Backstage) error { return fmt.Errorf("can not marshal deployment object: %w", err) } - merged, err := merge2.MergeStrings(string(conf.Raw), string(deplStr), false, kyaml.MergeOptions{}) + mergeOpts := kyaml.MergeOptions{} + switch backstage.GetAnnotations()[ListMergeAnnotation] { + case "prepend": + mergeOpts.ListIncreaseDirection = kyaml.MergeOptionsListPrepend + case "append": + mergeOpts.ListIncreaseDirection = kyaml.MergeOptionsListAppend + } + + merged, err := merge2.MergeStrings(string(conf.Raw), string(deplStr), false, mergeOpts) if err != nil { return fmt.Errorf("can not merge spec.deployment: %w", err) } + // TODO(asoro): once https://github.com/kubernetes-sigs/kustomize/issues/6146 is resolved, + // remove this second pass and use only ListPrepend above. + if mergeOpts.ListIncreaseDirection == kyaml.MergeOptionsListPrepend { + merged, err = merge2.MergeStrings(string(conf.Raw), merged, false, kyaml.MergeOptions{}) + if err != nil { + return fmt.Errorf("can not merge spec.deployment: %w", err) + } + } + b.deployable.SetEmpty() err = yaml.Unmarshal([]byte(merged), b.deployable.GetObject()) if err != nil { diff --git a/pkg/model/deployment_test.go b/pkg/model/deployment_test.go index c8e3f6efb..e956f31b0 100644 --- a/pkg/model/deployment_test.go +++ b/pkg/model/deployment_test.go @@ -140,8 +140,9 @@ spec: assert.Equal(t, "java", model.backstageDeployment.deployable.GetObject().GetLabels()["mylabel"]) assert.Equal(t, "backstage", model.backstageDeployment.deployable.PodObjectMeta().GetLabels()["pod"]) - // sidecar added + // sidecar appended (default merge behavior — no prepend annotation) assert.Equal(t, 2, len(model.backstageDeployment.podSpec().Containers)) + assert.Equal(t, "backstage-backend", model.backstageDeployment.podSpec().Containers[0].Name) assert.Equal(t, "sidecar", model.backstageDeployment.podSpec().Containers[1].Name) assert.Equal(t, "my-image:1.0.0", model.backstageDeployment.podSpec().Containers[1].Image) @@ -149,16 +150,108 @@ spec: assert.Equal(t, "backstage-backend", model.backstageDeployment.container().Name) assert.Equal(t, "257Mi", model.backstageDeployment.container().Resources.Requests.Memory().String()) - // volumes - // dynamic-plugins-root, dynamic-plugins-npmrc, dynamic-plugins-auth, my-vol + // volumes: dynamic-plugins-root (merged in-place), dynamic-plugins-npmrc, dynamic-plugins-registry-auth, my-vol (appended) assert.Equal(t, 4, len(model.backstageDeployment.podSpec().Volumes)) assert.Equal(t, "dynamic-plugins-root", model.backstageDeployment.podSpec().Volumes[0].Name) // overrides StorageClassName assert.Equal(t, "special", *model.backstageDeployment.podSpec().Volumes[0].Ephemeral.VolumeClaimTemplate.Spec.StorageClassName) - // adds new volume + // new volume appended at end assert.Equal(t, "my-vol", model.backstageDeployment.podSpec().Volumes[3].Name) } +// https://redhat.atlassian.net/browse/RHDHBUGS-2900 +func TestInitContainerOrderInSpecDeployment(t *testing.T) { + tests := []struct { + name string + patch string + expected []string + }{ + { + name: "new init container runs before existing", + patch: ` +spec: + template: + spec: + initContainers: + - name: my-init + image: busybox + command: ["sh", "-c", "echo init"] +`, + expected: []string{"my-init", "install-dynamic-plugins"}, + }, + { + name: "new init container runs before existing by anchoring", + patch: ` +spec: + template: + spec: + initContainers: + - name: my-init + image: busybox + command: ["sh", "-c", "echo init"] + - name: install-dynamic-plugins +`, + expected: []string{"my-init", "install-dynamic-plugins"}, + }, + { + name: "new init container runs after existing by anchoring", + patch: ` +spec: + template: + spec: + initContainers: + - name: install-dynamic-plugins + - name: my-init + image: busybox + command: ["sh", "-c", "echo init"] +`, + expected: []string{"install-dynamic-plugins", "my-init"}, + }, + { + name: "multiple new init containers with mixed ordering", + patch: ` +spec: + template: + spec: + initContainers: + - name: pre-init + image: busybox + command: ["sh", "-c", "echo pre"] + - name: install-dynamic-plugins + - name: post-init + image: busybox + command: ["sh", "-c", "echo post"] +`, + expected: []string{"pre-init", "install-dynamic-plugins", "post-init"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + bs := *deploymentTestBackstage.DeepCopy() + bs.Annotations = map[string]string{ + ListMergeAnnotation: "prepend", + } + bs.Spec.Deployment = &api.BackstageDeployment{} + bs.Spec.Deployment.Patch = &apiextensionsv1.JSON{ + Raw: []byte(tt.patch), + } + + testObj := createBackstageTest(bs).withDefaultConfig(true). + addToDefaultConfig("deployment.yaml", "rhdh-deployment.yaml") + + model, err := InitObjects(context.TODO(), bs, testObj.externalConfig, platform.OpenShift, testObj.scheme) + assert.NoError(t, err) + + initContainers := model.backstageDeployment.podSpec().InitContainers + assert.Equal(t, len(tt.expected), len(initContainers)) + for i, name := range tt.expected { + assert.Equal(t, name, initContainers[i].Name) + } + }) + } +} + func TestImageInCRPrevailsOnEnvVar(t *testing.T) { bs := *deploymentTestBackstage.DeepCopy() bs.Spec.Deployment = &api.BackstageDeployment{}