From 10a62c79161653c01407307b340ffca86d6dc8d2 Mon Sep 17 00:00:00 2001 From: Denis Moiseev Date: Thu, 20 May 2021 16:11:52 +0200 Subject: [PATCH 1/3] add azure images --- ...d-controller-manager-operator_01_images.configmap.yaml | 2 ++ manifests/image-references | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/manifests/0000_26_cloud-controller-manager-operator_01_images.configmap.yaml b/manifests/0000_26_cloud-controller-manager-operator_01_images.configmap.yaml index 4c238d8b5..26ee8bf87 100644 --- a/manifests/0000_26_cloud-controller-manager-operator_01_images.configmap.yaml +++ b/manifests/0000_26_cloud-controller-manager-operator_01_images.configmap.yaml @@ -10,5 +10,7 @@ data: images.json: > { "cloudControllerManagerAWS": "registry.ci.openshift.org/openshift:aws-cloud-controller-manager", + "cloudControllerManagerAzure": "registry.ci.openshift.org/openshift:azure-cloud-controller-manager", + "cloudNodeManagerAzure": "registry.ci.openshift.org/openshift:azure-cloud-node-manager", "cloudControllerManagerOpenStack": "registry.ci.openshift.org/openshift:openstack-cloud-controller-manager" } diff --git a/manifests/image-references b/manifests/image-references index c79cb13ee..29ed94fe9 100644 --- a/manifests/image-references +++ b/manifests/image-references @@ -10,6 +10,14 @@ spec: from: kind: DockerImage name: registry.ci.openshift.org/openshift:aws-cloud-controller-manager + - name: azure-cloud-controller-manager + from: + kind: DockerImage + name: registry.ci.openshift.org/openshift:azure-cloud-controller-manager + - name: azure-cloud-node-manager + from: + kind: DockerImage + name: registry.ci.openshift.org/openshift:azure-cloud-node-manager - name: openstack-cloud-controller-manager from: kind: DockerImage From 95e04e915d9ae29e45796c72c7ecf53cf5d375c6 Mon Sep 17 00:00:00 2001 From: Denis Moiseev Date: Fri, 21 May 2021 16:16:39 +0200 Subject: [PATCH 2/3] add cloud node manager image and daemonset substitution --- pkg/config/config.go | 15 +++++++ pkg/substitution/substitution.go | 22 +++++++--- pkg/substitution/substitution_test.go | 60 +++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 5 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 6ea5bda6b..877789260 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -13,6 +13,8 @@ import ( // imagesReference allows build systems to inject imagesReference for CCCMO components type imagesReference struct { CloudControllerManagerAWS string `json:"cloudControllerManagerAWS"` + CloudControllerManagerAzure string `json:"cloudControllerManagerAzure"` + CloudNodeManagerAzure string `json:"cloudNodeManagerAzure"` CloudControllerManagerOpenStack string `json:"cloudControllerManagerOpenStack"` } @@ -20,6 +22,7 @@ type imagesReference struct { type OperatorConfig struct { ManagedNamespace string ControllerImage string + CloudNodeImage string Platform configv1.PlatformType } @@ -53,6 +56,17 @@ func getCloudControllerManagerFromImages(platform configv1.PlatformType, images return images.CloudControllerManagerAWS case configv1.OpenStackPlatformType: return images.CloudControllerManagerOpenStack + case configv1.AzurePlatformType: + return images.CloudControllerManagerAzure + default: + return "" + } +} + +func getCloudNodeManagerFromImages(platform configv1.PlatformType, images imagesReference) string { + switch platform { + case configv1.AzurePlatformType: + return images.CloudNodeManagerAzure default: return "" } @@ -71,6 +85,7 @@ func ComposeConfig(platform configv1.PlatformType, imagesFile, managedNamespace } config.ControllerImage = getCloudControllerManagerFromImages(platform, images) + config.CloudNodeImage = getCloudNodeManagerFromImages(platform, images) return config, nil } diff --git a/pkg/substitution/substitution.go b/pkg/substitution/substitution.go index a899a8441..bf60507c9 100644 --- a/pkg/substitution/substitution.go +++ b/pkg/substitution/substitution.go @@ -11,6 +11,7 @@ const ( // Names in this list are unique and will be substituted with an image from config // cloudControllerManagerName is a name for default CCM controller container any provider may have cloudControllerManagerName = "cloud-controller-manager" + cloudNodeManagerName = "cloud-node-manager" ) // setDeploymentImages substitutes controller containers in Deployment with correct image @@ -20,11 +21,22 @@ func setDeploymentImages(config config.OperatorConfig, d *v1.Deployment) { continue } - klog.Infof("Substituting %q: %s", container.Name, config.ControllerImage) + klog.Infof("Substituting %q in %q with %s", container.Name, d.Kind, 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 + } + + klog.Infof("Substituting %q in %q with %s", container.Name, d.Kind, config.ControllerImage) + d.Spec.Template.Spec.Containers[i].Image = config.CloudNodeImage + } +} + func FillConfigValues(config config.OperatorConfig, templates []client.Object) []client.Object { objects := make([]client.Object, len(templates)) for i, objectTemplate := range templates { @@ -33,12 +45,12 @@ func FillConfigValues(config config.OperatorConfig, templates []client.Object) [ // Set namespaces for all object. Namespace on cluster-wide objects is stripped by API server and is not applied templateCopy.SetNamespace(config.ManagedNamespace) - dep, ok := templateCopy.(*v1.Deployment) - if ok { + switch dep := templateCopy.(type) { + case *v1.Deployment: setDeploymentImages(config, dep) - // TODO: add cloud-config calculated hash to annotations to account for redeployment on content change + case *v1.DaemonSet: + setDaemonSetImage(config, dep) } - objects[i] = templateCopy } return objects diff --git a/pkg/substitution/substitution_test.go b/pkg/substitution/substitution_test.go index 487054478..389b2b9f8 100644 --- a/pkg/substitution/substitution_test.go +++ b/pkg/substitution/substitution_test.go @@ -174,6 +174,66 @@ func TestFillConfigValues(t *testing.T) { ControllerImage: "correct_image:tag", ManagedNamespace: testManagementNamespace, }, + }, { + name: "Substitute image and namespace for more deployment and daemonset", + objects: []client.Object{&v1.DaemonSet{ + Spec: v1.DaemonSetSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: cloudNodeManagerName, + Image: "expect_change", + }}, + }, + }, + }, + }, &v1.Deployment{ + Spec: v1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: cloudControllerManagerName, + Image: "expect_change", + }}, + }, + }, + }, + }}, + expectedObjects: []client.Object{ + &v1.DaemonSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testManagementNamespace, + }, + Spec: v1.DaemonSetSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: cloudNodeManagerName, + Image: "correct_cloud_node_image:tag", + }}, + }, + }, + }, + }, &v1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testManagementNamespace, + }, + Spec: v1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: cloudControllerManagerName, + Image: "correct_image:tag", + }}, + }, + }, + }, + }}, + config: config.OperatorConfig{ + ControllerImage: "correct_image:tag", + CloudNodeImage: "correct_cloud_node_image:tag", + ManagedNamespace: testManagementNamespace, + }, }} for _, tc := range tc { From 98d2a83d72e4e27d07e842891fab46baeabb3035 Mon Sep 17 00:00:00 2001 From: Denis Moiseev Date: Tue, 25 May 2021 13:47:23 +0200 Subject: [PATCH 3/3] add missed tests --- pkg/config/config_test.go | 38 ++++++++++++++ pkg/substitution/substitution.go | 6 +-- pkg/substitution/substitution_test.go | 72 +++++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 3 deletions(-) diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 15ee2e0a0..83f180e3c 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -176,6 +176,44 @@ func TestGetProviderControllerFromImages(t *testing.T) { } } +func TestGetNodeControllerFromImages(t *testing.T) { + images := imagesReference{ + CloudControllerManagerAWS: "registry.ci.openshift.org/openshift:aws-cloud-controller-manager", + CloudControllerManagerAzure: "registry.ci.openshift.org/openshift:azure-cloud-controller-manager", + CloudNodeManagerAzure: "registry.ci.openshift.org/openshift:azure-cloud-node-manager", + CloudControllerManagerOpenStack: "registry.ci.openshift.org/openshift:openstack-cloud-controller-manager", + } + + tc := []struct { + name string + platformType configv1.PlatformType + expectedImage string + }{{ + name: "AWS platorm", + platformType: configv1.AWSPlatformType, + expectedImage: "", + }, { + name: "Azure platorm", + platformType: configv1.AzurePlatformType, + expectedImage: "registry.ci.openshift.org/openshift:azure-cloud-node-manager", + }, { + name: "OpenStack platorm", + platformType: configv1.OpenStackPlatformType, + expectedImage: "", + }, { + name: "Unknown platorm", + platformType: "unknown", + expectedImage: "", + }} + + for _, tc := range tc { + t.Run(tc.name, func(t *testing.T) { + image := getCloudNodeManagerFromImages(tc.platformType, images) + assert.Equal(t, tc.expectedImage, image) + }) + } +} + func TestComposeConfig(t *testing.T) { defaultManagementNamespace := "test-namespace" diff --git a/pkg/substitution/substitution.go b/pkg/substitution/substitution.go index bf60507c9..cba1b948e 100644 --- a/pkg/substitution/substitution.go +++ b/pkg/substitution/substitution.go @@ -45,11 +45,11 @@ func FillConfigValues(config config.OperatorConfig, templates []client.Object) [ // Set namespaces for all object. Namespace on cluster-wide objects is stripped by API server and is not applied templateCopy.SetNamespace(config.ManagedNamespace) - switch dep := templateCopy.(type) { + switch obj := templateCopy.(type) { case *v1.Deployment: - setDeploymentImages(config, dep) + setDeploymentImages(config, obj) case *v1.DaemonSet: - setDaemonSetImage(config, dep) + setDaemonSetImage(config, obj) } objects[i] = templateCopy } diff --git a/pkg/substitution/substitution_test.go b/pkg/substitution/substitution_test.go index 389b2b9f8..2bfd510c0 100644 --- a/pkg/substitution/substitution_test.go +++ b/pkg/substitution/substitution_test.go @@ -84,6 +84,78 @@ func TestSetDeploymentImages(t *testing.T) { } +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{ + Spec: corev1.PodSpec{ + Containers: tc.containers, + }, + }, + }, + } + + setDaemonSetImage(tc.config, ds) + + assert.EqualValues(t, ds.Spec.Template.Spec.Containers, tc.expectedContainers) + }) + } +} + func TestFillConfigValues(t *testing.T) { testManagementNamespace := "test-namespace"