Merge pull request #60337 from verult/repd-feature-gate

Automatic merge from submit-queue (batch tested with PRs 60157, 60337, 60246, 59714, 60467). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Feature gate for regional PDs

**What this PR does / why we need it**: Adding beta feature gate around regional PD support.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: Partially addresses #59988

**Special notes for your reviewer**: This feature has already been in alpha for two releases, but at the time it was not gated with a Kubernetes feature gate. Instead it was controlled by a GCE-specific alpha gate. However, there are additional changes with GCE PD StorageClass parameters that we'd like to gate as well, and this is out of scope of GCE alpha gates.

/cc @saad-ali @lavalamp

pkg/cloudprovider/providers/gce/BUILD

@@ -51,6 +51,7 @@ go_library(
         "//pkg/cloudprovider/providers/gce/cloud/filter:go_default_library",
         "//pkg/cloudprovider/providers/gce/cloud/meta:go_default_library",
         "//pkg/controller:go_default_library",
+        "//pkg/features:go_default_library",
         "//pkg/kubelet/apis:go_default_library",
         "//pkg/master/ports:go_default_library",
         "//pkg/util/net/sets:go_default_library",
@@ -79,6 +80,7 @@ go_library(
         "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/watch:go_default_library",
+        "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library",
         "//vendor/k8s.io/client-go/informers:go_default_library",
         "//vendor/k8s.io/client-go/kubernetes:go_default_library",
         "//vendor/k8s.io/client-go/kubernetes/scheme:go_default_library",

pkg/cloudprovider/providers/gce/gce_alpha.go

@@ -29,15 +29,12 @@ const (
 	// tier to use. Currently supports "Standard" and "Premium" (default).
 	AlphaFeatureNetworkTiers = "NetworkTiers"
 
-	AlphaFeatureGCEDisk = "DiskAlphaAPI"
-
 	AlphaFeatureNetworkEndpointGroup = "NetworkEndpointGroup"
 )
 
 // All known alpha features
 var knownAlphaFeatures = map[string]bool{
 	AlphaFeatureNetworkTiers:         true,
-	AlphaFeatureGCEDisk:              true,
 	AlphaFeatureNetworkEndpointGroup: true,
 }
 

pkg/cloudprovider/providers/gce/gce_disks.go

@@ -37,6 +37,8 @@ import (
 	computealpha "google.golang.org/api/compute/v0.alpha"
 	compute "google.golang.org/api/compute/v1"
 	"google.golang.org/api/googleapi"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	"k8s.io/kubernetes/pkg/features"
 )
 
 type DiskType string
@@ -143,7 +145,7 @@ func (manager *gceServiceManager) CreateRegionalDiskOnCloudProvider(
 	diskType string,
 	replicaZones sets.String) (gceObject, error) {
 
-	if manager.gce.AlphaFeatureGate.Enabled(AlphaFeatureGCEDisk) {
+	if utilfeature.DefaultFeatureGate.Enabled(features.GCERegionalPersistentDisk) {
 		diskTypeURI, err := manager.getDiskTypeURI(
 			manager.gce.region /* diskRegion */, multiZone{replicaZones}, diskType, true /* useAlphaAPI */)
 		if err != nil {
@@ -166,7 +168,7 @@ func (manager *gceServiceManager) CreateRegionalDiskOnCloudProvider(
 			manager.gce.projectID, manager.gce.region, diskToCreateAlpha).Do()
 	}
 
-	return nil, fmt.Errorf("The regional PD feature is only available via the GCE Alpha API. Enable \"DiskAlphaAPI\" in the list of \"alpha-features\" in \"gce.conf\" to use the feature.")
+	return nil, fmt.Errorf("the regional PD feature is only available with the %s Kubernetes feature gate enabled", features.GCERegionalPersistentDisk)
 }
 
 func (manager *gceServiceManager) AttachDiskOnCloudProvider(
@@ -238,7 +240,7 @@ func (manager *gceServiceManager) GetDiskFromCloudProvider(
 func (manager *gceServiceManager) GetRegionalDiskFromCloudProvider(
 	diskName string) (*GCEDisk, error) {
 
-	if manager.gce.AlphaFeatureGate.Enabled(AlphaFeatureGCEDisk) {
+	if utilfeature.DefaultFeatureGate.Enabled(features.GCERegionalPersistentDisk) {
 		diskAlpha, err := manager.gce.serviceAlpha.RegionDisks.Get(
 			manager.gce.projectID, manager.gce.region, diskName).Do()
 		if err != nil {
@@ -260,7 +262,7 @@ func (manager *gceServiceManager) GetRegionalDiskFromCloudProvider(
 		}, nil
 	}
 
-	return nil, fmt.Errorf("The regional PD feature is only available via the GCE Alpha API. Enable \"DiskAlphaAPI\" in the list of \"alpha-features\" in \"gce.conf\" to use the feature.")
+	return nil, fmt.Errorf("the regional PD feature is only available with the %s Kubernetes feature gate enabled", features.GCERegionalPersistentDisk)
 }
 
 func (manager *gceServiceManager) DeleteDiskOnCloudProvider(
@@ -272,12 +274,12 @@ func (manager *gceServiceManager) DeleteDiskOnCloudProvider(
 
 func (manager *gceServiceManager) DeleteRegionalDiskOnCloudProvider(
 	diskName string) (gceObject, error) {
-	if manager.gce.AlphaFeatureGate.Enabled(AlphaFeatureGCEDisk) {
+	if utilfeature.DefaultFeatureGate.Enabled(features.GCERegionalPersistentDisk) {
 		return manager.gce.serviceAlpha.RegionDisks.Delete(
 			manager.gce.projectID, manager.gce.region, diskName).Do()
 	}
 
-	return nil, fmt.Errorf("DeleteRegionalDiskOnCloudProvider is a regional PD feature and is only available via the GCE Alpha API. Enable \"DiskAlphaAPI\" in the list of \"alpha-features\" in \"gce.conf\" to use the feature.")
+	return nil, fmt.Errorf("the regional PD feature is only available with the %s Kubernetes feature gate enabled", features.GCERegionalPersistentDisk)
 }
 
 func (manager *gceServiceManager) WaitForZoneOp(
@@ -417,13 +419,13 @@ func (manager *gceServiceManager) ResizeDiskOnCloudProvider(disk *GCEDisk, sizeG
 }
 
 func (manager *gceServiceManager) RegionalResizeDiskOnCloudProvider(disk *GCEDisk, sizeGb int64) (gceObject, error) {
-	if manager.gce.AlphaFeatureGate.Enabled(AlphaFeatureGCEDisk) {
+	if utilfeature.DefaultFeatureGate.Enabled(features.GCERegionalPersistentDisk) {
 		resizeServiceRequest := &computealpha.RegionDisksResizeRequest{
 			SizeGb: sizeGb,
 		}
 		return manager.gce.serviceAlpha.RegionDisks.Resize(manager.gce.projectID, disk.Region, disk.Name, resizeServiceRequest).Do()
 	}
-	return nil, fmt.Errorf("RegionalResizeDiskOnCloudProvider is a regional PD feature and is only available via the GCE Alpha API. Enable \"DiskAlphaAPI\" in the list of \"alpha-features\" in \"gce.conf\" to use the feature.")
+	return nil, fmt.Errorf("the regional PD feature is only available with the %s Kubernetes feature gate enabled", features.GCERegionalPersistentDisk)
 }
 
 // Disks is interface for manipulation with GCE PDs.
@@ -530,7 +532,7 @@ func (gce *GCECloud) AttachDisk(diskName string, nodeName types.NodeName, readOn
 	// Try fetching as regional PD
 	var disk *GCEDisk
 	var mc *metricContext
-	if regional {
+	if regional && utilfeature.DefaultFeatureGate.Enabled(features.GCERegionalPersistentDisk) {
 		disk, err = gce.getRegionalDiskByName(diskName)
 		if err != nil {
 			glog.V(5).Infof("Could not find regional PD named %q to Attach. Will look for a zonal PD", diskName)
@@ -795,17 +797,20 @@ func (gce *GCECloud) ResizeDisk(diskToResize string, oldSize resource.Quantity,
 		}
 		return newSizeQuant, nil
 	case multiZone:
-		mc = newDiskMetricContextRegional("resize", disk.Region)
-		resizeOp, err := gce.manager.RegionalResizeDiskOnCloudProvider(disk, requestGB)
+		if utilfeature.DefaultFeatureGate.Enabled(features.GCERegionalPersistentDisk) {
+			mc = newDiskMetricContextRegional("resize", disk.Region)
+			resizeOp, err := gce.manager.RegionalResizeDiskOnCloudProvider(disk, requestGB)
 
-		if err != nil {
-			return oldSize, mc.Observe(err)
+			if err != nil {
+				return oldSize, mc.Observe(err)
+			}
+			waitErr := gce.manager.WaitForRegionalOp(resizeOp, mc)
+			if waitErr != nil {
+				return oldSize, waitErr
+			}
+			return newSizeQuant, nil
 		}
-		waitErr := gce.manager.WaitForRegionalOp(resizeOp, mc)
-		if waitErr != nil {
-			return oldSize, waitErr
-		}
-		return newSizeQuant, nil
+		return oldSize, fmt.Errorf("disk.ZoneInfo has unexpected type %T", zoneInfo)
 	case nil:
 		return oldSize, fmt.Errorf("PD has nil ZoneInfo: %v", disk)
 	default:
@@ -838,19 +843,26 @@ func (gce *GCECloud) GetAutoLabelsForPD(name string, zone string) (map[string]st
 		// We could assume the disks exists; we have all the information we need
 		// However it is more consistent to ensure the disk exists,
 		// and in future we may gather addition information (e.g. disk type, IOPS etc)
-		zoneSet, err := volumeutil.LabelZonesToSet(zone)
-		if err != nil {
-			glog.Warningf("Failed to parse zone field: %q. Will use raw field.", zone)
-		}
-
-		if len(zoneSet) > 1 {
-			// Regional PD
-			disk, err = gce.getRegionalDiskByName(name)
+		if utilfeature.DefaultFeatureGate.Enabled(features.GCERegionalPersistentDisk) {
+			zoneSet, err := volumeutil.LabelZonesToSet(zone)
 			if err != nil {
-				return nil, err
+				glog.Warningf("Failed to parse zone field: %q. Will use raw field.", zone)
+			}
+
+			if len(zoneSet) > 1 {
+				// Regional PD
+				disk, err = gce.getRegionalDiskByName(name)
+				if err != nil {
+					return nil, err
+				}
+			} else {
+				// Zonal PD
+				disk, err = gce.getDiskByName(name, zone)
+				if err != nil {
+					return nil, err
+				}
 			}
 		} else {
-			// Zonal PD
 			disk, err = gce.getDiskByName(name, zone)
 			if err != nil {
 				return nil, err
@@ -936,7 +948,7 @@ func (gce *GCECloud) getRegionalDiskByName(diskName string) (*GCEDisk, error) {
 // Prefer getDiskByName, if the zone can be established
 // Return cloudprovider.DiskNotFound if the given disk cannot be found in any zone
 func (gce *GCECloud) GetDiskByNameUnknownZone(diskName string) (*GCEDisk, error) {
-	if gce.AlphaFeatureGate.Enabled(AlphaFeatureGCEDisk) {
+	if utilfeature.DefaultFeatureGate.Enabled(features.GCERegionalPersistentDisk) {
 		regionalDisk, err := gce.getRegionalDiskByName(diskName)
 		if err == nil {
 			return regionalDisk, err
@@ -1020,12 +1032,15 @@ func (gce *GCECloud) doDeleteDisk(diskToDelete string) error {
 		}
 		return gce.manager.WaitForZoneOp(deleteOp, zoneInfo.zone, mc)
 	case multiZone:
-		mc = newDiskMetricContextRegional("delete", disk.Region)
-		deleteOp, err := gce.manager.DeleteRegionalDiskOnCloudProvider(disk.Name)
-		if err != nil {
-			return mc.Observe(err)
+		if utilfeature.DefaultFeatureGate.Enabled(features.GCERegionalPersistentDisk) {
+			mc = newDiskMetricContextRegional("delete", disk.Region)
+			deleteOp, err := gce.manager.DeleteRegionalDiskOnCloudProvider(disk.Name)
+			if err != nil {
+				return mc.Observe(err)
+			}
+			return gce.manager.WaitForRegionalOp(deleteOp, mc)
 		}
-		return gce.manager.WaitForRegionalOp(deleteOp, mc)
+		return fmt.Errorf("disk.ZoneInfo has unexpected type %T", zoneInfo)
 	case nil:
 		return fmt.Errorf("PD has nil ZoneInfo: %v", disk)
 	default:

pkg/cloudprovider/providers/gce/gce_disks_test.go

@@ -100,16 +100,11 @@ func TestCreateRegionalDisk_Basic(t *testing.T) {
 	gceRegion := "fake-region"
 	zonesWithNodes := []string{"zone1", "zone3", "zone2"}
 	fakeManager := newFakeManager(gceProjectId, gceRegion)
-	alphaFeatureGate, featureGateErr := NewAlphaFeatureGate([]string{AlphaFeatureGCEDisk})
 
-	if featureGateErr != nil {
-		t.Error(featureGateErr)
-	}
 	gce := GCECloud{
 		manager:            fakeManager,
 		managedZones:       zonesWithNodes,
 		projectID:          gceProjectId,
-		AlphaFeatureGate:   alphaFeatureGate,
 		nodeZones:          createNodeZones(zonesWithNodes),
 		nodeInformerSynced: func() bool { return true },
 	}

pkg/features/kube_features.go

@@ -255,6 +255,12 @@ const (
 	//
 	// Enable container log rotation for cri container runtime
 	CRIContainerLogRotation utilfeature.Feature = "CRIContainerLogRotation"
+
+	// owner: @verult
+	// beta: v1.10
+	//
+	// Enables the regional PD feature on GCE.
+	GCERegionalPersistentDisk utilfeature.Feature = "GCERegionalPersistentDisk"
 )
 
 func init() {
@@ -299,6 +305,7 @@ var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureS
 	NoDaemonSetScheduler:                        {Default: false, PreRelease: utilfeature.Alpha},
 	TokenRequest:                                {Default: false, PreRelease: utilfeature.Alpha},
 	CRIContainerLogRotation:                     {Default: false, PreRelease: utilfeature.Alpha},
+	GCERegionalPersistentDisk:                   {Default: true, PreRelease: utilfeature.Beta},
 
 	// inherited features from generic apiserver, relisted here to get a conflict if it is changed
 	// unintentionally on either side:

pkg/volume/gce_pd/BUILD

@@ -19,6 +19,7 @@ go_library(
     deps = [
         "//pkg/cloudprovider:go_default_library",
         "//pkg/cloudprovider/providers/gce:go_default_library",
+        "//pkg/features:go_default_library",
         "//pkg/kubelet/apis:go_default_library",
         "//pkg/util/mount:go_default_library",
         "//pkg/util/strings:go_default_library",
@@ -31,6 +32,7 @@ go_library(
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
+        "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library",
         "//vendor/k8s.io/utils/exec:go_default_library",
     ],
 )

pkg/volume/gce_pd/gce_util.go

@@ -26,8 +26,10 @@ import (
 	"github.com/golang/glog"
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/kubernetes/pkg/cloudprovider"
 	gcecloud "k8s.io/kubernetes/pkg/cloudprovider/providers/gce"
+	"k8s.io/kubernetes/pkg/features"
 	kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis"
 	"k8s.io/kubernetes/pkg/volume"
 	volumeutil "k8s.io/kubernetes/pkg/volume/util"
@@ -109,6 +111,11 @@ func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner) (strin
 			zonesPresent = true
 			configuredZones = v
 		case "replication-type":
+			if !utilfeature.DefaultFeatureGate.Enabled(features.GCERegionalPersistentDisk) {
+				return "", 0, nil, "",
+					fmt.Errorf("the %q option for volume plugin %v is only supported with the %q Kubernetes feature gate enabled",
+						k, c.plugin.GetPluginName(), features.GCERegionalPersistentDisk)
+			}
 			replicationType = strings.ToLower(v)
 		case volume.VolumeParameterFSType:
 			fstype = v

test/e2e/e2e.go

@@ -74,7 +74,6 @@ func setupProviderConfig() error {
 
 		gceAlphaFeatureGate, err := gcecloud.NewAlphaFeatureGate([]string{
 			gcecloud.AlphaFeatureNetworkEndpointGroup,
-			gcecloud.AlphaFeatureGCEDisk,
 		})
 		if err != nil {
 			glog.Errorf("Encountered error for creating alpha feature gate: %v", err)