Availability set deprecation (#1025)

* implement migration

* fix ccm not found error handling

* redundant checks and consts for strings

* cosmetics

.gitignore

@@ -5,6 +5,7 @@
 **/dev
 /bin
 hack/tools/bin
+/.run
 
 *.coverprofile
 *.html
@@ -21,4 +22,4 @@ TODO
 
 # GitGuardian
 .cache_ggshield
-.gitguardian.yaml
+.gitguardian.yaml

Makefile

@@ -79,6 +79,7 @@ start-admission:
 		./cmd/$(EXTENSION_PREFIX)-$(ADMISSION_NAME) \
 		--webhook-config-server-host=0.0.0.0 \
 		--webhook-config-server-port=$(WEBHOOK_CONFIG_PORT) \
+		--leader-election-namespace=garden \
         --webhook-config-mode=$(WEBHOOK_CONFIG_MODE) \
         $(WEBHOOK_PARAM)
 

hack/api-reference/api.md

@@ -1021,7 +1021,20 @@ ResourceGroup
 </em>
 </td>
 <td>
-<p>AvailabilitySets is a list of created availability sets</p>
+<p>AvailabilitySets is a list of created availability sets
+Deprecated: Will be removed in future versions.</p>
+</td>
+</tr>
+<tr>
+<td>
+<code>migratingToVMO</code></br>
+<em>
+bool
+</em>
+</td>
+<td>
+<p>MigratingToVMO indicates whether the infrastructure controller has prepared the migration from Availability set.
+Deprecated: Will be removed in future versions.</p>
 </td>
 </tr>
 <tr>
@@ -1034,7 +1047,7 @@ ResourceGroup
 </em>
 </td>
 <td>
-<p>AvailabilitySets is a list of created route tables</p>
+<p>RouteTables is a list of created route tables</p>
 </td>
 </tr>
 <tr>

pkg/admission/validator/shoot.go

@@ -21,7 +21,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 
 	api "github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure"
-	"github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure/helper"
 	azurevalidation "github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure/validation"
 )
 
