Skip to content

Commit

Permalink
bug: support container image SHA256 version
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
rollandf committed Jan 8, 2025
1 parent 4cb0d8a commit a981903
Show file tree
Hide file tree
Showing 26 changed files with 194 additions and 42 deletions.
1 change: 0 additions & 1 deletion api/v1alpha1/nicclusterpolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 0 additions & 11 deletions config/crd/bases/mellanox.com_nicclusterpolicies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ spec:
type: string
version:
description: Version of the image to use
pattern: '[a-zA-Z0-9\.-]+'
type: string
required:
- image
Expand Down Expand Up @@ -208,7 +207,6 @@ spec:
type: string
version:
description: Version of the image to use
pattern: '[a-zA-Z0-9\.-]+'
type: string
required:
- image
Expand Down Expand Up @@ -280,7 +278,6 @@ spec:
type: string
version:
description: Version of the image to use
pattern: '[a-zA-Z0-9\.-]+'
type: string
required:
- image
Expand Down Expand Up @@ -552,7 +549,6 @@ spec:
type: string
version:
description: Version of the image to use
pattern: '[a-zA-Z0-9\.-]+'
type: string
required:
- image
Expand Down Expand Up @@ -887,7 +883,6 @@ spec:
type: object
version:
description: Version of the image to use
pattern: '[a-zA-Z0-9\.-]+'
type: string
required:
- image
Expand Down Expand Up @@ -965,7 +960,6 @@ spec:
type: boolean
version:
description: Version of the image to use
pattern: '[a-zA-Z0-9\.-]+'
type: string
required:
- image
Expand Down Expand Up @@ -1041,7 +1035,6 @@ spec:
type: string
version:
description: Version of the image to use
pattern: '[a-zA-Z0-9\.-]+'
type: string
required:
- image
Expand Down Expand Up @@ -1108,7 +1101,6 @@ spec:
type: string
version:
description: Version of the image to use
pattern: '[a-zA-Z0-9\.-]+'
type: string
required:
- image
Expand Down Expand Up @@ -1175,7 +1167,6 @@ spec:
type: string
version:
description: Version of the image to use
pattern: '[a-zA-Z0-9\.-]+'
type: string
required:
- image
Expand Down Expand Up @@ -1245,7 +1236,6 @@ spec:
type: string
version:
description: Version of the image to use
pattern: '[a-zA-Z0-9\.-]+'
type: string
required:
- image
Expand Down Expand Up @@ -1324,7 +1314,6 @@ spec:
type: boolean
version:
description: Version of the image to use
pattern: '[a-zA-Z0-9\.-]+'
type: string
required:
- image
Expand Down
4 changes: 2 additions & 2 deletions controllers/nicclusterpolicy_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ spec:
type: string
version:
description: Version of the image to use
pattern: '[a-zA-Z0-9\.-]+'
type: string
required:
- image
Expand Down Expand Up @@ -208,7 +207,6 @@ spec:
type: string
version:
description: Version of the image to use
pattern: '[a-zA-Z0-9\.-]+'
type: string
required:
- image
Expand Down Expand Up @@ -280,7 +278,6 @@ spec:
type: string
version:
description: Version of the image to use
pattern: '[a-zA-Z0-9\.-]+'
type: string
required:
- image
Expand Down Expand Up @@ -552,7 +549,6 @@ spec:
type: string
version:
description: Version of the image to use
pattern: '[a-zA-Z0-9\.-]+'
type: string
required:
- image
Expand Down Expand Up @@ -887,7 +883,6 @@ spec:
type: object
version:
description: Version of the image to use
pattern: '[a-zA-Z0-9\.-]+'
type: string
required:
- image
Expand Down Expand Up @@ -965,7 +960,6 @@ spec:
type: boolean
version:
description: Version of the image to use
pattern: '[a-zA-Z0-9\.-]+'
type: string
required:
- image
Expand Down Expand Up @@ -1041,7 +1035,6 @@ spec:
type: string
version:
description: Version of the image to use
pattern: '[a-zA-Z0-9\.-]+'
type: string
required:
- image
Expand Down Expand Up @@ -1108,7 +1101,6 @@ spec:
type: string
version:
description: Version of the image to use
pattern: '[a-zA-Z0-9\.-]+'
type: string
required:
- image
Expand Down Expand Up @@ -1175,7 +1167,6 @@ spec:
type: string
version:
description: Version of the image to use
pattern: '[a-zA-Z0-9\.-]+'
type: string
required:
- image
Expand Down Expand Up @@ -1245,7 +1236,6 @@ spec:
type: string
version:
description: Version of the image to use
pattern: '[a-zA-Z0-9\.-]+'
type: string
required:
- image
Expand Down Expand Up @@ -1324,7 +1314,6 @@ spec:
type: boolean
version:
description: Version of the image to use
pattern: '[a-zA-Z0-9\.-]+'
type: string
required:
- image
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion manifests/state-ib-kubernetes/0070-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
2 changes: 1 addition & 1 deletion manifests/state-ipoib-cni/0050-ipoib-cni-ds.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions manifests/state-multus-cni/0050-multus-ds.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion manifests/state-nv-ipam-cni/040-nv-ipam-controller.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion manifests/state-nv-ipam-cni/040-nv-ipam-node.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ spec:
{{if .DeployInitContainer}}
initContainers:
- name: ofed-driver-validation
image: {{ .CrSpec.Repository }}/{{ .CrSpec.Image }}:{{ .CrSpec.Version }}
image: {{ imagePath .CrSpec.Repository .CrSpec.Image .CrSpec.Version }}
imagePullPolicy: IfNotPresent
command: [ 'sh', '-c' ]
args: [ "until lsmod | grep mlx5_core; do echo waiting for OFED drivers to be loaded; sleep 30; done" ]
Expand All @@ -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 }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,14 @@ spec:
{{- if .DeployInitContainer}}
initContainers:
- name: ofed-driver-validation
image: {{ .CrSpec.ImageSpec.Repository }}/{{ .CrSpec.ImageSpec.Image }}:{{ .CrSpec.ImageSpec.Version }}
image: {{ imagePath .CrSpec.ImageSpec.Repository .CrSpec.ImageSpec.Image .CrSpec.ImageSpec.Version }}
imagePullPolicy: IfNotPresent
command: ['sh', '-c']
args: ["until lsmod | grep mlx5_core; do echo waiting for OFED drivers to be loaded; sleep 30; done"]
{{- end}}
containers:
- name: kube-sriovdp
image: {{ .CrSpec.ImageSpec.Repository }}/{{ .CrSpec.ImageSpec.Image }}:{{ .CrSpec.ImageSpec.Version }}
image: {{ imagePath .CrSpec.ImageSpec.Repository .CrSpec.ImageSpec.Image .CrSpec.ImageSpec.Version }}
imagePullPolicy: IfNotPresent
args:
- --log-dir=sriovdp
Expand Down
2 changes: 1 addition & 1 deletion manifests/state-whereabouts-cni/0050-whereabouts-ds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions pkg/render/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
16 changes: 13 additions & 3 deletions pkg/state/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"encoding/json"
"fmt"
"strings"

. "github.com/onsi/gomega"

Expand Down Expand Up @@ -51,6 +52,7 @@ const (
defaultTestRepository = "myRepo"
defaultTestImage = "myImage"
defaultTestVersion = "myVersion"
defaultTestVersionSha256 = "sha256:1699d23027ea30c9fa"
)

var testLogger = log.Log.WithName("testLog")
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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(
Expand Down
Loading

0 comments on commit a981903

Please sign in to comment.