Skip to content
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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` directive from the Kubernetes Strategic Merge Patch specification is **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.
Comment thread
rm3l marked this conversation as resolved.
Outdated

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.
Expand Down
20 changes: 19 additions & 1 deletion pkg/model/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
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{}

Expand Down Expand Up @@ -227,11 +228,28 @@
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,

Check warning on line 244 in pkg/model/deployment.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Complete the task associated to this TODO comment.

See more on https://sonarcloud.io/project/issues?id=redhat-developer_rhdh-operator&issues=AZ4cJdca7cPIRS32xFDk&open=AZ4cJdca7cPIRS32xFDk&pullRequest=2824
// 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 {
Expand Down
101 changes: 97 additions & 4 deletions pkg/model/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,25 +140,118 @@ 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)

// backstage container resources updated
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{}
Expand Down
Loading