Add image override logic for DaemonSet

pkg/reconciler/knativeserving/common/certs_test.go

@@ -84,7 +84,7 @@ var customCertsTests = []customCertsTest{
 }
 
 func TestOnlyTransformCustomCertsForController(t *testing.T) {
-	before := makeDeployment(t, "not-controller", v1.PodSpec{
+	before := makeDeployment("not-controller", v1.PodSpec{
 		Containers: []v1.Container{{
 			Name: "definitely-not-controller",
 		}},
@@ -116,7 +116,7 @@ func TestCustomCertsTransform(t *testing.T) {
 }
 
 func runCustomCertsTransformTest(t *testing.T, tt *customCertsTest) {
-	unstructured := makeUnstructured(t, makeDeployment(t, "controller", v1.PodSpec{
+	unstructured := makeUnstructured(t, makeDeployment("controller", v1.PodSpec{
 		Containers: []v1.Container{{
 			Name: "controller",
 		}},

pkg/reconciler/knativeserving/common/extensions.go

@@ -32,7 +32,6 @@ func (platforms Platforms) Transformers(kubeClientSet kubernetes.Interface, inst
 		mf.InjectOwner(instance),
 		mf.InjectNamespace(instance.GetNamespace()),
 		ConfigMapTransform(instance, log),
-		DeploymentTransform(instance, log),
 		ImageTransform(instance, log),
 		GatewayTransform(instance, log),
 		CustomCertsTransform(instance, log),

pkg/reconciler/knativeserving/common/images.go

@@ -38,42 +38,51 @@ var (
 	containerNameVariable = "${NAME}"
 )
 
-func DeploymentTransform(instance *servingv1alpha1.KnativeServing, log *zap.SugaredLogger) mf.Transformer {
-	return func(u *unstructured.Unstructured) error {
-		// Update the deployment with the new registry and tag
-		if u.GetKind() == "Deployment" {
-			return updateDeployment(instance, u, log)
-		}
-		return nil
-	}
-}
-
+// ImageTransform updates image with a new registry and tag
 func ImageTransform(instance *servingv1alpha1.KnativeServing, log *zap.SugaredLogger) mf.Transformer {
 	return func(u *unstructured.Unstructured) error {
-		// Update the image with the new registry and tag
-		if u.GetAPIVersion() == "caching.internal.knative.dev/v1alpha1" && u.GetKind() == "Image" {
-			return updateCachingImage(instance, u)
+		switch u.GetKind() {
+		// TODO need to use PodSpecable duck type in order to remove duplicates of deployment, daemonSet
+		case "Deployment":
+			return updateDeployment(instance, u, log)
+		case "DaemonSet":
+			return updateDaemonSet(instance, u, log)
+		case "Image":
+			if u.GetAPIVersion() == "caching.internal.knative.dev/v1alpha1" {
+				return updateCachingImage(instance, u)
+			}
 		}
 		return nil
 	}
 }
 
 func updateDeployment(instance *servingv1alpha1.KnativeServing, u *unstructured.Unstructured, log *zap.SugaredLogger) error {
 	var deployment = &appsv1.Deployment{}
-	err := scheme.Scheme.Convert(u, deployment, nil)
-	if err != nil {
+	if err := scheme.Scheme.Convert(u, deployment, nil); err != nil {
 		log.Error(err, "Error converting Unstructured to Deployment", "unstructured", u, "deployment", deployment)
 		return err
 	}
 
-	registry := instance.Spec.Registry
-	log.Debugw("Updating Deployment", "name", u.GetName(), "registry", registry)
+	updateRegistry(&deployment.Spec.Template.Spec, instance, log, deployment.GetName())
+	if err := scheme.Scheme.Convert(deployment, u, nil); err != nil {
+		return err
+	}
+	// The zero-value timestamp defaulted by the conversion causes
+	// superfluous updates
+	u.SetCreationTimestamp(metav1.Time{})
+
+	log.Debugw("Finished conversion", "name", u.GetName(), "unstructured", u.Object)
+	return nil
+}
 
-	updateDeploymentImage(deployment, &registry, log)
-	deployment.Spec.Template.Spec.ImagePullSecrets = addImagePullSecrets(
-		deployment.Spec.Template.Spec.ImagePullSecrets, &registry, log)
-	err = scheme.Scheme.Convert(deployment, u, nil)
-	if err != nil {
+func updateDaemonSet(instance *servingv1alpha1.KnativeServing, u *unstructured.Unstructured, log *zap.SugaredLogger) error {
+	var daemonSet = &appsv1.DaemonSet{}
+	if err := scheme.Scheme.Convert(u, daemonSet, nil); err != nil {
+		log.Error(err, "Error converting Unstructured to daemonSet", "unstructured", u, "daemonSet", daemonSet)
+		return err
+	}
+	updateRegistry(&daemonSet.Spec.Template.Spec, instance, log, daemonSet.GetName())
+	if err := scheme.Scheme.Convert(daemonSet, u, nil); err != nil {
 		return err
 	}
 	// The zero-value timestamp defaulted by the conversion causes
@@ -84,23 +93,31 @@ func updateDeployment(instance *servingv1alpha1.KnativeServing, u *unstructured.
 	return nil
 }
 
-// updateDeploymentImage updates the image of the deployment with a new registry and tag
-func updateDeploymentImage(deployment *appsv1.Deployment, registry *servingv1alpha1.Registry, log *zap.SugaredLogger) {
-	containers := deployment.Spec.Template.Spec.Containers
+func updateRegistry(spec *corev1.PodSpec, instance *servingv1alpha1.KnativeServing, log *zap.SugaredLogger, name string) {
+	registry := instance.Spec.Registry
+	log.Debugw("Updating ", "name", name, "registry", registry)
+
+	updateImage(spec, &registry, log, name)
+	spec.ImagePullSecrets = addImagePullSecrets(
+		spec.ImagePullSecrets, &registry, log)
+}
+
+// updateImage updates the image with a new registry and tag
+func updateImage(spec *corev1.PodSpec, registry *servingv1alpha1.Registry, log *zap.SugaredLogger, name string) {
+	containers := spec.Containers
 	for index := range containers {
 		container := &containers[index]
 		newImage := getNewImage(registry, container.Name)
 		if newImage != "" {
 			updateContainer(container, newImage, log)
 		}
 	}
-	log.Debugw("Finished updating images", "name", deployment.GetName(), "containers", deployment.Spec.Template.Spec.Containers)
+	log.Debugw("Finished updating images", "name", name, "containers", spec.Containers)
 }
 
 func updateCachingImage(instance *servingv1alpha1.KnativeServing, u *unstructured.Unstructured) error {
 	var image = &caching.Image{}
-	err := scheme.Scheme.Convert(u, image, nil)
-	if err != nil {
+	if err := scheme.Scheme.Convert(u, image, nil); err != nil {
 		log.Error(err, "Error converting Unstructured to Image", "unstructured", u, "image", image)
 		return err
 	}
@@ -109,8 +126,7 @@ func updateCachingImage(instance *servingv1alpha1.KnativeServing, u *unstructure
 	log.Debugw("Updating Image", "name", u.GetName(), "registry", registry)
 
 	updateImageSpec(image, &registry, log)
-	err = scheme.Scheme.Convert(image, u, nil)
-	if err != nil {
+	if err := scheme.Scheme.Convert(image, u, nil); err != nil {
 		return err
 	}
 	// Cleanup zero-value default to prevent superfluous updates
@@ -123,8 +139,7 @@ func updateCachingImage(instance *servingv1alpha1.KnativeServing, u *unstructure
 
 // updateImageSpec updates the image of a with a new registry and tag
 func updateImageSpec(image *caching.Image, registry *servingv1alpha1.Registry, log *zap.SugaredLogger) {
-	newImage := getNewImage(registry, image.Name)
-	if newImage != "" {
+	if newImage := getNewImage(registry, image.Name); newImage != "" {
 		log.Debugf("Updating image from: %v, to: %v", image.Spec.Image, newImage)
 		image.Spec.Image = newImage
 	}
@@ -133,8 +148,7 @@ func updateImageSpec(image *caching.Image, registry *servingv1alpha1.Registry, l
 }
 
 func getNewImage(registry *servingv1alpha1.Registry, containerName string) string {
-	overrideImage := registry.Override[containerName]
-	if overrideImage != "" {
+	if overrideImage := registry.Override[containerName]; overrideImage != "" {
 		return overrideImage
 	}
 	return replaceName(registry.Default, containerName)

pkg/reconciler/knativeserving/common/images_test.go

@@ -30,14 +30,14 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 )
 
-type updateDeploymentImageTest struct {
+type updateImageTest struct {
 	name       string
 	containers []corev1.Container
 	registry   servingv1alpha1.Registry
 	expected   []string
 }
 
-var updateDeploymentImageTests = []updateDeploymentImageTest{
+var updateImageTests = []updateImageTest{
 	{
 		name: "UsesNameFromDefault",
 		containers: []corev1.Container{{
@@ -110,26 +110,39 @@ var updateDeploymentImageTests = []updateDeploymentImageTest{
 	},
 }
 
-func TestDeploymentTransform(t *testing.T) {
-	for _, tt := range updateDeploymentImageTests {
+func TestResourceTransform(t *testing.T) {
+	for _, tt := range updateImageTests {
 		t.Run(tt.name, func(t *testing.T) {
-			runDeploymentTransformTest(t, &tt)
+			runResourceTransformTest(t, &tt)
 		})
 	}
 }
-func runDeploymentTransformTest(t *testing.T, tt *updateDeploymentImageTest) {
-	unstructuredDeployment := makeUnstructured(t, makeDeployment(t, tt.name, corev1.PodSpec{Containers: tt.containers}))
+
+func runResourceTransformTest(t *testing.T, tt *updateImageTest) {
+	// test for deployment
+	unstructuredDeployment := makeUnstructured(t, makeDeployment(tt.name, corev1.PodSpec{Containers: tt.containers}))
 	instance := &servingv1alpha1.KnativeServing{
 		Spec: servingv1alpha1.KnativeServingSpec{
 			Registry: tt.registry,
 		},
 	}
-	deploymentTransform := DeploymentTransform(instance, log)
+	deploymentTransform := ImageTransform(instance, log)
 	deploymentTransform(&unstructuredDeployment)
 	validateUnstructedDeploymentChanged(t, tt, &unstructuredDeployment)
+
+	// test for daemonSet
+	unstructuredDaemonSet := makeUnstructured(t, makeDaemonSet(tt.name, corev1.PodSpec{Containers: tt.containers}))
+	instance = &servingv1alpha1.KnativeServing{
+		Spec: servingv1alpha1.KnativeServingSpec{
+			Registry: tt.registry,
+		},
+	}
+	daemonSetTransform := ImageTransform(instance, log)
+	daemonSetTransform(&unstructuredDaemonSet)
+	validateUnstructedDaemonSetChanged(t, tt, &unstructuredDaemonSet)
 }
 
-func validateUnstructedDeploymentChanged(t *testing.T, tt *updateDeploymentImageTest, u *unstructured.Unstructured) {
+func validateUnstructedDeploymentChanged(t *testing.T, tt *updateImageTest, u *unstructured.Unstructured) {
 	var deployment = &appsv1.Deployment{}
 	err := scheme.Scheme.Convert(u, deployment, nil)
 	assertEqual(t, err, nil)
@@ -138,7 +151,16 @@ func validateUnstructedDeploymentChanged(t *testing.T, tt *updateDeploymentImage
 	}
 }
 
-func makeDeployment(t *testing.T, name string, podSpec corev1.PodSpec) *appsv1.Deployment {
+func validateUnstructedDaemonSetChanged(t *testing.T, tt *updateImageTest, u *unstructured.Unstructured) {
+	var daemonSet = &appsv1.DaemonSet{}
+	err := scheme.Scheme.Convert(u, daemonSet, nil)
+	assertEqual(t, err, nil)
+	for i, expected := range tt.expected {
+		assertEqual(t, daemonSet.Spec.Template.Spec.Containers[i].Image, expected)
+	}
+}
+
+func makeDeployment(name string, podSpec corev1.PodSpec) *appsv1.Deployment {
 	return &appsv1.Deployment{
 		TypeMeta: metav1.TypeMeta{
 			Kind: "Deployment",
@@ -154,6 +176,22 @@ func makeDeployment(t *testing.T, name string, podSpec corev1.PodSpec) *appsv1.D
 	}
 }
 
+func makeDaemonSet(name string, podSpec corev1.PodSpec) *appsv1.DaemonSet {
+	return &appsv1.DaemonSet{
+		TypeMeta: metav1.TypeMeta{
+			Kind: "DaemonSet",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name: name,
+		},
+		Spec: appsv1.DaemonSetSpec{
+			Template: corev1.PodTemplateSpec{
+				Spec: podSpec,
+			},
+		},
+	}
+}
+
 func makeUnstructured(t *testing.T, obj interface{}) unstructured.Unstructured {
 	var result = unstructured.Unstructured{}
 	err := scheme.Scheme.Convert(obj, &result, nil)
@@ -299,20 +337,35 @@ func TestImagePullSecrets(t *testing.T) {
 }
 
 func runImagePullSecretsTest(t *testing.T, tt *addImagePullSecretsTest) {
-	unstructuredDeployment := makeUnstructured(t, makeDeployment(t, tt.name, corev1.PodSpec{ImagePullSecrets: tt.existingSecrets}))
+	unstructuredDeployment := makeUnstructured(t, makeDeployment(tt.name, corev1.PodSpec{ImagePullSecrets: tt.existingSecrets}))
 	instance := &servingv1alpha1.KnativeServing{
 		Spec: servingv1alpha1.KnativeServingSpec{
 			Registry: tt.registry,
 		},
 	}
-	deploymentTransform := DeploymentTransform(instance, log)
+	deploymentTransform := ImageTransform(instance, log)
 	deploymentTransform(&unstructuredDeployment)
 
 	var deployment = &appsv1.Deployment{}
 	err := scheme.Scheme.Convert(&unstructuredDeployment, deployment, nil)
 
 	assertEqual(t, err, nil)
 	assertDeepEqual(t, deployment.Spec.Template.Spec.ImagePullSecrets, tt.expectedSecrets)
+
+	unstructuredDaemonSet := makeUnstructured(t, makeDaemonSet(tt.name, corev1.PodSpec{ImagePullSecrets: tt.existingSecrets}))
+	daemonSetinstance := &servingv1alpha1.KnativeServing{
+		Spec: servingv1alpha1.KnativeServingSpec{
+			Registry: tt.registry,
+		},
+	}
+	daemonSetTransform := ImageTransform(daemonSetinstance, log)
+	daemonSetTransform(&unstructuredDaemonSet)
+
+	var daemonSet = &appsv1.DaemonSet{}
+	err = scheme.Scheme.Convert(&unstructuredDaemonSet, daemonSet, nil)
+
+	assertEqual(t, err, nil)
+	assertDeepEqual(t, daemonSet.Spec.Template.Spec.ImagePullSecrets, tt.expectedSecrets)
 }
 
 func assertEqual(t *testing.T, actual, expected interface{}) {