@@ -114,7 +113,7 @@ func (s *shoot) validateShoot(shoot *core.Shoot, oldInfraConfig, infraConfig *ap
 		// Cloudprofile validation
 		allErrs = append(allErrs, azurevalidation.ValidateInfrastructureConfigAgainstCloudProfile(oldInfraConfig, infraConfig, shoot.Spec.Region, cloudProfileSpec, infraConfigPath)...)
 		// Provider validation
-		allErrs = append(allErrs, azurevalidation.ValidateInfrastructureConfig(infraConfig, shoot.Spec.Networking, helper.HasShootVmoAlphaAnnotation(shoot.Annotations), infraConfigPath)...)
+		allErrs = append(allErrs, azurevalidation.ValidateInfrastructureConfig(infraConfig, shoot, infraConfigPath)...)
 	}
 	if cpConfig != nil {
 		allErrs = append(allErrs, azurevalidation.ValidateControlPlaneConfig(cpConfig, shoot.Spec.Kubernetes.Version, cpConfigPath)...)
@@ -169,7 +168,6 @@ func (s *shoot) validateUpdate(oldShoot, shoot *core.Shoot, cloudProfileSpec *ga
 		allErrs = append(allErrs, azurevalidation.ValidateInfrastructureConfigUpdate(oldInfraConfig, infraConfig, metaDataPath)...)
 	}
 
-	allErrs = append(allErrs, azurevalidation.ValidateVmoConfigUpdate(helper.HasShootVmoAlphaAnnotation(oldShoot.Annotations), helper.HasShootVmoAlphaAnnotation(shoot.Annotations), metaDataPath)...)
 	allErrs = append(allErrs, azurevalidation.ValidateWorkersUpdate(oldShoot.Spec.Provider.Workers, shoot.Spec.Provider.Workers, workersPath)...)
 
 	allErrs = append(allErrs, s.validateShoot(shoot, oldInfraConfig, infraConfig, cloudProfileSpec, cpConfig)...)

pkg/apis/azure/helper/helper.go

@@ -125,9 +125,19 @@ func FindImageFromCloudProfile(cloudProfileConfig *api.CloudProfileConfig, image
 	return nil, fmt.Errorf("no machine image found with name %q, architecture %q and version %q", imageName, *architecture, imageVersion)
 }
 
-// IsVmoRequired determines if VMO is required.
+// IsVmoRequired determines if VMO is required. It is different from the condition in the infrastructure as this one depends on whether the infra controller
+// has finished migrating the Availability sets.
 func IsVmoRequired(infrastructureStatus *api.InfrastructureStatus) bool {
-	return !infrastructureStatus.Zoned && len(infrastructureStatus.AvailabilitySets) == 0
+	return !infrastructureStatus.Zoned && (len(infrastructureStatus.AvailabilitySets) == 0 || infrastructureStatus.MigratingToVMO)
+}
+
+// HasShootVmoMigrationAnnotation determines if the passed Shoot annotations contain instruction to use VMO.
+func HasShootVmoMigrationAnnotation(shootAnnotations map[string]string) bool {
+	value, exists := shootAnnotations[azure.ShootVmoMigrationAnnotation]
+	if exists && value == "true" {
+		return true
+	}
+	return false
 }
 
 // HasShootVmoAlphaAnnotation determines if the passed Shoot annotations contain instruction to use VMO.

pkg/apis/azure/helper/helper_test.go

@@ -11,7 +11,6 @@ import (
 
 	api "github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure"
 	. "github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure/helper"
-	"github.com/gardener/gardener-extension-provider-azure/pkg/azure"
 )
 
 var (
@@ -151,40 +150,30 @@ var _ = Describe("Helper", func() {
 			"ubuntu", "1", ptr.To("foo"), &api.MachineImage{Name: "ubuntu", Version: "1", Image: api.Image{SharedGalleryImageID: &profileSharedImageId}, Architecture: ptr.To("foo")}),
 	)
 
-	DescribeTable("#IsVmoRequired",
-		func(zoned bool, availabilitySet *api.AvailabilitySet, expectedVmoRequired bool) {
+	DescribeTable("#IsVmoRequiredForInfrastructure",
+		func(zoned bool, availabilitySet *api.AvailabilitySet, migrateToVMO bool, expectedVmoRequired bool) {
 			var infrastructureStatus = &api.InfrastructureStatus{
 				Zoned: zoned,
 			}
 			if availabilitySet != nil {
 				infrastructureStatus.AvailabilitySets = append(infrastructureStatus.AvailabilitySets, *availabilitySet)
 			}
+			infrastructureStatus.MigratingToVMO = migrateToVMO
 
 			Expect(IsVmoRequired(infrastructureStatus)).To(Equal(expectedVmoRequired))
 		},
-		Entry("should require a VMO", false, nil, true),
-		Entry("should not require VMO for zoned cluster", true, nil, false),
+		Entry("should require a VMO", false, nil, false, true),
+		Entry("should not require VMO for zoned cluster", true, nil, false, false),
 		Entry("should not require VMO for a cluster with primary availabilityset (non zoned)", false, &api.AvailabilitySet{
 			ID:      "/my/azure/availabilityset/id",
 			Name:    "my-availabilityset",
 			Purpose: api.PurposeNodes,
-		}, false),
-	)
-
-	DescribeTable("#HasShootVmoAlphaAnnotation",
-		func(hasVmoAnnotaion, hasCorrectVmoAnnotationValue, expectedResult bool) {
-			var annotations = map[string]string{}
-			if hasVmoAnnotaion {
-				annotations[azure.ShootVmoUsageAnnotation] = "some-arbitrary-value"
-			}
-			if hasCorrectVmoAnnotationValue {
-				annotations[azure.ShootVmoUsageAnnotation] = "true"
-			}
-			Expect(HasShootVmoAlphaAnnotation(annotations)).To(Equal(expectedResult))
-		},
-		Entry("should return true as shoot annotations contain vmo alpha annotation with value true", true, true, true),
-		Entry("should return false as shoot annotations contain vmo alpha annotation with wrong value", true, false, false),
-		Entry("should return false as shoot annotations do not contain vmo alpha annotation", false, false, false),
+		}, false, false),
+		Entry("should require VMO for a cluster with primary availabilityset with migration to VMO", false, &api.AvailabilitySet{
+			ID:      "/my/azure/availabilityset/id",
+			Name:    "my-availabilityset",
+			Purpose: api.PurposeNodes,
+		}, true, true),
 	)
 })
 

pkg/apis/azure/types_infrastructure.go

