Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Availability set deprecation #1025

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
**/dev
/bin
hack/tools/bin
/.run

*.coverprofile
*.html
Expand All @@ -21,4 +22,4 @@ TODO

# GitGuardian
.cache_ggshield
.gitguardian.yaml
.gitguardian.yaml
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
17 changes: 15 additions & 2 deletions hack/api-reference/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand Down
4 changes: 1 addition & 3 deletions pkg/admission/validator/shoot.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)...)
Expand Down Expand Up @@ -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)...)
Expand Down
14 changes: 12 additions & 2 deletions pkg/apis/azure/helper/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
33 changes: 11 additions & 22 deletions pkg/apis/azure/helper/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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),
)
})

Expand Down
6 changes: 5 additions & 1 deletion pkg/apis/azure/types_infrastructure.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion pkg/apis/azure/v1alpha1/types_infrastructure.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/azure/v1alpha1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

43 changes: 10 additions & 33 deletions pkg/apis/azure/validation/infrastructure.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"))
Expand All @@ -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 {

Expand Down Expand Up @@ -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"))
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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) {
Expand Down Expand Up @@ -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")
Expand Down
Loading
Loading