Enhance shoot update test and promote WorkerPoolKubernetesVersion t…

…o GA (gardener#6166)

* Make creating shoot client from admin kubeconfig reusable

* Make shoot update test work with worker pools with overridden Kubernetes versions

* Change label of default e2e shoot test from `fast` to `simple`

* Perform shoot update with multiple worker pools in simple e2e test

* Promote `WorkerPoolKubernetesVersion` feature gate to GA

* Address PR review feedback

* Address PR review feedback

* Move CreateShootClientFromAdminKubeconfig into access pkg

Co-authored-by: Tim Ebert <timebertt@gmail.com>

.test-defs/ShootKubernetesUpdateTest.yaml

@@ -18,4 +18,5 @@ spec:
     -shoot-name=$SHOOT_NAME
     -project-namespace=$PROJECT_NAMESPACE
     -version=$K8S_VERSION
+    -version-worker-pools=$K8S_VERSION_WORKER_POOLS
   image: eu.gcr.io/gardener-project/3rd/golang:1.18.1

Makefile

@@ -259,7 +259,7 @@ verify-extended: check-generate check format test-cov test-cov-clean test-integr
 # Rules for local environment                                       #
 #####################################################################
 
-kind-up kind-down gardener-up gardener-down register-local-env tear-down-local-env register-kind2-env tear-down-kind2-env test-e2e-local-fast test-e2e-local: export KUBECONFIG = $(GARDENER_LOCAL_KUBECONFIG)
+kind-up kind-down gardener-up gardener-down register-local-env tear-down-local-env register-kind2-env tear-down-kind2-env test-e2e-local-simple test-e2e-local: export KUBECONFIG = $(GARDENER_LOCAL_KUBECONFIG)
 
 kind2-up kind2-down gardenlet-kind2-up gardenlet-kind2-down: export KUBECONFIG = $(GARDENER_LOCAL2_KUBECONFIG)
 
@@ -325,8 +325,8 @@ register-kind2-env:
 tear-down-kind2-env:
 	kubectl delete -k $(REPO_ROOT)/example/provider-local/seed-kind2/local
 
-test-e2e-local-fast: $(GINKGO)
-	./hack/test-e2e-local.sh --label-filter "Shoot && fast"
+test-e2e-local-simple: $(GINKGO)
+	./hack/test-e2e-local.sh --label-filter "Shoot && simple"
 
 test-e2e-local: $(GINKGO)
 	@# run at maximum 5 tests in parallel for now until we have better experience of how much load a single prow pod can take

cmd/gardener-apiserver/app/gardener_apiserver.go

@@ -18,8 +18,6 @@ import (
 	"context"
 	"errors"
 	"flag"
-	"fmt"
-	"time"
 
 	"k8s.io/component-base/logs"
 
@@ -44,15 +42,13 @@ import (
 	seedmanagementinformer "github.com/gardener/gardener/pkg/client/seedmanagement/informers/externalversions"
 	settingsclientset "github.com/gardener/gardener/pkg/client/settings/clientset/versioned"
 	settingsinformer "github.com/gardener/gardener/pkg/client/settings/informers/externalversions"
-	"github.com/gardener/gardener/pkg/features"
 	"github.com/gardener/gardener/pkg/logger"
 	"github.com/gardener/gardener/pkg/openapi"
 
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
@@ -69,7 +65,6 @@ import (
 	kubeinformers "k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
-	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/tools/clientcmd"
 	"k8s.io/component-base/version"
 	"k8s.io/component-base/version/verflag"
@@ -348,35 +343,6 @@ func (o *Options) Run(ctx context.Context) error {
 		return err
 	}
 
-	if !utilfeature.DefaultFeatureGate.Enabled(features.WorkerPoolKubernetesVersion) {
-		if err := server.GenericAPIServer.AddPostStartHook("validate-WorkerPoolKubernetesVersion-feature-gate", func(hookContext genericapiserver.PostStartHookContext) error {
-			timeoutCtx, cancel := context.WithTimeout(ctx, time.Minute)
-			defer cancel()
-
-			shootInformer := o.CoreInformerFactory.Core().InternalVersion().Shoots()
-			if !cache.WaitForCacheSync(timeoutCtx.Done(), shootInformer.Informer().HasSynced) {
-				return fmt.Errorf("error while waiting for shoot informer cache to be synced")
-			}
-
-			shoots, err := shootInformer.Lister().List(labels.Everything())
-			if err != nil {
-				return err
-			}
-
-			for _, shoot := range shoots {
-				for _, worker := range shoot.Spec.Provider.Workers {
-					if worker.Kubernetes != nil && worker.Kubernetes.Version != nil {
-						return fmt.Errorf("WorkerPoolKubernetesVersion feature gate cannot be disabled since shoot %q still has worker pool %q which specifies .kubernetes.version", shoot.Name, worker.Name)
-					}
-				}
-			}
-
-			return nil
-		}); err != nil {
-			return err
-		}
-	}
-
 	return server.GenericAPIServer.PrepareRun().Run(ctx.Done())
 }
 

docs/deployment/feature_gates.md

@@ -35,8 +35,6 @@ The following tables are a summary of the feature gates that you can set on diff
 | RotateSSHKeypairOnMaintenance                | `false` | `Alpha` | `1.28` | `1.44` |
 | RotateSSHKeypairOnMaintenance                | `true`  | `Beta`  | `1.45` |        |
 | RotateSSHKeypairOnMaintenance (deprecated)   | `false` | `Beta`  | `1.48` |        |
-| WorkerPoolKubernetesVersion                  | `false` | `Alpha` | `1.35` | `1.45` |
-| WorkerPoolKubernetesVersion                  | `true`  | `Beta`  | `1.46` |        |
 | CopyEtcdBackupsDuringControlPlaneMigration   | `false` | `Alpha` | `1.37` |        |
 | SecretBindingProviderValidation              | `false` | `Alpha` | `1.38` |        |
 | ForceRestore                                 | `false` | `Alpha` | `1.39` |        |
@@ -82,6 +80,9 @@ The following tables are a summary of the feature gates that you can set on diff
 | ShootMaxTokenExpirationValidation            | `false` | `Alpha`   | `1.43` | `1.45` |
 | ShootMaxTokenExpirationValidation            | `true`  | `Beta`    | `1.46` | `1.47` |
 | ShootMaxTokenExpirationValidation            | `true`  | `GA`      | `1.48` |        |
+| WorkerPoolKubernetesVersion                  | `false` | `Alpha`   | `1.35` | `1.45` |
+| WorkerPoolKubernetesVersion                  | `true`  | `Beta`    | `1.46` | `1.49` |
+| WorkerPoolKubernetesVersion                  | `true`  | `GA`      | `1.50` |        |
 
 ## Using a feature
 

docs/deployment/getting_started_locally.md

@@ -78,7 +78,7 @@ local   local          local      local    1.21.0        Awake         Create Pr
 (Optional): You could also execute a simple e2e test (creating and deleting a shoot) by running
 
 ```shell
-make test-e2e-local-fast KUBECONFIG="$PWD/example/gardener-local/kind/kubeconfig"
+make test-e2e-local-simple KUBECONFIG="$PWD/example/gardener-local/kind/kubeconfig"
 ```
 
 ### Accessing the `Shoot` cluster

docs/development/getting_started_locally.md

@@ -123,7 +123,7 @@ local   local          local      local    1.21.0        Awake         Create Pr
 (Optional): You could also execute a simple e2e test (creating and deleting a shoot) by running
 
 ```shell
-make test-e2e-local-fast KUBECONFIG="$PWD/example/gardener-local/kind/kubeconfig"
+make test-e2e-local-simple KUBECONFIG="$PWD/example/gardener-local/kind/kubeconfig"
 ```
 
 When the shoot got successfully created you can access it as follows:

docs/development/testing.md

@@ -66,7 +66,7 @@ You can also run these tests on your development machine, using the following co
 make kind-up
 export KUBECONFIG=$PWD/example/gardener-local/kind/kubeconfig
 make gardener-up
-make test-e2e-local  # alternatively: make test-e2e-local-fast
+make test-e2e-local  # alternatively: make test-e2e-local-simple
 ```
 
 If you want to run a specific set of e2e test cases, you can also execute them using `./hack/test-e2e-local.sh` directly in combination with [ginkgo label filters](https://onsi.github.io/ginkgo/#spec-labels). For example:

pkg/apis/core/validation/shoot.go

@@ -1171,13 +1171,9 @@ func ValidateWorker(worker core.Worker, kubernetesVersion string, fldPath *field
 	}
 	if worker.Kubernetes != nil {
 		if worker.Kubernetes.Version != nil {
-			if !utilfeature.DefaultFeatureGate.Enabled(features.WorkerPoolKubernetesVersion) {
-				allErrs = append(allErrs, field.Forbidden(fldPath.Child("kubernetes", "version"), "worker pool kubernetes version may only be set if WorkerPoolKubernetesVersion feature gate is enabled"))
-			} else {
-				workerGroupKubernetesVersion := *worker.Kubernetes.Version
-				allErrs = append(allErrs, validateWorkerGroupAndControlPlaneKubernetesVersion(kubernetesVersion, workerGroupKubernetesVersion, fldPath.Child("kubernetes", "version"))...)
-				kubernetesVersion = workerGroupKubernetesVersion
-			}
+			workerGroupKubernetesVersion := *worker.Kubernetes.Version
+			allErrs = append(allErrs, validateWorkerGroupAndControlPlaneKubernetesVersion(kubernetesVersion, workerGroupKubernetesVersion, fldPath.Child("kubernetes", "version"))...)
+			kubernetesVersion = workerGroupKubernetesVersion
 		}
 
 		if worker.Kubernetes.Kubelet != nil {

pkg/apis/core/validation/shoot_test.go

@@ -2271,22 +2271,7 @@ var _ = Describe("Shoot Validation Tests", func() {
 		})
 
 		Context("worker pool kubernetes version", func() {
-			It("should forbid specifying a worker pool kubernetes version since the WorkerPoolKubernetesVersion feature gate is disabled", func() {
-				defer test.WithFeatureGate(utilfeature.DefaultFeatureGate, features.WorkerPoolKubernetesVersion, false)()
-
-				newShoot := prepareShootForUpdate(shoot)
-				newShoot.Spec.Provider.Workers[0].Kubernetes = &core.WorkerKubernetes{Version: &shoot.Spec.Kubernetes.Version}
-
-				Expect(ValidateShootUpdate(newShoot, shoot)).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{
-					"Type":   Equal(field.ErrorTypeForbidden),
-					"Field":  Equal("spec.provider.workers[0].kubernetes.version"),
-					"Detail": Equal("worker pool kubernetes version may only be set if WorkerPoolKubernetesVersion feature gate is enabled"),
-				}))))
-			})
-
 			It("should forbid worker pool kubernetes version higher than control plane", func() {
-				defer test.WithFeatureGate(utilfeature.DefaultFeatureGate, features.WorkerPoolKubernetesVersion, true)()
-
 				newShoot := prepareShootForUpdate(shoot)
 				newShoot.Spec.Provider.Workers[0].Kubernetes = &core.WorkerKubernetes{Version: pointer.String("1.21.0")}
 
@@ -2298,17 +2283,13 @@ var _ = Describe("Shoot Validation Tests", func() {
 			})
 
 			It("should work to set worker pool kubernetes version equal to control plane version", func() {
-				defer test.WithFeatureGate(utilfeature.DefaultFeatureGate, features.WorkerPoolKubernetesVersion, true)()
-
 				newShoot := prepareShootForUpdate(shoot)
 				newShoot.Spec.Provider.Workers[0].Kubernetes = &core.WorkerKubernetes{Version: pointer.String("1.20.2")}
 
 				Expect(ValidateShootUpdate(newShoot, shoot)).To(BeEmpty())
 			})
 
 			It("should work to set worker pool kubernetes version lower one minor than control plane version", func() {
-				defer test.WithFeatureGate(utilfeature.DefaultFeatureGate, features.WorkerPoolKubernetesVersion, true)()
-
 				shoot.Spec.Provider.Workers[0].Kubernetes = &core.WorkerKubernetes{Version: pointer.String("1.20.2")}
 
 				newShoot := prepareShootForUpdate(shoot)
@@ -2318,8 +2299,6 @@ var _ = Describe("Shoot Validation Tests", func() {
 			})
 
 			It("should work to set worker pool kubernetes version lower two minor than control plane version", func() {
-				defer test.WithFeatureGate(utilfeature.DefaultFeatureGate, features.WorkerPoolKubernetesVersion, true)()
-
 				shoot.Spec.Kubernetes.Version = "1.21.0"
 				shoot.Spec.Provider.Workers[0].Kubernetes = &core.WorkerKubernetes{Version: pointer.String("1.20.2")}
 
@@ -2330,8 +2309,6 @@ var _ = Describe("Shoot Validation Tests", func() {
 			})
 
 			It("forbid to set worker pool kubernetes version lower three minor than control plane version", func() {
-				defer test.WithFeatureGate(utilfeature.DefaultFeatureGate, features.WorkerPoolKubernetesVersion, true)()
-
 				shoot.Spec.Kubernetes.Version = "1.22.0"
 				shoot.Spec.Provider.Workers[0].Kubernetes = &core.WorkerKubernetes{Version: pointer.String("1.20.2")}
 
@@ -2346,8 +2323,6 @@ var _ = Describe("Shoot Validation Tests", func() {
 			})
 
 			It("should work to set worker pool kubernetes version to nil with one minor difference", func() {
-				defer test.WithFeatureGate(utilfeature.DefaultFeatureGate, features.WorkerPoolKubernetesVersion, true)()
-
 				shoot.Spec.Kubernetes.Version = "1.21.0"
 				shoot.Spec.Provider.Workers[0].Kubernetes = &core.WorkerKubernetes{Version: pointer.String("1.20.2")}
 
@@ -2358,8 +2333,6 @@ var _ = Describe("Shoot Validation Tests", func() {
 			})
 
 			It("forbid to set worker pool kubernetes version to nil with two minor difference", func() {
-				defer test.WithFeatureGate(utilfeature.DefaultFeatureGate, features.WorkerPoolKubernetesVersion, true)()
-
 				shoot.Spec.Kubernetes.Version = "1.21.0"
 				shoot.Spec.Provider.Workers[0].Kubernetes = &core.WorkerKubernetes{Version: pointer.String("1.19.2")}
 

pkg/features/features.go

@@ -112,6 +112,7 @@ const (
 	// owner: @rfranzke @majst01 @mwennrich
 	// alpha: v1.35.0
 	// beta: v1.46.0
+	// GA: v1.50.0
 	WorkerPoolKubernetesVersion featuregate.Feature = "WorkerPoolKubernetesVersion"
 
 	// CopyEtcdBackupsDuringControlPlaneMigration enables the copy of etcd backups from the object store of the source seed
@@ -197,7 +198,7 @@ var allFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
 	UseDNSRecords:                 {Default: true, PreRelease: featuregate.GA, LockToDefault: true},
 	RotateSSHKeypairOnMaintenance: {Default: false, PreRelease: featuregate.Beta},
 	DenyInvalidExtensionResources: {Default: true, PreRelease: featuregate.GA, LockToDefault: true},
-	WorkerPoolKubernetesVersion:   {Default: true, PreRelease: featuregate.Beta},
+	WorkerPoolKubernetesVersion:   {Default: true, PreRelease: featuregate.GA, LockToDefault: true},
 	CopyEtcdBackupsDuringControlPlaneMigration: {Default: false, PreRelease: featuregate.Alpha},
 	SecretBindingProviderValidation:            {Default: false, PreRelease: featuregate.Alpha},
 	ForceRestore:                               {Default: false, PreRelease: featuregate.Alpha},

test/e2e/shoot/create_and_delete.go → test/e2e/shoot/create_update_delete.go

@@ -18,19 +18,36 @@ import (
 	"context"
 	"time"
 
-	"github.com/gardener/gardener/test/e2e/shoot/internal/access"
+	gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
+	"github.com/gardener/gardener/test/framework"
+	"github.com/gardener/gardener/test/utils/shoots/access"
+	shootupdatesuite "github.com/gardener/gardener/test/utils/shoots/update"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/utils/pointer"
 )
 
 var _ = Describe("Shoot Tests", Label("Shoot"), func() {
 	f := defaultShootCreationFramework()
 	f.Shoot = defaultShoot("")
 	f.Shoot.Name = "e2e-default"
 
-	It("Create and Delete", Label("fast"), func() {
+	// explicitly use one version below the latest supported minor version so that Kubernetes version update test can be
+	// performed
+	f.Shoot.Spec.Kubernetes.Version = "1.23.6"
+
+	// create two additional worker pools which explicitly specify the kubernetes version
+	pool1 := f.Shoot.Spec.Provider.Workers[0]
+	pool2, pool3 := pool1.DeepCopy(), pool1.DeepCopy()
+	pool2.Name += "2"
+	pool2.Kubernetes = &gardencorev1beta1.WorkerKubernetes{Version: &f.Shoot.Spec.Kubernetes.Version}
+	pool3.Name += "3"
+	pool3.Kubernetes = &gardencorev1beta1.WorkerKubernetes{Version: pointer.String("1.22.0")}
+	f.Shoot.Spec.Provider.Workers = append(f.Shoot.Spec.Provider.Workers, *pool2, *pool3)
+
+	It("Create, Update, Delete", Label("simple"), func() {
 		By("Create Shoot")
 		ctx, cancel := context.WithTimeout(parentCtx, 15*time.Minute)
 		defer cancel()
@@ -45,6 +62,12 @@ var _ = Describe("Shoot Tests", Label("Shoot"), func() {
 			g.Expect(shootClient.Client().List(ctx, &corev1.NamespaceList{})).To(Succeed())
 		}).Should(Succeed())
 
+		By("Update Shoot")
+		shootupdatesuite.RunTest(ctx, &framework.ShootFramework{
+			GardenerFramework: f.GardenerFramework,
+			Shoot:             f.Shoot,
+		}, nil, nil)
+
 		By("Delete Shoot")
 		ctx, cancel = context.WithTimeout(parentCtx, 15*time.Minute)
 		defer cancel()

test/e2e/shoot/internal/rotation/secret_encryption.go

@@ -23,8 +23,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/gardener/gardener/pkg/client/kubernetes"
-	"github.com/gardener/gardener/test/e2e/shoot/internal/access"
 	"github.com/gardener/gardener/test/framework"
+	"github.com/gardener/gardener/test/utils/shoots/access"
 )
 
 // SecretEncryptionVerifier creates and reads secrets in the shoot to verify correct configuration of etcd encryption.

test/e2e/shoot/internal/rotation/shoot_access.go

@@ -22,8 +22,8 @@ import (
 	corev1 "k8s.io/api/core/v1"
 
 	"github.com/gardener/gardener/pkg/client/kubernetes"
-	"github.com/gardener/gardener/test/e2e/shoot/internal/access"
 	"github.com/gardener/gardener/test/framework"
+	"github.com/gardener/gardener/test/utils/shoots/access"
 )
 
 type clients struct {

test/testmachinery/system/shoot_update/update_test.go

@@ -29,18 +29,18 @@ package shootupdate
 import (
 	"context"
 	"flag"
-	"fmt"
 	"time"
 
-	gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
-	gardencorev1beta1helper "github.com/gardener/gardener/pkg/apis/core/v1beta1/helper"
 	"github.com/gardener/gardener/test/framework"
+	shootupdatesuite "github.com/gardener/gardener/test/utils/shoots/update"
 
 	. "github.com/onsi/ginkgo/v2"
-	. "github.com/onsi/gomega"
 )
 
-var kubernetesVersion = flag.String("version", "", "the version to update the shoot")
+var (
+	newControlPlaneKubernetesVersion = flag.String("version", "", "the version to use for .spec.kubernetes.version and .spec.provider.workers[].kubernetes.version (only when nil or equal to .spec.kubernetes.version)")
+	newWorkerPoolKubernetesVersion   = flag.String("version-worker-pools", "", "the version to use for .spec.provider.workers[].kubernetes.version (only when not equal to .spec.kubernetes.version)")
+)
 
 const UpdateKubernetesVersionTimeout = 45 * time.Minute
 
@@ -49,38 +49,9 @@ func init() {
 }
 
 var _ = Describe("Shoot update testing", func() {
-
 	f := framework.NewShootFramework(nil)
 
-	framework.CIt("should update the kubernetes version of the shoot to the next version", func(ctx context.Context) {
-		currentVersion := f.Shoot.Spec.Kubernetes.Version
-		newVersion := *kubernetesVersion
-		if currentVersion == newVersion {
-			Skip("shoot already has the desired kubernetes version")
-		}
-		if newVersion == "" {
-			var (
-				err                       error
-				ok                        bool
-				consecutiveMinorAvailable bool
-			)
-			cloudprofile, err := f.GetCloudProfile(ctx)
-			Expect(err).ToNot(HaveOccurred())
-			consecutiveMinorAvailable, newVersion, err = gardencorev1beta1helper.GetKubernetesVersionForMinorUpdate(cloudprofile, currentVersion)
-			Expect(err).ToNot(HaveOccurred())
-			Expect(consecutiveMinorAvailable).To(BeTrue())
-			if !ok {
-				Skip("no new version found")
-			}
-		}
-
-		By(fmt.Sprintf("updating shoot %s to version %s", f.Shoot.GetName(), newVersion))
-		err := f.UpdateShoot(ctx, func(shoot *gardencorev1beta1.Shoot) error {
-			shoot.Spec.Kubernetes.Version = newVersion
-			return nil
-		})
-		Expect(err).ToNot(HaveOccurred())
-
+	framework.CIt("should update the kubernetes version of the shoot and its worker pools to the respective next versions", func(ctx context.Context) {
+		shootupdatesuite.RunTest(ctx, f, newControlPlaneKubernetesVersion, newWorkerPoolKubernetesVersion)
 	}, UpdateKubernetesVersionTimeout)
-
 })