From e915cb589aa98bbc1e2b75a29348a7de4efd134e Mon Sep 17 00:00:00 2001 From: Michael Burman Date: Tue, 26 Sep 2023 16:34:23 +0300 Subject: [PATCH] Ensure server-config-init and server-config-init-base are set in the initContainers, even if one or the other is using overrides (#572) --- CHANGELOG.md | 2 + .../construct_podtemplatespec.go | 38 ++-- .../construct_podtemplatespec_test.go | 166 ++++++++++++++++++ ...rack-single-node-addtional-volumes-dc.yaml | 3 + 4 files changed, 197 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eaa9cc12..e121fe87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ Changelog for Cass Operator, new PRs should update the `main / unreleased` secti ## unreleased +* [ENHANCEMENT] [#571](https://github.com/k8ssandra/cass-operator/issues/571) Ensure both "server-config-init" as well as "server-config-init-base" are always created in the initContainers if 4.1.x is used. + ## v1.17.1 * [CHANGE] [#541](https://github.com/k8ssandra/cass-operator/issues/541) Revert when deployed through OLM, add serviceAccount to Cassandra pods that use nonroot priviledge. This is no longer necessary with 1.17.0 and up. diff --git a/pkg/reconciliation/construct_podtemplatespec.go b/pkg/reconciliation/construct_podtemplatespec.go index 70872283..6abf799f 100644 --- a/pkg/reconciliation/construct_podtemplatespec.go +++ b/pkg/reconciliation/construct_podtemplatespec.go @@ -376,14 +376,20 @@ func symmetricDifference(list1 []corev1.Volume, list2 []corev1.Volume) []corev1. func buildInitContainers(dc *api.CassandraDatacenter, rackName string, baseTemplate *corev1.PodTemplateSpec) error { serverCfg := &corev1.Container{} - foundOverrides := false + configContainer := &corev1.Container{} + + serverContainerIndex := -1 + configContainerIndex := -1 for i, c := range baseTemplate.Spec.InitContainers { if c.Name == ServerConfigContainerName { // Modify the existing container - foundOverrides = true + serverContainerIndex = i serverCfg = &baseTemplate.Spec.InitContainers[i] - break + } + if c.Name == ServerBaseConfigContainerName { + configContainerIndex = i + configContainer = &baseTemplate.Spec.InitContainers[i] } } @@ -420,7 +426,6 @@ func buildInitContainers(dc *api.CassandraDatacenter, rackName string, baseTempl configMounts := []corev1.VolumeMount{serverCfgMount} - var configContainer *corev1.Container if dc.UseClientImage() { configBaseMount := corev1.VolumeMount{ @@ -431,8 +436,10 @@ func buildInitContainers(dc *api.CassandraDatacenter, rackName string, baseTempl configMounts = append(configMounts, configBaseMount) // Similar to k8ssandra 1.x, use config-container if use new config-builder replacement - configContainer = &corev1.Container{ - Name: ServerBaseConfigContainerName, + if configContainerIndex < 0 { + configContainer = &corev1.Container{ + Name: ServerBaseConfigContainerName, + } } if configContainer.Image == "" { @@ -483,16 +490,23 @@ func buildInitContainers(dc *api.CassandraDatacenter, rackName string, baseTempl serverCfg.Env = combineEnvSlices(envDefaults, serverCfg.Env) - if !foundOverrides { - // Note that append makes a copy, so we must do this after - // serverCfg has been properly set up. - if dc.UseClientImage() { - baseTemplate.Spec.InitContainers = append(baseTemplate.Spec.InitContainers, *configContainer, *serverCfg) + if dc.UseClientImage() && configContainerIndex < 0 { + if serverContainerIndex >= 0 { + // Set before serverCfg + if serverContainerIndex == 0 { + baseTemplate.Spec.InitContainers = append([]corev1.Container{*configContainer}, baseTemplate.Spec.InitContainers...) + } else { + baseTemplate.Spec.InitContainers = append(baseTemplate.Spec.InitContainers[:serverContainerIndex], append([]corev1.Container{*configContainer}, baseTemplate.Spec.InitContainers[serverContainerIndex:]...)...) + } } else { - baseTemplate.Spec.InitContainers = append(baseTemplate.Spec.InitContainers, *serverCfg) + baseTemplate.Spec.InitContainers = append(baseTemplate.Spec.InitContainers, *configContainer) } } + if serverContainerIndex < 0 { + baseTemplate.Spec.InitContainers = append(baseTemplate.Spec.InitContainers, *serverCfg) + } + return nil } diff --git a/pkg/reconciliation/construct_podtemplatespec_test.go b/pkg/reconciliation/construct_podtemplatespec_test.go index d694fe2f..b9886b94 100644 --- a/pkg/reconciliation/construct_podtemplatespec_test.go +++ b/pkg/reconciliation/construct_podtemplatespec_test.go @@ -1207,6 +1207,172 @@ func TestCassandraDatacenter_buildPodTemplateSpec_clientImage(t *testing.T) { assert.True(volumesContains(volumes, volumeNameMatcher("vector-lib"))) } +func TestCassandraDatacenter_buildPodTemplateSpec_clientImage_withContainerOverrides(t *testing.T) { + assert := assert.New(t) + + // Example from k8ssandra-operator when medusa is used + dc41 := &api.CassandraDatacenter{ + Spec: api.CassandraDatacenterSpec{ + ClusterName: "bob", + ServerType: "cassandra", + ServerVersion: "4.1.2", + Racks: []api.Rack{ + { + Name: "default", + }, + }, + PodTemplateSpec: &corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + InitContainers: []corev1.Container{ + { + Name: ServerConfigContainerName, + Resources: corev1.ResourceRequirements{}, + }, + }, + }, + }, + }, + } + + spec41, err := buildPodTemplateSpec(dc41, dc41.Spec.Racks[0], false) + assert.NoError(err, "should not have gotten error when building podTemplateSpec") + + initContainers := spec41.Spec.InitContainers + + assert.Equal(2, len(initContainers)) + + serverBaseConfigInitContainer := initContainers[0] + assert.Equal(ServerBaseConfigContainerName, serverBaseConfigInitContainer.Name) + assert.Equal(1, len(serverBaseConfigInitContainer.VolumeMounts)) + // We use a contains check here because the ordering is not important + assert.True(volumeMountsContains(serverBaseConfigInitContainer.VolumeMounts, volumeMountNameMatcher("server-config-base"))) + + serverConfigInitContainer := initContainers[1] + assert.Equal(ServerConfigContainerName, serverConfigInitContainer.Name) + assert.Equal(2, len(serverConfigInitContainer.VolumeMounts)) + // We use a contains check here because the ordering is not important + assert.True(volumeMountsContains(serverConfigInitContainer.VolumeMounts, volumeMountNameMatcher("server-config"))) + assert.True(volumeMountsContains(serverConfigInitContainer.VolumeMounts, volumeMountNameMatcher("server-config-base"))) + + volumes := spec41.Spec.Volumes + assert.Equal(4, len(volumes)) + // We use a contains check here because the ordering is not important + assert.True(volumesContains(volumes, volumeNameMatcher("server-config"))) + assert.True(volumesContains(volumes, volumeNameMatcher("server-logs"))) + assert.True(volumesContains(volumes, volumeNameMatcher("server-config-base"))) + assert.True(volumesContains(volumes, volumeNameMatcher("vector-lib"))) + + // Give extra containers and swap the orders + + dc41 = &api.CassandraDatacenter{ + Spec: api.CassandraDatacenterSpec{ + ClusterName: "bob", + ServerType: "cassandra", + ServerVersion: "4.1.2", + Racks: []api.Rack{ + { + Name: "default", + }, + }, + PodTemplateSpec: &corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + InitContainers: []corev1.Container{ + { + Name: "my-own-init-container", + }, + { + Name: ServerBaseConfigContainerName, + Resources: corev1.ResourceRequirements{}, + }, + }, + }, + }, + }, + } + + spec41, err = buildPodTemplateSpec(dc41, dc41.Spec.Racks[0], false) + assert.NoError(err, "should not have gotten error when building podTemplateSpec") + initContainers = spec41.Spec.InitContainers + assert.Equal(3, len(initContainers)) + + assert.Equal(ServerBaseConfigContainerName, initContainers[1].Name) + assert.Equal(ServerConfigContainerName, initContainers[2].Name) + + // + + dc41 = &api.CassandraDatacenter{ + Spec: api.CassandraDatacenterSpec{ + ClusterName: "bob", + ServerType: "cassandra", + ServerVersion: "4.1.2", + Racks: []api.Rack{ + { + Name: "default", + }, + }, + PodTemplateSpec: &corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + InitContainers: []corev1.Container{ + { + Name: ServerBaseConfigContainerName, + Resources: corev1.ResourceRequirements{}, + }, + { + Name: "my-own-init-container", + }, + }, + }, + }, + }, + } + + spec41, err = buildPodTemplateSpec(dc41, dc41.Spec.Racks[0], false) + assert.NoError(err, "should not have gotten error when building podTemplateSpec") + initContainers = spec41.Spec.InitContainers + assert.Equal(3, len(initContainers)) + + assert.Equal(ServerBaseConfigContainerName, initContainers[0].Name) + assert.Equal("my-own-init-container", initContainers[1].Name) + assert.Equal(ServerConfigContainerName, initContainers[2].Name) + + // + + dc41 = &api.CassandraDatacenter{ + Spec: api.CassandraDatacenterSpec{ + ClusterName: "bob", + ServerType: "cassandra", + ServerVersion: "4.1.2", + Racks: []api.Rack{ + { + Name: "default", + }, + }, + PodTemplateSpec: &corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + InitContainers: []corev1.Container{ + { + Name: ServerConfigContainerName, + Resources: corev1.ResourceRequirements{}, + }, + { + Name: "my-own-init-container", + }, + }, + }, + }, + }, + } + + spec41, err = buildPodTemplateSpec(dc41, dc41.Spec.Racks[0], false) + assert.NoError(err, "should not have gotten error when building podTemplateSpec") + initContainers = spec41.Spec.InitContainers + assert.Equal(3, len(initContainers)) + + assert.Equal(ServerBaseConfigContainerName, initContainers[0].Name) + assert.Equal(ServerConfigContainerName, initContainers[1].Name) + assert.Equal("my-own-init-container", initContainers[2].Name) +} + func TestCassandraDatacenter_buildContainers_DisableSystemLoggerSidecar(t *testing.T) { dc := &api.CassandraDatacenter{ Spec: api.CassandraDatacenterSpec{ diff --git a/tests/testdata/default-single-rack-single-node-addtional-volumes-dc.yaml b/tests/testdata/default-single-rack-single-node-addtional-volumes-dc.yaml index 3b506971..07cefa42 100644 --- a/tests/testdata/default-single-rack-single-node-addtional-volumes-dc.yaml +++ b/tests/testdata/default-single-rack-single-node-addtional-volumes-dc.yaml @@ -28,6 +28,9 @@ spec: - name: r1 podTemplateSpec: spec: + initContainers: + - name: server-config-init + resources: {} containers: - args: ["/bin/sh", "-c", "tail -n+1 -F /var/log/cassandra/system.log"] image: busybox