Cleanup CloudProfile docker migration and Add Validation MachineImage…

…Version CRI (#6480)

* Add validation that cri should contain docker and can't be nil

* Add defaulting for cri

* Drop migration code from controller

* make generate

* Adapt changes in local provider

* Address PR Review

* Address PR Review

example/provider-local/garden/base/cloudprofile.yaml

@@ -28,6 +28,10 @@ spec:
     - version: 1.0.0
       cri:
       - name: containerd
+      # provider-local image doesn't contain a full docker runtime but includes nerdctl imitating the docker CLI for fulfilling gardener's bootstrap needs:
+      # see https://github.com/gardener/machine-controller-manager-provider-local/blob/f2c93198c794afc4e8e742a26026584ecce9aadf/node/Dockerfile#L9-L20
+      # this can be removed as soon as https://github.com/gardener/gardener/issues/4673 is resolved
+      - name: docker
   providerConfig:
     apiVersion: local.provider.extensions.gardener.cloud/v1alpha1
     kind: CloudProfileConfig

pkg/apis/core/v1alpha1/defaults.go

@@ -81,6 +81,21 @@ func defaultSubject(obj *rbacv1.Subject) {
 	}
 }
 
+// SetDefaults_MachineImageVersion sets default values for MachineImageVersion objects.
+func SetDefaults_MachineImageVersion(obj *MachineImageVersion) {
+	if len(obj.CRI) == 0 {
+		obj.CRI = []CRI{
+			{
+				Name: CRINameDocker,
+			},
+		}
+	}
+
+	if len(obj.Architectures) == 0 {
+		obj.Architectures = []string{v1beta1constants.ArchitectureAMD64}
+	}
+}
+
 // SetDefaults_MachineType sets default values for MachineType objects.
 func SetDefaults_MachineType(obj *MachineType) {
 	if obj.Usable == nil {

pkg/apis/core/v1alpha1/defaults_test.go

@@ -742,4 +742,20 @@ var _ = Describe("Defaults", func() {
 			Expect(obj.RecommenderInterval).To(PointTo(Equal(metav1.Duration{Duration: time.Minute})))
 		})
 	})
+
+	Describe("#SetDefaults_MachineImageVersion", func() {
+		var obj *MachineImageVersion
+
+		BeforeEach(func() {
+			obj = &MachineImageVersion{}
+		})
+
+		It("should correctly default the MachineImageVersion", func() {
+			SetDefaults_MachineImageVersion(obj)
+
+			Expect(len(obj.CRI)).To(Equal(1))
+			Expect(obj.CRI[0].Name).To(Equal(CRINameDocker))
+			Expect(obj.Architectures).To(Equal([]string{"amd64"}))
+		})
+	})
 })

pkg/apis/core/v1beta1/defaults.go

@@ -434,6 +434,14 @@ func SetDefaults_ControllerRegistrationDeployment(obj *ControllerRegistrationDep
 
 // SetDefaults_MachineImageVersion sets default values for MachineImageVersion objects.
 func SetDefaults_MachineImageVersion(obj *MachineImageVersion) {
+	if len(obj.CRI) == 0 {
+		obj.CRI = []CRI{
+			{
+				Name: CRINameDocker,
+			},
+		}
+	}
+
 	if len(obj.Architectures) == 0 {
 		obj.Architectures = []string{v1beta1constants.ArchitectureAMD64}
 	}

pkg/apis/core/v1beta1/defaults_test.go

@@ -753,6 +753,8 @@ var _ = Describe("Defaults", func() {
 		It("should correctly set the default MachineImageVersion", func() {
 			SetDefaults_MachineImageVersion(obj)
 
+			Expect(len(obj.CRI)).To(Equal(1))
+			Expect(obj.CRI[0].Name).To(Equal(CRINameDocker))
 			Expect(obj.Architectures).To(Equal([]string{"amd64"}))
 		})
 	})

pkg/apis/core/validation/cloudprofile.go

@@ -336,6 +336,12 @@ func validateMachineImages(machineImages []core.MachineImage, fldPath *field.Pat
 func validateContainerRuntimesInterfaces(cris []core.CRI, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 	duplicateCRI := sets.String{}
+	hasDocker := false
+
+	if len(cris) == 0 {
+		allErrs = append(allErrs, field.Required(fldPath, "must provide at least one supported container runtime"))
+		return allErrs
+	}
 
 	for i, cri := range cris {
 		criPath := fldPath.Index(i)
@@ -344,12 +350,20 @@ func validateContainerRuntimesInterfaces(cris []core.CRI, fldPath *field.Path) f
 		}
 		duplicateCRI.Insert(string(cri.Name))
 
+		if cri.Name == core.CRINameDocker {
+			hasDocker = true
+		}
+
 		if !availableWorkerCRINames.Has(string(cri.Name)) {
 			allErrs = append(allErrs, field.NotSupported(criPath, cri, availableWorkerCRINames.List()))
 		}
 		allErrs = append(allErrs, validateContainerRuntimes(cri.ContainerRuntimes, criPath.Child("containerRuntimes"))...)
 	}
 
+	if !hasDocker {
+		allErrs = append(allErrs, field.Invalid(fldPath, cris, "must provide docker as supported container runtime"))
+	}
+
 	return allErrs
 }
 

pkg/apis/core/validation/cloudprofile_test.go

@@ -206,6 +206,7 @@ var _ = Describe("CloudProfile Validation Tests ", func() {
 										ExpirableVersion: core.ExpirableVersion{
 											Version: "1.2.3",
 										},
+										CRI: []core.CRI{{Name: "docker"}},
 									},
 								},
 							},
@@ -411,6 +412,7 @@ var _ = Describe("CloudProfile Validation Tests ", func() {
 										Version:        "3.4.6",
 										Classification: &supportedClassification,
 									},
+									CRI: []core.CRI{{Name: "docker"}},
 								},
 							},
 						},
