diff --git a/pkg/apis/gcp/validation/shoot.go b/pkg/apis/gcp/validation/shoot.go index 1a2df746e..cf92003d6 100644 --- a/pkg/apis/gcp/validation/shoot.go +++ b/pkg/apis/gcp/validation/shoot.go @@ -18,8 +18,10 @@ import ( "fmt" "github.com/gardener/gardener/pkg/apis/core" + "github.com/gardener/gardener/pkg/apis/core/helper" "github.com/gardener/gardener/pkg/apis/core/validation" + "k8s.io/apimachinery/pkg/api/equality" apivalidation "k8s.io/apimachinery/pkg/api/validation" "k8s.io/apimachinery/pkg/util/validation/field" ) @@ -84,21 +86,19 @@ func validateVolume(vol *core.Volume, fldPath *field.Path) field.ErrorList { func ValidateWorkersUpdate(oldWorkers, newWorkers []core.Worker, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} for i, newWorker := range newWorkers { - workerFldPath := fldPath.Index(i) - for _, oldWorker := range oldWorkers { - if newWorker.Name == oldWorker.Name { - if validation.ShouldEnforceImmutability(newWorker.Zones, oldWorker.Zones) { - allErrs = append(allErrs, apivalidation.ValidateImmutableField(newWorker.Zones, oldWorker.Zones, workerFldPath.Child("zones"))...) - } - break - } + oldWorker := helper.FindWorkerByName(oldWorkers, newWorker.Name) + + if oldWorker != nil && validation.ShouldEnforceImmutability(newWorker.Zones, oldWorker.Zones) { + allErrs = append(allErrs, apivalidation.ValidateImmutableField(newWorker.Zones, oldWorker.Zones, workerFldPath.Child("zones"))...) } // TODO: This check won't be needed after generic support to scale from zero is introduced in CA // Ongoing issue - https://github.com/gardener/autoscaler/issues/27 - if err := ValidateWorkerAutoScaling(newWorker, workerFldPath.Child("minimum").String()); err != nil { - allErrs = append(allErrs, field.Forbidden(workerFldPath.Child("minimum"), err.Error())) + if !equality.Semantic.DeepEqual(&newWorker, oldWorker) { + if err := ValidateWorkerAutoScaling(newWorker, workerFldPath.Child("minimum").String()); err != nil { + allErrs = append(allErrs, field.Forbidden(workerFldPath.Child("minimum"), err.Error())) + } } } return allErrs diff --git a/pkg/apis/gcp/validation/shoot_test.go b/pkg/apis/gcp/validation/shoot_test.go index 528b16575..9cc4075ba 100644 --- a/pkg/apis/gcp/validation/shoot_test.go +++ b/pkg/apis/gcp/validation/shoot_test.go @@ -131,7 +131,7 @@ var _ = Describe("Shoot validation", func() { oldWorkers := []core.Worker{ { - Name: "worker-test", + Name: "worker-test2", Machine: core.Machine{ Type: "n1-standard-2", Image: &core.ShootMachineImage{ @@ -156,7 +156,7 @@ var _ = Describe("Shoot validation", func() { Expect(errorList).To(BeEmpty()) }) - It("should return an error because new worker pool added does not honor having min workers >= number of zones", func() { + It("should return an error because updated worker pool does not honor having min workers >= number of zones", func() { newWorkers := []core.Worker{ { Name: "worker-test", @@ -205,8 +205,94 @@ var _ = Describe("Shoot validation", func() { errorList := ValidateWorkersUpdate(oldWorkers, newWorkers, workerPath) + Expect(errorList).ToNot(HaveLen(0)) Expect(errorList[0].Error()).To(Equal("spec.workers[0].minimum: Forbidden: spec.workers[0].minimum value must be >= " + fmt.Sprint(len(newWorkers[0].Zones)) + " (number of zones) if maximum value > 0 (auto scaling to 0 & from 0 is not supported)")) }) + + It("should return an error because newly added worker pool does not honor having min workers >= number of zones", func() { + newWorkers := []core.Worker{ + { + Name: "worker-test", + Machine: core.Machine{ + Type: "n1-standard-2", + Image: &core.ShootMachineImage{ + Name: "gardenlinux", + Version: "318.8.0", + }, + }, + Maximum: 6, + Minimum: 1, + Zones: []string{ + "europe-west1-b", + "europe-west1-c", + }, + Volume: &core.Volume{ + Type: pointer.StringPtr("pd-standard"), + VolumeSize: "50Gi", + }, + }, + } + + oldWorkers := []core.Worker{} + + errorList := ValidateWorkersUpdate(oldWorkers, newWorkers, workerPath) + + Expect(errorList).ToNot(HaveLen(0)) + Expect(errorList[0].Error()).To(Equal("spec.workers[0].minimum: Forbidden: spec.workers[0].minimum value must be >= " + fmt.Sprint(len(newWorkers[0].Zones)) + " (number of zones) if maximum value > 0 (auto scaling to 0 & from 0 is not supported)")) + }) + + It("should not return any error because existing worker pool does not honor having min workers >= number of zones", func() { + newWorkers := []core.Worker{ + { + Name: "worker-test", + Machine: core.Machine{ + Type: "n1-standard-2", + Image: &core.ShootMachineImage{ + Name: "gardenlinux", + Version: "318.8.0", + }, + }, + Maximum: 6, + Minimum: 2, + Zones: []string{ + "europe-west1-b", + "europe-west1-c", + "europe-west1-d", + }, + Volume: &core.Volume{ + Type: pointer.StringPtr("pd-standard"), + VolumeSize: "50Gi", + }, + }, + } + + oldWorkers := []core.Worker{ + { + Name: "worker-test", + Machine: core.Machine{ + Type: "n1-standard-2", + Image: &core.ShootMachineImage{ + Name: "gardenlinux", + Version: "318.8.0", + }, + }, + Maximum: 6, + Minimum: 2, + Zones: []string{ + "europe-west1-b", + "europe-west1-c", + "europe-west1-d", + }, + Volume: &core.Volume{ + Type: pointer.StringPtr("pd-standard"), + VolumeSize: "50Gi", + }, + }, + } + + errorList := ValidateWorkersUpdate(oldWorkers, newWorkers, workerPath) + Expect(errorList).To(BeEmpty()) + }) }) })