Merge pull request #58 from Danil-Grigorev/substitute-pod

[OCPCLOUD-1162] Implement image substitution for Pod and generalise the approach for other objects
openshift · Jun 23, 2021 · 3832ff4 · 3832ff4
2 parents b0fb4b3 + 919d3f6
commit 3832ff4
Show file tree

Hide file tree

Showing 2 changed files with 87 additions and 88 deletions.
diff --git 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
@@ -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,