bug: support container image SHA256 version

In OCP disconnected environment, the mirrored images
are not using tags but SHA256.

This change support boths format in the NCP except for DOCA-Driver.

Signed-off-by: Fred Rolland <[email protected]>

api/v1alpha1/nicclusterpolicy_types.go

@@ -51,7 +51,6 @@ type ImageSpec struct {
 	// +kubebuilder:validation:Pattern=[a-zA-Z0-9\.\-\/]+
 	Repository string `json:"repository"`
 	// Version of the image to use
-	// +kubebuilder:validation:Pattern=[a-zA-Z0-9\.-]+
 	Version string `json:"version"`
 	// ImagePullSecrets is an optional list of references to secrets in the same
 	// namespace to use for pulling the image

controllers/nicclusterpolicy_controller_test.go

@@ -305,7 +305,7 @@ var _ = Describe("NicClusterPolicyReconciler Controller", func() {
 		imageRepo := "nvcr.io/nvidia/doca"
 		imageName := "doca-telemetry-service"
 		imageVersion := "1.15.5-doca2.5.0"
-		updatedVersion := "v9.9.9-doca2.5.0"
+		updatedVersion := "sha256:1699d23027ea30c9fa"
 		ctx := context.Background()
 		It("should create, update and delete doca-telemetry-service through NICClusterPolicy", func() {
 			By("Create doca-telemetry-service through NICClusterPolicy")
@@ -355,7 +355,7 @@ var _ = Describe("NicClusterPolicyReconciler Controller", func() {
 			}, timeout*3, interval).Should(BeTrue())
 
 			By("Update DOCATelemetryService through NICClusterPolicy")
-			expectedImageName := fmt.Sprintf("%v/%v:%v", imageRepo, imageName, updatedVersion)
+			expectedImageName := fmt.Sprintf("%v/%v@%v", imageRepo, imageName, updatedVersion)
 
 			// Patch the NICClusterPolicy with the updated DOCATelemetryService version number.
 			patch := []byte(fmt.Sprintf(`{"spec": {"docaTelemetryService":{"version": %q}}}`, updatedVersion))

manifests/state-container-networking-plugins/0010-container-networking-plugins-ds.yml

@@ -51,7 +51,7 @@ spec:
           effect: NoSchedule
       containers:
         - name: cni-plugins
-          image: {{ .CrSpec.Repository }}/{{ .CrSpec.Image }}:{{ .CrSpec.Version }}
+          image: {{ imagePath .CrSpec.Repository .CrSpec.Image .CrSpec.Version }}
           imagePullPolicy: IfNotPresent
           securityContext:
             privileged: true

manifests/state-doca-telemetry-service/0010-doca-telemetry-service.yaml

@@ -42,7 +42,7 @@ spec:
           effect: NoSchedule
       containers:
       - name: doca-telemetry-service
-        image: {{ .CrSpec.Repository }}/{{ .CrSpec.Image }}:{{ .CrSpec.Version }}
+        image: {{ imagePath .CrSpec.Repository .CrSpec.Image .CrSpec.Version }}
         {{- with .RuntimeSpec.ContainerResources }}
         {{- with index . "doca-telemetry-service" }}
         resources:

manifests/state-ib-kubernetes/0070-deployment.yaml

@@ -67,7 +67,7 @@ spec:
           effect: NoSchedule
       containers:
         - name: ib-kubernetes
-          image: {{ .CrSpec.Repository }}/{{ .CrSpec.Image }}:{{ .CrSpec.Version }}
+          image: {{ imagePath .CrSpec.Repository .CrSpec.Image .CrSpec.Version }}
           imagePullPolicy: IfNotPresent
           command: ["/usr/bin/ib-kubernetes"]
           {{- with .RuntimeSpec.ContainerResources }}

manifests/state-ipoib-cni/0050-ipoib-cni-ds.yml

@@ -57,7 +57,7 @@ spec:
           effect: NoSchedule
       containers:
         - name: ipoib-cni
-          image: {{ .CrSpec.Repository }}/{{ .CrSpec.Image }}:{{ .CrSpec.Version }}
+          image: {{ imagePath .CrSpec.Repository .CrSpec.Image .CrSpec.Version }}
           {{- with .RuntimeSpec.ContainerResources }}
           {{- with index . "ipoib-cni" }}
           resources:

manifests/state-multus-cni/0050-multus-ds.yml

@@ -45,7 +45,7 @@ spec:
       terminationGracePeriodSeconds: 10        
       initContainers:
         - name: install-multus-binary
-          image: {{ .CrSpec.Repository }}/{{ .CrSpec.Image }}:{{ .CrSpec.Version }}
+          image: {{ imagePath .CrSpec.Repository .CrSpec.Image .CrSpec.Version }}
           command: ["/install_multus"]
           args:
             - "--type"
@@ -64,7 +64,7 @@ spec:
       {{- end }}
       containers:
         - name: kube-multus
-          image: {{ .CrSpec.Repository }}/{{ .CrSpec.Image }}:{{ .CrSpec.Version }}
+          image: {{ imagePath .CrSpec.Repository .CrSpec.Image .CrSpec.Version }}
           {{- if hasPrefix .CrSpec.Version "v4" }}
           command: ["/thin_entrypoint"]
           args:

manifests/state-nic-feature-discovery/030-nic-feature-discovery-ds.yaml

@@ -58,7 +58,7 @@ spec:
       {{- end }}
       containers:
         - name: nic-feature-discovery
-          image: {{ .CrSpec.Repository }}/{{ .CrSpec.Image }}:{{ .CrSpec.Version }}
+          image: {{ imagePath .CrSpec.Repository .CrSpec.Image .CrSpec.Version }}
           command: [ "/nic-feature-discovery" ]
           args:
             - --v=0

manifests/state-nv-ipam-cni/040-nv-ipam-controller.yaml

@@ -87,7 +87,7 @@ spec:
       {{- end }}
       containers:
         - name: nv-ipam-controller
-          image: {{ .CrSpec.Repository }}/{{ .CrSpec.Image }}:{{ .CrSpec.Version }}
+          image: {{ imagePath .CrSpec.Repository .CrSpec.Image .CrSpec.Version }}
           imagePullPolicy: IfNotPresent
           command: ["/ipam-controller"]
           args:

manifests/state-nv-ipam-cni/040-nv-ipam-node.yaml

@@ -56,7 +56,7 @@ spec:
       {{- end }}
       containers:
       - name: nv-ipam-node
-        image: {{ .CrSpec.Repository }}/{{ .CrSpec.Image }}:{{ .CrSpec.Version }}
+        image: {{ imagePath .CrSpec.Repository .CrSpec.Image .CrSpec.Version }}
         imagePullPolicy: IfNotPresent
         env:
         - name: NODE_NAME

manifests/state-rdma-shared-device-plugin/0060_rdma-shared-dev-plugin-ds.yaml

@@ -40,7 +40,7 @@ spec:
 {{if .DeployInitContainer}}
       initContainers:
         - name: ofed-driver-validation
-          image: {{ .CrSpec.Repository }}/{{ .CrSpec.Image }}:{{ .CrSpec.Version }}
+          image: 
           imagePullPolicy: IfNotPresent
           command: [ 'sh', '-c' ]
           args: [ "until lsmod | grep mlx5_core; do echo waiting for OFED drivers to be loaded; sleep 30; done" ]
@@ -52,7 +52,7 @@ spec:
       {{- end }}
       {{- end }}
       containers:
-      - image: {{ .CrSpec.Repository }}/{{ .CrSpec.Image }}:{{ .CrSpec.Version }}
+      - image: {{ imagePath .CrSpec.Repository .CrSpec.Image .CrSpec.Version }}
         name: rdma-shared-dp
         command: [ "/bin/k8s-rdma-shared-dp" ]
         {{- if .CrSpec.UseCdi }}

manifests/state-whereabouts-cni/0050-whereabouts-ds.yaml

@@ -54,7 +54,7 @@ spec:
           effect: NoSchedule
       containers:
       - name: whereabouts
-        image: {{ .CrSpec.Repository }}/{{ .CrSpec.Image }}:{{ .CrSpec.Version }}
+        image: {{ imagePath .CrSpec.Repository .CrSpec.Image .CrSpec.Version }}
         command: [ "/bin/sh" ]
         args:
         - -c

pkg/render/render.go

@@ -103,6 +103,14 @@ func nindentPrefix(spaces int, prefix, v string) string {
 	return strings.Replace(nindent(spaces, prefix+v), " ", "", len(prefix))
 }
 
+// imagePath returns contaimer image path, supporting sha256 format
+func imagePath(repository, image, version string) string {
+	if strings.HasPrefix(version, "sha256:") {
+		return repository + "/" + image + "@" + version
+	}
+	return repository + "/" + image + ":" + version
+}
+
 // renderFile renders a single file to a list of k8s unstructured objects
 func (r *textTemplateRenderer) renderFile(filePath string, data *TemplatingData) ([]*unstructured.Unstructured, error) {
 	// Read file
@@ -125,6 +133,7 @@ func (r *textTemplateRenderer) renderFile(filePath string, data *TemplatingData)
 		"nindent":       nindent,
 		"nindentPrefix": nindentPrefix,
 		"hasPrefix":     strings.HasPrefix,
+		"imagePath":     imagePath,
 	})
 
 	if data.Funcs != nil {

pkg/state/common_test.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"strings"
 
 	. "github.com/onsi/gomega"
 
@@ -51,6 +52,7 @@ const (
 	defaultTestRepository               = "myRepo"
 	defaultTestImage                    = "myImage"
 	defaultTestVersion                  = "myVersion"
+	defaultTestVersionSha256            = "sha256:1699d23027ea30c9fa"
 )
 
 var testLogger = log.Log.WithName("testLog")
@@ -97,6 +99,10 @@ func (t *testScope) New(fn clientBuilderFunc, dir string) testScope {
 	}
 }
 
+func getImagePathWithSha256() string {
+	return defaultTestRepository + "/" + defaultTestImage + "@" + defaultTestVersionSha256
+}
+
 func getTestCatalog() state.InfoCatalog {
 	return getTestCatalogForOpenshift(false)
 }
@@ -159,9 +165,13 @@ func getNADConfigIPAMJSON(ipam nadConfigIPAM) string {
 
 func assertCommonPodTemplateFields(template *corev1.PodTemplateSpec, image *mellanoxv1alpha1.ImageSpec) {
 	// Image name
-	Expect(template.Spec.Containers[0].Image).To(Equal(
-		fmt.Sprintf("%v/%v:%v", image.Repository, image.Image, image.Version)),
-	)
+	if strings.HasPrefix(image.Version, "sha256:") {
+		Expect(template.Spec.Containers[0].Image).To(Equal(getImagePathWithSha256()))
+	} else {
+		Expect(template.Spec.Containers[0].Image).To(Equal(
+			fmt.Sprintf("%v/%v:%v", image.Repository, image.Image, image.Version)),
+		)
+	}
 
 	// ImagePullSecrets
 	Expect(template.Spec.ImagePullSecrets).To(ConsistOf(

pkg/state/state_cni_plugins_test.go

@@ -59,6 +59,22 @@ var _ = Describe("CNI plugins state", func() {
 			Expect(ds.Spec).To(BeEquivalentTo(expectedDs.Spec))
 		})
 
+		It("should create Daemonset - SHA256 version", func() {
+			By("Sync")
+			cr := getMinimalNicClusterPolicyWithCNIPlugins()
+			cr.Spec.SecondaryNetwork.CniPlugins.Version = "sha256:1699d23027ea30c9fa"
+			status, err := ts.state.Sync(context.Background(), cr, ts.catalog)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(status).To(BeEquivalentTo(state.SyncStateNotReady))
+			By("Verify DaemonSet")
+			ds := &appsv1.DaemonSet{}
+			err = ts.client.Get(context.Background(), types.NamespacedName{Namespace: ts.namespace, Name: "cni-plugins-ds"}, ds)
+			Expect(err).NotTo(HaveOccurred())
+			expectedDs := getExpectedMinimalCniPluginDS()
+			expectedDs.Spec.Template.Spec.Containers[0].Image = "myrepo/myimage@sha256:1699d23027ea30c9fa"
+			Expect(ds.Spec).To(BeEquivalentTo(expectedDs.Spec))
+		})
+
 		It("should create Daemonset with ImagePullSecrets when specified in CR", func() {
 			By("Sync")
 			cr := getMinimalNicClusterPolicyWithCNIPlugins()

pkg/state/state_doca_telemetry_service_test.go

@@ -103,4 +103,22 @@ var _ = Describe("DOCATelemetryService Controller", func() {
 		expectedKinds := []string{"ServiceAccount", "DaemonSet", "ConfigMap", "Role", "RoleBinding"}
 		assertUnstructuredListHasExactKinds(got, expectedKinds...)
 	})
+	It("should use SHA256 format", func() {
+		withSha256 := cr.DeepCopy()
+		withSha256.Spec.DOCATelemetryService.Version = defaultTestVersionSha256
+		got, err := s.GetManifestObjects(ctx, withSha256, getTestCatalog(), log.FromContext(ctx))
+		Expect(err).ToNot(HaveOccurred())
+		expectedKinds := []string{"ServiceAccount", "DaemonSet", "ConfigMap"}
+		assertUnstructuredListHasExactKinds(got, expectedKinds...)
+
+		for _, obj := range got {
+			// The image should be in SHA256 format
+			if obj.GetKind() == "DaemonSet" {
+				ds := &appsv1.DaemonSet{}
+				Expect(runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), ds)).To(Succeed())
+				Expect(ds.Spec.Template.Spec.Containers[0].Image).To(Equal(getImagePathWithSha256()))
+			}
+
+		}
+	})
 })

pkg/state/state_ib_kubernetes_test.go

@@ -241,5 +241,42 @@ var _ = Describe("IB Kubernetes state rendering tests", func() {
 			limits := resources["limits"]
 			Expect(limits).To(BeNil())
 		})
+		It("Should have image in SHA256 format", func() {
+			manifestBaseDir := "../../manifests/state-ib-kubernetes"
+
+			files, err := utils.GetFilesWithSuffix(manifestBaseDir, render.ManifestFileSuffix...)
+			Expect(err).NotTo(HaveOccurred())
+			renderer := render.NewRenderer(files)
+
+			ibKubernetesState := stateIBKubernetes{
+				stateSkel: stateSkel{
+					renderer: renderer,
+				},
+			}
+
+			Expect(err).NotTo(HaveOccurred())
+
+			ibKubernetesSpec := &mellanoxv1alpha1.IBKubernetesSpec{}
+			ibKubernetesSpec.Image = "myImage"
+			ibKubernetesSpec.ImagePullSecrets = []string{}
+			ibKubernetesSpec.Version = "sha256:1699d23027ea30c9fa"
+			ibKubernetesSpec.Repository = "myRepo"
+			cr := &mellanoxv1alpha1.NicClusterPolicy{}
+			cr.Spec.IBKubernetes = ibKubernetesSpec
+
+			catalog := NewInfoCatalog()
+			catalog.Add(InfoTypeClusterType, &dummyProvider{})
+
+			objs, err := ibKubernetesState.GetManifestObjects(context.TODO(), cr, catalog, testLogger)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(len(objs)).To(Equal(4))
+			ds := objs[3]
+			spec := ds.Object["spec"].(map[string]interface{})
+			template := spec["template"].(map[string]interface{})
+			templateSpec := template["spec"].(map[string]interface{})
+			containers := templateSpec["containers"].([]interface{})
+			image := containers[0].(map[string]interface{})["image"]
+			Expect(image).To(Equal("myRepo/myImage@sha256:1699d23027ea30c9fa"))
+		})
 	})
 })

pkg/state/state_ipoib_cni_test.go

@@ -46,13 +46,22 @@ var _ = Describe("IPoIB CNI State tests", func() {
 			Expect(len(objs)).To(Equal(1))
 			GetManifestObjectsTest(ts.context, cr, ts.catalog, cr.Spec.SecondaryNetwork.IPoIB, ts.renderer)
 		})
+		It("manifests with IPoIB CNI - image as SHA256", func() {
+			cr := getNICForIPoIBCNI()
+			cr.Spec.SecondaryNetwork.IPoIB.Version = defaultTestVersionSha256
+			objs, err := ts.renderer.GetManifestObjects(context.TODO(), cr, ts.catalog, testLogger)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(len(objs)).To(Equal(1))
+			GetManifestObjectsTest(ts.context, cr, ts.catalog, cr.Spec.SecondaryNetwork.IPoIB, ts.renderer)
+		})
 		It("manifests with IPoIB CNI for Openshift", func() {
 			cr := getNICForIPoIBCNI()
 			objs, err := ts.renderer.GetManifestObjects(context.TODO(), cr, ts.openshiftCatalog, testLogger)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(len(objs)).To(Equal(4))
 			GetManifestObjectsTest(ts.context, cr, ts.catalog, cr.Spec.SecondaryNetwork.IPoIB, ts.renderer)
 		})
+
 	})
 
 	Context("should sync", func() {

pkg/state/state_multus_cni_test.go

@@ -113,6 +113,22 @@ var _ = Describe("Multus CNI state", func() {
 		})).To(BeTrue())
 	})
 