@@ -422,6 +424,7 @@ var _ = Describe("CloudProfile Validation Tests ", func() {
 										Version:        "3.4.5",
 										Classification: &previewClassification,
 									},
+									CRI: []core.CRI{{Name: "docker"}},
 								},
 							},
 						},
@@ -461,6 +464,7 @@ var _ = Describe("CloudProfile Validation Tests ", func() {
 										Version:        "0.1.2",
 										Classification: &supportedClassification,
 									},
+									CRI: []core.CRI{{Name: "docker"}},
 								},
 							},
 						},
@@ -472,6 +476,7 @@ var _ = Describe("CloudProfile Validation Tests ", func() {
 										Version:        "a.b.c",
 										Classification: &supportedClassification,
 									},
+									CRI: []core.CRI{{Name: "docker"}},
 								},
 							},
 						},
@@ -500,12 +505,14 @@ var _ = Describe("CloudProfile Validation Tests ", func() {
 										ExpirationDate: expirationDate,
 										Classification: &previewClassification,
 									},
+									CRI: []core.CRI{{Name: "docker"}},
 								},
 								{
 									ExpirableVersion: core.ExpirableVersion{
 										Version:        "0.1.1",
 										Classification: &supportedClassification,
 									},
+									CRI: []core.CRI{{Name: "docker"}},
 								},
 							},
 						},
@@ -518,6 +525,7 @@ var _ = Describe("CloudProfile Validation Tests ", func() {
 										ExpirationDate: expirationDate,
 										Classification: &supportedClassification,
 									},
+									CRI: []core.CRI{{Name: "docker"}},
 								},
 							},
 						},
@@ -538,6 +546,7 @@ var _ = Describe("CloudProfile Validation Tests ", func() {
 										Version:        "0.1.2",
 										Classification: &classification,
 									},
+									CRI: []core.CRI{{Name: "docker"}},
 								},
 							},
 						},
@@ -560,18 +569,21 @@ var _ = Describe("CloudProfile Validation Tests ", func() {
 									ExpirableVersion: core.ExpirableVersion{
 										Version: "0.1.2",
 									},
+									CRI:           []core.CRI{{Name: "docker"}},
 									Architectures: []string{"amd64", "arm64"},
 								},
 								{
 									ExpirableVersion: core.ExpirableVersion{
 										Version: "0.1.3",
 									},
+									CRI:           []core.CRI{{Name: "docker"}},
 									Architectures: []string{"amd64"},
 								},
 								{
 									ExpirableVersion: core.ExpirableVersion{
 										Version: "0.1.4",
 									},
+									CRI:           []core.CRI{{Name: "docker"}},
 									Architectures: []string{"arm64"},
 								},
 							},
@@ -591,6 +603,7 @@ var _ = Describe("CloudProfile Validation Tests ", func() {
 									ExpirableVersion: core.ExpirableVersion{
 										Version: "0.1.2",
 									},
+									CRI:           []core.CRI{{Name: "docker"}},
 									Architectures: []string{"foo"},
 								},
 							},
@@ -605,6 +618,32 @@ var _ = Describe("CloudProfile Validation Tests ", func() {
 				})
 			})
 
