diff --git a/pkg/reconciler/knativeserving/common/certs_test.go b/pkg/reconciler/knativeserving/common/certs_test.go index 3214d7ec..a3a2ae69 100644 --- a/pkg/reconciler/knativeserving/common/certs_test.go +++ b/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", }}, diff --git a/pkg/reconciler/knativeserving/common/extensions.go b/pkg/reconciler/knativeserving/common/extensions.go index 50bac1fa..2bf97d18 100644 --- a/pkg/reconciler/knativeserving/common/extensions.go +++ b/pkg/reconciler/knativeserving/common/extensions.go @@ -32,7 +32,7 @@ func (platforms Platforms) Transformers(kubeClientSet kubernetes.Interface, inst mf.InjectOwner(instance), mf.InjectNamespace(instance.GetNamespace()), ConfigMapTransform(instance, log), - DeploymentTransform(instance, log), + ResourceTransform(instance, log), ImageTransform(instance, log), GatewayTransform(instance, log), CustomCertsTransform(instance, log), diff --git a/pkg/reconciler/knativeserving/common/images.go b/pkg/reconciler/knativeserving/common/images.go index 14e50973..6060e020 100644 --- a/pkg/reconciler/knativeserving/common/images.go +++ b/pkg/reconciler/knativeserving/common/images.go @@ -38,12 +38,17 @@ var ( containerNameVariable = "${NAME}" ) -func DeploymentTransform(instance *servingv1alpha1.KnativeServing, log *zap.SugaredLogger) mf.Transformer { +// ResourceTransform updates deployment and daemonSet with a new registry and tag +func ResourceTransform(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) } + // Update the daemonSet with the new registry and tag + if u.GetKind() == "DaemonSet" { + return updateDaemonSet(instance, u, log) + } return nil } } @@ -60,8 +65,7 @@ func ImageTransform(instance *servingv1alpha1.KnativeServing, log *zap.SugaredLo 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 } @@ -69,11 +73,34 @@ func updateDeployment(instance *servingv1alpha1.KnativeServing, u *unstructured. registry := instance.Spec.Registry log.Debugw("Updating Deployment", "name", u.GetName(), "registry", registry) - updateDeploymentImage(deployment, ®istry, log) + updateImage(deployment.Spec.Template.Spec.Containers, deployment.GetName(), ®istry, log) deployment.Spec.Template.Spec.ImagePullSecrets = addImagePullSecrets( deployment.Spec.Template.Spec.ImagePullSecrets, ®istry, log) - err = scheme.Scheme.Convert(deployment, u, nil) - if err != nil { + 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 +} + +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 + } + + registry := instance.Spec.Registry + log.Debugw("Updating DaemonSet", "name", u.GetName(), "registry", registry) + + updateImage(daemonSet.Spec.Template.Spec.Containers, daemonSet.GetName(), ®istry, log) + daemonSet.Spec.Template.Spec.ImagePullSecrets = addImagePullSecrets( + daemonSet.Spec.Template.Spec.ImagePullSecrets, ®istry, log) + if err := scheme.Scheme.Convert(daemonSet, u, nil); err != nil { return err } // The zero-value timestamp defaulted by the conversion causes @@ -84,23 +111,20 @@ 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 +// updateImage updates the image of the deployment and daemonSet with a new registry and tag +func updateImage(containers []corev1.Container, name string, registry *servingv1alpha1.Registry, log *zap.SugaredLogger) { for index := range containers { container := &containers[index] - newImage := getNewImage(registry, container.Name) - if newImage != "" { + if newImage := getNewImage(registry, container.Name); 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", 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 +133,7 @@ func updateCachingImage(instance *servingv1alpha1.KnativeServing, u *unstructure log.Debugw("Updating Image", "name", u.GetName(), "registry", registry) updateImageSpec(image, ®istry, 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 +146,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 +155,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) diff --git a/pkg/reconciler/knativeserving/common/images_test.go b/pkg/reconciler/knativeserving/common/images_test.go index f30cfd7a..8cea7d25 100644 --- a/pkg/reconciler/knativeserving/common/images_test.go +++ b/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 := ResourceTransform(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 := ResourceTransform(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,13 +337,13 @@ 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 := ResourceTransform(instance, log) deploymentTransform(&unstructuredDeployment) var deployment = &appsv1.Deployment{} @@ -313,6 +351,21 @@ func runImagePullSecretsTest(t *testing.T, tt *addImagePullSecretsTest) { 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 := ResourceTransform(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{}) {