+	It("should render Daemonset image with SHA256 format", func() {
+		cr := getMinimalNicClusterPolicyWithMultus()
+		cr.Spec.SecondaryNetwork.Multus.Version = "sha256:1699d23027ea30c9fa"
+
+		objs, err := ts.renderer.GetManifestObjects(context.TODO(), cr, ts.catalog, testLogger)
+		Expect(err).NotTo(HaveOccurred())
+
+		Expect(runFuncForObjectInSlice(objs, "DaemonSet", func(obj *unstructured.Unstructured) {
+			var daemonSet appsv1.DaemonSet
+			err = runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), &daemonSet)
+			Expect(err).NotTo(HaveOccurred())
+
+			Expect(daemonSet.Spec.Template.Spec.Containers[0].Image).To(Equal("myrepo/myimage@sha256:1699d23027ea30c9fa"))
+		})).To(BeTrue())
+	})
+
 	It("should render Daemonset with NodeAffinity when specified in CR", func() {
 		cr := getMinimalNicClusterPolicyWithMultus()
 

pkg/state/state_nic_feature_discovery_test.go

@@ -41,4 +41,11 @@ var _ = Describe("NicClusterPolicyReconciler Controller", func() {
 	It("should test fields are set correctly", func() {
 		GetManifestObjectsTest(ctx, cr, getTestCatalog(), imageSpec, s)
 	})
+	It("should use SHA256 format", func() {
+		withSha256 := cr.DeepCopy()
+		withSha256.Spec.NicFeatureDiscovery.Version = defaultTestVersionSha256
+		imageSpecWithSha256 := imageSpec.DeepCopy()
+		imageSpecWithSha256.Version = defaultTestVersionSha256
+		GetManifestObjectsTest(ctx, withSha256, getTestCatalog(), imageSpecWithSha256, s)
+	})
 })

pkg/state/state_nv_ipam_cni_test.go

@@ -95,6 +95,34 @@ var _ = Describe("NVIPAM Controller", func() {
 			assertCommonDeploymentFields(d, &cr.Spec.NvIpam.ImageSpec)
 		})
 
+		It("should create Daemonset - SHA256 image format", func() {
+			By("Sync")
+			cr := getMinimalNicClusterPolicyWithNVIpam("nv-ipam-node")
+			cr.Spec.NvIpam.Version = defaultTestVersionSha256
+			status, err := nvIpamState.Sync(context.Background(), cr, catalog)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(status).To(BeEquivalentTo(state.SyncStateNotReady))
+			By("Verify DaemonSet")
+			ds := &appsv1.DaemonSet{}
+			err = client.Get(context.Background(), types.NamespacedName{Namespace: namespace, Name: "nv-ipam-node"}, ds)
+			Expect(err).NotTo(HaveOccurred())
+			assertCommonDaemonSetFields(ds, &cr.Spec.NvIpam.ImageSpec, cr)
+		})
+
+		It("should create Deployment - SHA256 image format", func() {
+			By("Sync")
+			cr := getMinimalNicClusterPolicyWithNVIpam("nv-ipam-controller")
+			cr.Spec.NvIpam.Version = defaultTestVersionSha256
+			status, err := nvIpamState.Sync(context.Background(), cr, catalog)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(status).To(BeEquivalentTo(state.SyncStateNotReady))
+			By("Verify Deployment")
+			d := &appsv1.Deployment{}
+			err = client.Get(context.Background(), types.NamespacedName{Namespace: namespace, Name: "nv-ipam-controller"}, d)
+			Expect(err).NotTo(HaveOccurred())
+			assertCommonDeploymentFields(d, &cr.Spec.NvIpam.ImageSpec)
+		})
+
 		It("should create Deployment with Webhook when specified in CR", func() {
 			By("Sync")
 			cr := getMinimalNicClusterPolicyWithNVIpam("nv-ipam-controller")

pkg/state/state_rdma_shared_device_plugin_test.go

@@ -42,6 +42,14 @@ var _ = Describe("RDMA Shared Device Plugin", func() {
 			Expect(len(objs)).To(Equal(1))
 			GetManifestObjectsTest(ts.context, cr, ts.catalog, &cr.Spec.RdmaSharedDevicePlugin.ImageSpec, ts.renderer)
 		})
+		It("manifests with RdmaSharedDevicePlugin - image as SHA256", func() {
+			cr := getRDMASharedDevicePlugin()
+			cr.Spec.RdmaSharedDevicePlugin.Version = defaultTestVersionSha256
+			objs, err := ts.renderer.GetManifestObjects(ts.context, cr, ts.catalog, testLogger)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(len(objs)).To(Equal(1))
+			GetManifestObjectsTest(ts.context, cr, ts.catalog, &cr.Spec.RdmaSharedDevicePlugin.ImageSpec, ts.renderer)
+		})
 		It("Openshift manifests", func() {
 			cr := getRDMASharedDevicePlugin()
 			objs, err := ts.renderer.GetManifestObjects(ts.context, cr, ts.openshiftCatalog, testLogger)