@@ -105,8 +105,12 @@ type InfrastructureStatus struct {
 	// ResourceGroup is azure resource group
 	ResourceGroup ResourceGroup
 	// AvailabilitySets is a list of created availability sets
+	// Deprecated: Will be removed in future versions.
 	AvailabilitySets []AvailabilitySet
-	// AvailabilitySets is a list of created route tables
+	// MigratingToVMO indicates whether the infrastructure controller has prepared the migration from Availability set.
+	// Deprecated: Will be removed in future versions.
+	MigratingToVMO bool
+	// RouteTables is a list of created route tables
 	RouteTables []RouteTable
 	// SecurityGroups is a list of created security groups
 	SecurityGroups []SecurityGroup

pkg/apis/azure/v1alpha1/types_infrastructure.go

@@ -118,8 +118,12 @@ type InfrastructureStatus struct {
 	// ResourceGroup is azure resource group
 	ResourceGroup ResourceGroup `json:"resourceGroup"`
 	// AvailabilitySets is a list of created availability sets
+	// Deprecated: Will be removed in future versions.
 	AvailabilitySets []AvailabilitySet `json:"availabilitySets"`
-	// AvailabilitySets is a list of created route tables
+	// MigratingToVMO indicates whether the infrastructure controller has prepared the migration from Availability set.
+	// Deprecated: Will be removed in future versions.
+	MigratingToVMO bool `json:"migratingToVMO,omitempty"`
+	// RouteTables is a list of created route tables
 	RouteTables []RouteTable `json:"routeTables"`
 	// SecurityGroups is a list of created security groups
 	SecurityGroups []SecurityGroup `json:"securityGroups"`

pkg/apis/azure/validation/infrastructure.go

@@ -16,7 +16,7 @@ import (
 
 	apisazure "github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure"
 	"github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure/helper"
-	"github.com/gardener/gardener-extension-provider-azure/pkg/azure"
+	azuretypes "github.com/gardener/gardener-extension-provider-azure/pkg/azure"
 )
 
 const (
@@ -75,8 +75,9 @@ func validateInfrastructureConfigZones(oldInfra, infra *apisazure.Infrastructure
 }
 
 // ValidateInfrastructureConfig validates a InfrastructureConfig object.
-func ValidateInfrastructureConfig(infra *apisazure.InfrastructureConfig, networking *core.Networking, hasVmoAlphaAnnotation bool, fldPath *field.Path) field.ErrorList {
+func ValidateInfrastructureConfig(infra *apisazure.InfrastructureConfig, shoot *core.Shoot, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
+	networking := shoot.Spec.Networking
 
 	var (
 		nodes, pods, services             cidrvalidation.CIDR
@@ -105,11 +106,7 @@ func ValidateInfrastructureConfig(infra *apisazure.InfrastructureConfig, network
 		allErrs = append(allErrs, field.Invalid(fldPath.Child("resourceGroup"), infra.ResourceGroup, "specifying an existing resource group is not supported yet"))
 	}
 
-	if infra.Zoned && hasVmoAlphaAnnotation {
-		allErrs = append(allErrs, field.Invalid(fldPath.Child("zoned"), infra.Zoned, fmt.Sprintf("specifying a zoned cluster and having the %q annotation is not allowed", azure.ShootVmoUsageAnnotation)))
-	}
-
-	allErrs = append(allErrs, validateNetworkConfig(infra, nodes, pods, services, hasVmoAlphaAnnotation, fldPath)...)
+	allErrs = append(allErrs, validateNetworkConfig(shoot, infra, nodes, pods, services, fldPath)...)
 
 	if infra.Identity != nil && (infra.Identity.Name == "" || infra.Identity.ResourceGroup == "") {
 		allErrs = append(allErrs, field.Invalid(fldPath.Child("identity"), infra.Identity, "specifying an identity requires the name of the identity and the resource group which hosts the identity"))
@@ -119,11 +116,11 @@ func ValidateInfrastructureConfig(infra *apisazure.InfrastructureConfig, network
 }
 
 func validateNetworkConfig(
+	shoot *core.Shoot,
 	infra *apisazure.InfrastructureConfig,
 	nodes cidrvalidation.CIDR,
 	pods cidrvalidation.CIDR,
 	services cidrvalidation.CIDR,
-	hasVmoAlphaAnnotation bool,
 	fldPath *field.Path,
 ) field.ErrorList {
 
@@ -164,18 +161,18 @@ func validateNetworkConfig(
 			allErrs = append(allErrs, nodes.ValidateSubset(workerCIDR)...)
 		}
 
-		allErrs = append(allErrs, validateNatGatewayConfig(config.NatGateway, infra.Zoned, hasVmoAlphaAnnotation, networksPath.Child("natGateway"))...)
+		allErrs = append(allErrs, validateNatGatewayConfig(config.NatGateway, helper.HasShootVmoMigrationAnnotation(shoot.GetAnnotations()), networksPath.Child("natGateway"))...)
 		return allErrs
 	}
 
 	// handle multiple subnet layout validation.
 	if !infra.Zoned {
 		allErrs = append(allErrs, field.Forbidden(zonesPath, "cannot specify zones in an non-zonal cluster"))
 	}
-
 	if config.NatGateway != nil {
 		allErrs = append(allErrs, field.Forbidden(workersPath, "natGateway cannot be specified when workers field is missing"))
 	}
+
 	if len(config.ServiceEndpoints) > 0 {
 		allErrs = append(allErrs, field.Forbidden(workersPath, "serviceEndpoints cannot be specified when workers field is missing"))
 	}
@@ -282,7 +279,7 @@ func validateZones(zones []apisazure.Zone, nodes, pods, services cidrvalidation.
 	return allErrs
 }
 
-func validateNatGatewayConfig(natGatewayConfig *apisazure.NatGatewayConfig, zoned bool, hasVmoAlphaAnnotation bool, natGatewayPath *field.Path) field.ErrorList {
+func validateNatGatewayConfig(natGatewayConfig *apisazure.NatGatewayConfig, hasShootVmoMigrationAnnotation bool, natGatewayPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
 	if natGatewayConfig == nil {
@@ -296,11 +293,8 @@ func validateNatGatewayConfig(natGatewayConfig *apisazure.NatGatewayConfig, zone
 		return nil
 	}
 
-	// NatGateway cannot be offered for Shoot clusters with a primary AvailabilitySet.
-	// The NatGateway is not compatible with the Basic SKU Loadbalancers which are
-	// required to use for Shoot clusters with AvailabilitySet.
-	if !zoned && !hasVmoAlphaAnnotation {
-		return append(allErrs, field.Forbidden(natGatewayPath, "NatGateway is currently only supported for zonal and VMO clusters"))
+	if hasShootVmoMigrationAnnotation {
+		allErrs = append(allErrs, field.Forbidden(natGatewayPath.Child("enabled"), fmt.Sprintf("natGateway cannot be enabled with the annotation %s", azuretypes.ShootVmoMigrationAnnotation)))
 	}
 
 	if natGatewayConfig.IdleConnectionTimeoutMinutes != nil && (*natGatewayConfig.IdleConnectionTimeoutMinutes < natGatewayMinTimeoutInMinutes || *natGatewayConfig.IdleConnectionTimeoutMinutes > natGatewayMaxTimeoutInMinutes) {
@@ -395,23 +389,6 @@ func ValidateInfrastructureConfigUpdate(oldConfig, newConfig *apisazure.Infrastr
 	return allErrs
 }
 
-// ValidateVmoConfigUpdate validates the VMO configuration on update.
-func ValidateVmoConfigUpdate(oldShootHasAlphaVmoAnnotation, newShootHasAlphaVmoAnnotation bool, metaDataPath *field.Path) field.ErrorList {
-	allErrs := field.ErrorList{}
-
-	// Check if old shoot has not the vmo alpha annotation and forbid to add it.
-	if !oldShootHasAlphaVmoAnnotation && newShootHasAlphaVmoAnnotation {
-		allErrs = append(allErrs, field.Forbidden(metaDataPath.Child("annotations"), fmt.Sprintf("not allowed to add annotation %q to an already existing shoot cluster", azure.ShootVmoUsageAnnotation)))
-	}
-
-	// Check if old shoot has the vmo alpha annotaion and forbid to remove it.
-	if oldShootHasAlphaVmoAnnotation && !newShootHasAlphaVmoAnnotation {
-		allErrs = append(allErrs, field.Forbidden(metaDataPath.Child("annotations"), fmt.Sprintf("not allowed to remove annotation %q to an already existing shoot cluster", azure.ShootVmoUsageAnnotation)))
-	}
-
-	return allErrs
-}
-
 func validateVnetConfigUpdate(oldNeworkConfig, newNetworkConfig *apisazure.NetworkConfig, networkConfigPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 	vnetPath := networkConfigPath.Child("vnet")