From 919d3f67e69a912d6589634129e65a1505d240d4 Mon Sep 17 00:00:00 2001 From: Danil-Grigorev Date: Tue, 15 Jun 2021 14:59:59 +0200 Subject: [PATCH] Implement image substitution for Pod and generalise the approach for other objects --- pkg/substitution/substitution.go | 45 ++++----- pkg/substitution/substitution_test.go | 130 +++++++++++++------------- 2 files changed, 87 insertions(+), 88 deletions(-) diff --git a/pkg/substitution/substitution.go b/pkg/substitution/substitution.go index cc8deef47..bc8b086d7 100644 --- a/pkg/substitution/substitution.go +++ b/pkg/substitution/substitution.go @@ -2,7 +2,8 @@ package substitution import ( "github.com/openshift/cluster-cloud-controller-manager-operator/pkg/config" - v1 "k8s.io/api/apps/v1" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -14,27 +15,27 @@ const ( cloudNodeManagerName = "cloud-node-manager" ) -// setDeploymentImages substitutes controller containers in Deployment with correct image -func setDeploymentImages(config config.OperatorConfig, d *v1.Deployment) { - for i, container := range d.Spec.Template.Spec.Containers { - if container.Name != cloudControllerManagerName { +// setCloudControllerImage substitutes controller containers in provided pod specs with correct image +func setCloudControllerImage(config config.OperatorConfig, p corev1.PodSpec) corev1.PodSpec { + updatedPod := *p.DeepCopy() + for i, container := range p.Containers { + substituteName := "" + switch container.Name { + case cloudControllerManagerName: + substituteName = config.ControllerImage + case cloudNodeManagerName: + substituteName = config.CloudNodeImage + default: continue } - klog.Infof("Substituting %q in Deployment %q with %s", container.Name, d.Name, config.ControllerImage) - d.Spec.Template.Spec.Containers[i].Image = config.ControllerImage - } -} - -func setDaemonSetImage(config config.OperatorConfig, d *v1.DaemonSet) { - for i, container := range d.Spec.Template.Spec.Containers { - if container.Name != cloudNodeManagerName { - continue + if substituteName != "" { + klog.Infof("Substituting container image for container %q with %q", container.Name, substituteName) + updatedPod.Containers[i].Image = substituteName } - - klog.Infof("Substituting %q in DaemonSet %q with %s", container.Name, d.Name, config.ControllerImage) - d.Spec.Template.Spec.Containers[i].Image = config.CloudNodeImage } + + return updatedPod } func FillConfigValues(config config.OperatorConfig, templates []client.Object) []client.Object { @@ -46,10 +47,12 @@ func FillConfigValues(config config.OperatorConfig, templates []client.Object) [ templateCopy.SetNamespace(config.ManagedNamespace) switch obj := templateCopy.(type) { - case *v1.Deployment: - setDeploymentImages(config, obj) - case *v1.DaemonSet: - setDaemonSetImage(config, obj) + case *appsv1.Deployment: + obj.Spec.Template.Spec = setCloudControllerImage(config, obj.Spec.Template.Spec) + case *appsv1.DaemonSet: + obj.Spec.Template.Spec = setCloudControllerImage(config, obj.Spec.Template.Spec) + case *corev1.Pod: + obj.Spec = setCloudControllerImage(config, obj.Spec) } objects[i] = templateCopy } diff --git a/pkg/substitution/substitution_test.go b/pkg/substitution/substitution_test.go index 2bfd510c0..16e214580 100644 --- a/pkg/substitution/substitution_test.go +++ b/pkg/substitution/substitution_test.go @@ -11,7 +11,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -func TestSetDeploymentImages(t *testing.T) { +func TestSetCloudControllerImage(t *testing.T) { tc := []struct { name string containers []corev1.Container @@ -43,6 +43,19 @@ func TestSetDeploymentImages(t *testing.T) { config: config.OperatorConfig{ ControllerImage: "correct_image:tag", }, + }, { + name: "Substitute cloud-node-manager container image", + containers: []corev1.Container{{ + Name: cloudNodeManagerName, + Image: "expect_change", + }}, + expectedContainers: []corev1.Container{{ + Name: cloudNodeManagerName, + Image: "correct_node_image:tag", + }}, + config: config.OperatorConfig{ + CloudNodeImage: "correct_node_image:tag", + }, }, { name: "Combination of container image names", containers: []corev1.Container{{ @@ -76,69 +89,6 @@ func TestSetDeploymentImages(t *testing.T) { }, } - setDeploymentImages(tc.config, deploy) - - assert.EqualValues(t, deploy.Spec.Template.Spec.Containers, tc.expectedContainers) - }) - } - -} - -func TestSetDaemonsetImages(t *testing.T) { - tc := []struct { - name string - containers []corev1.Container - config config.OperatorConfig - expectedContainers []corev1.Container - }{{ - name: "Unknown container name", - containers: []corev1.Container{{ - Name: "different_name", - Image: "no_change", - }}, - expectedContainers: []corev1.Container{{ - Name: "different_name", - Image: "no_change", - }}, - config: config.OperatorConfig{ - CloudNodeImage: "correct_image:tag", - }, - }, { - name: "Substitute cloud-node-manager container image", - containers: []corev1.Container{{ - Name: cloudNodeManagerName, - Image: "expect_change", - }}, - expectedContainers: []corev1.Container{{ - Name: cloudNodeManagerName, - Image: "correct_image:tag", - }}, - config: config.OperatorConfig{ - CloudNodeImage: "correct_image:tag", - }, - }, { - name: "Combination of container image names", - containers: []corev1.Container{{ - Name: cloudNodeManagerName, - Image: "expect_change", - }, { - Name: "some-stuff-there", - Image: "no_change", - }}, - expectedContainers: []corev1.Container{{ - Name: cloudNodeManagerName, - Image: "correct_image:tag", - }, { - Name: "some-stuff-there", - Image: "no_change", - }}, - config: config.OperatorConfig{ - CloudNodeImage: "correct_image:tag", - }, - }} - - for _, tc := range tc { - t.Run(tc.name, func(t *testing.T) { ds := &v1.DaemonSet{ Spec: v1.DaemonSetSpec{ Template: corev1.PodTemplateSpec{ @@ -149,11 +99,23 @@ func TestSetDaemonsetImages(t *testing.T) { }, } - setDaemonSetImage(tc.config, ds) + pod := &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: tc.containers, + }, + } + + spec := setCloudControllerImage(tc.config, deploy.Spec.Template.Spec) + assert.EqualValues(t, spec.Containers, tc.expectedContainers) - assert.EqualValues(t, ds.Spec.Template.Spec.Containers, tc.expectedContainers) + spec = setCloudControllerImage(tc.config, ds.Spec.Template.Spec) + assert.EqualValues(t, spec.Containers, tc.expectedContainers) + + spec = setCloudControllerImage(tc.config, pod.Spec) + assert.EqualValues(t, spec.Containers, tc.expectedContainers) }) } + } func TestFillConfigValues(t *testing.T) { @@ -247,7 +209,7 @@ func TestFillConfigValues(t *testing.T) { ManagedNamespace: testManagementNamespace, }, }, { - name: "Substitute image and namespace for more deployment and daemonset", + name: "Substitute image and namespace for deployment, daemonset and pod", objects: []client.Object{&v1.DaemonSet{ Spec: v1.DaemonSetSpec{ Template: corev1.PodTemplateSpec{ @@ -259,6 +221,20 @@ func TestFillConfigValues(t *testing.T) { }, }, }, + }, &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: cloudControllerManagerName, + Image: "expect_change", + }}, + }, + }, &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: cloudNodeManagerName, + Image: "expect_change", + }}, + }, }, &v1.Deployment{ Spec: v1.DeploymentSpec{ Template: corev1.PodTemplateSpec{ @@ -286,6 +262,26 @@ func TestFillConfigValues(t *testing.T) { }, }, }, + }, &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testManagementNamespace, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: cloudControllerManagerName, + Image: "correct_image:tag", + }}, + }, + }, &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testManagementNamespace, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: cloudNodeManagerName, + Image: "correct_cloud_node_image:tag", + }}, + }, }, &v1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Namespace: testManagementNamespace,