+			It("should forbid if no cri is present", func() {
+				cloudProfile.Spec.MachineImages[0].Versions[0].CRI = nil
+
+				errorList := ValidateCloudProfile(cloudProfile)
+
+				Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{
+					"Type":  Equal(field.ErrorTypeRequired),
+					"Field": Equal("spec.machineImages[0].versions[0].cri"),
+				}))))
+			})
+
+			It("should forbid if docker container runtime interface not present", func() {
+				cloudProfile.Spec.MachineImages[0].Versions[0].CRI = []core.CRI{
+					{
+						Name: core.CRINameContainerD,
+					},
+				}
+
+				errorList := ValidateCloudProfile(cloudProfile)
+
+				Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{
+					"Type":  Equal(field.ErrorTypeInvalid),
+					"Field": Equal("spec.machineImages[0].versions[0].cri"),
+				}))))
+			})
+
 			It("should forbid non-supported container runtime interface names", func() {
 				cloudProfile.Spec.MachineImages = []core.MachineImage{
 					{
@@ -618,6 +657,9 @@ var _ = Describe("CloudProfile Validation Tests ", func() {
 									{
 										Name: "invalid-cri-name",
 									},
+									{
+										Name: "docker",
+									},
 								},
 							},
 						},
@@ -633,6 +675,9 @@ var _ = Describe("CloudProfile Validation Tests ", func() {
 									{
 										Name: core.CRINameContainerD,
 									},
+									{
+										Name: "docker",
+									},
 								},
 							},
 						},
@@ -655,6 +700,9 @@ var _ = Describe("CloudProfile Validation Tests ", func() {
 					{
 						Name: core.CRINameContainerD,
 					},
+					{
+						Name: "docker",
+					},
 				}
 
 				errorList := ValidateCloudProfile(cloudProfile)
@@ -678,6 +726,9 @@ var _ = Describe("CloudProfile Validation Tests ", func() {
 							},
 						},
 					},
+					{
+						Name: "docker",
+					},
 				}
 
 				errorList := ValidateCloudProfile(cloudProfile)
@@ -898,6 +949,7 @@ var _ = Describe("CloudProfile Validation Tests ", func() {
 										Version:        "1.2.3",
 										Classification: &supportedClassification,
 									},
+									CRI: []core.CRI{{Name: "docker"}},
 								},
 							},
 						},
@@ -948,20 +1000,23 @@ var _ = Describe("CloudProfile Validation Tests ", func() {
 							Version:        "2135.6.2",
 							Classification: &deprecatedClassification,
 						},
+						CRI: []core.CRI{{Name: "docker"}},
 					},
 					{
 						ExpirableVersion: core.ExpirableVersion{
 							Version:        "2135.6.1",
 							Classification: &deprecatedClassification,
 							ExpirationDate: dateInThePast,
 						},
+						CRI: []core.CRI{{Name: "docker"}},
 					},
 					{
 						ExpirableVersion: core.ExpirableVersion{
 							Version:        "2135.6.0",
 							Classification: &deprecatedClassification,
 							ExpirationDate: dateInThePast,
 						},
+						CRI: []core.CRI{{Name: "docker"}},
 					},
 				}
 				cloudProfileNew.Spec.MachineImages[0].Versions = versions[0:1]
@@ -1028,6 +1083,7 @@ var _ = Describe("CloudProfile Validation Tests ", func() {
 									ExpirableVersion: core.ExpirableVersion{
 										Version: "1.2.3",
 									},
+									CRI: []core.CRI{{Name: "docker"}},
 								},
 							},
 						},
@@ -1073,12 +1129,14 @@ var _ = Describe("CloudProfile Validation Tests ", func() {
 						ExpirableVersion: core.ExpirableVersion{
 							Version: "1.17.2",
 						},
+						CRI: []core.CRI{{Name: "docker"}},
 					},
 					{
 						ExpirableVersion: core.ExpirableVersion{
 							Version:        "1.17.1",
 							ExpirationDate: dateInThePast,
 						},
+						CRI: []core.CRI{{Name: "docker"}},
 					},
 				}
 				cloudProfile.Spec.MachineImages[0].Versions = versions

pkg/controllermanager/controller/cloudprofile/reconciler.go

@@ -91,39 +91,5 @@ func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
 		}
 	}
 
-	// TODO voelzmo - this migration step ensures that all MachineImageVersions in the Cloud Profile contain `docker` in their list of supported Container Runtimes
-	// This can be removed in a couple of versions. Note that while this is still in here, it is impossible to add an image without `docker` support!
-	migrationHappened := migrateMachineImageVersionCRISupport(cloudProfile)
-
-	if migrationHappened {
-		log.Info("Migrated Machine Image Versions to explicitly contain `docker` as supported CRI")
-		if err := r.Client.Update(ctx, cloudProfile); err != nil {
-			return reconcile.Result{}, fmt.Errorf("failed to update Cloud Profile spec for machine image CRI migration: %w", err)
-		}
-	}
-
 	return reconcile.Result{}, nil
 }
-
-func migrateMachineImageVersionCRISupport(cloudProfile *gardencorev1beta1.CloudProfile) bool {
-	var migrationHappened bool
-	for i, image := range cloudProfile.Spec.MachineImages {
-		for j, version := range image.Versions {
-			if containsDockerCRIName(version.CRI) {
-				continue
-			}
-			cloudProfile.Spec.MachineImages[i].Versions[j].CRI = append(version.CRI, gardencorev1beta1.CRI{Name: gardencorev1beta1.CRINameDocker})
-			migrationHappened = true
-		}
-	}
-	return migrationHappened
-}
-
-func containsDockerCRIName(cris []gardencorev1beta1.CRI) bool {
-	for _, cri := range cris {
-		if cri.Name == gardencorev1beta1.CRINameDocker {
-			return true
-		}
-	}
-	return false
-}