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.

Sign up for GitHub

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

[release-v1.19] Remove version constraints for lb config #292

Merged
merged 1 commit into from
May 27, 2021
Merged

[release-v1.19] Remove version constraints for lb config #292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 0 additions & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ go 1.16

require (
github.com/Masterminds/goutils v1.1.1 // indirect
github.com/Masterminds/semver v1.5.0
github.com/ahmetb/gen-crd-api-reference-docs v0.2.0
github.com/coreos/go-systemd/v22 v22.1.0
github.com/frankban/quicktest v1.9.0 // indirect
Expand Down
14 changes: 3 additions & 11 deletions pkg/admission/validator/shoot.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"github.com/gardener/gardener-extension-provider-openstack/pkg/openstack"
extensionswebhook "github.com/gardener/gardener/extensions/pkg/webhook"

"github.com/Masterminds/semver"
"github.com/gardener/gardener/pkg/apis/core"
gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
kutil "github.com/gardener/gardener/pkg/utils/kubernetes"
Expand Down Expand Up @@ -51,7 +50,6 @@ type validationContext struct {
cpConfig *api.ControlPlaneConfig
cloudProfile *gardencorev1beta1.CloudProfile
cloudProfileConfig *api.CloudProfileConfig
shootVersion *semver.Version
}

var (
Expand Down Expand Up @@ -109,7 +107,7 @@ func (s *shoot) validateShootCreation(ctx context.Context, shoot *core.Shoot, do
allErrs := field.ErrorList{}

allErrs = append(allErrs, openstackvalidation.ValidateInfrastructureConfigAgainstCloudProfile(nil, valContext.infraConfig, domain, valContext.shoot.Spec.Region, valContext.cloudProfileConfig, infraConfigPath)...)
allErrs = append(allErrs, openstackvalidation.ValidateControlPlaneConfigAgainstCloudProfile(nil, valContext.cpConfig, domain, valContext.shoot.Spec.Region, valContext.infraConfig.FloatingPoolName, valContext.shootVersion, valContext.cloudProfileConfig, cpConfigPath)...)
allErrs = append(allErrs, openstackvalidation.ValidateControlPlaneConfigAgainstCloudProfile(nil, valContext.cpConfig, domain, valContext.shoot.Spec.Region, valContext.infraConfig.FloatingPoolName, valContext.cloudProfileConfig, cpConfigPath)...)
allErrs = append(allErrs, s.validateShoot(valContext)...)
return allErrs.ToAggregate()
}
Expand Down Expand Up @@ -144,7 +142,7 @@ func (s *shoot) validateShootUpdate(ctx context.Context, oldShoot, shoot *core.S
oldCpConfig.Zone != cpConfig.Zone ||
!equality.Semantic.DeepEqual(oldCpConfig.LoadBalancerClasses, cpConfig.LoadBalancerClasses) ||
oldValContext.infraConfig.FloatingPoolName != valContext.infraConfig.FloatingPoolName {
allErrs = append(allErrs, openstackvalidation.ValidateControlPlaneConfigAgainstCloudProfile(oldCpConfig, cpConfig, domain, valContext.shoot.Spec.Region, valContext.infraConfig.FloatingPoolName, valContext.shootVersion, valContext.cloudProfileConfig, cpConfigPath)...)
allErrs = append(allErrs, openstackvalidation.ValidateControlPlaneConfigAgainstCloudProfile(oldCpConfig, cpConfig, domain, valContext.shoot.Spec.Region, valContext.infraConfig.FloatingPoolName, valContext.cloudProfileConfig, cpConfigPath)...)
}

if errList := openstackvalidation.ValidateWorkersUpdate(oldValContext.shoot.Spec.Provider.Workers, valContext.shoot.Spec.Provider.Workers, workersPath); len(errList) > 0 {
Expand All @@ -159,7 +157,7 @@ func (s *shoot) validateShoot(context *validationContext) field.ErrorList {
allErrs := field.ErrorList{}
allErrs = append(allErrs, openstackvalidation.ValidateNetworking(context.shoot.Spec.Networking, nwPath)...)
allErrs = append(allErrs, openstackvalidation.ValidateInfrastructureConfig(context.infraConfig, context.shoot.Spec.Networking.Nodes, infraConfigPath)...)
allErrs = append(allErrs, openstackvalidation.ValidateControlPlaneConfig(context.cpConfig, context.shootVersion, cpConfigPath)...)
allErrs = append(allErrs, openstackvalidation.ValidateControlPlaneConfig(context.cpConfig, cpConfigPath)...)
allErrs = append(allErrs, openstackvalidation.ValidateWorkers(context.shoot.Spec.Provider.Workers, context.cloudProfileConfig, workersPath)...)
return allErrs
}
Expand Down Expand Up @@ -194,17 +192,11 @@ func newValidationContext(ctx context.Context, decoder runtime.Decoder, c client
return nil, fmt.Errorf("an error occurred while reading the cloud profile %q: %v", cloudProfile.Name, err)
}

k8sVersion, err := semver.NewVersion(shoot.Spec.Kubernetes.Version)
if err != nil {
return nil, err
}

return &validationContext{
shoot: shoot,
infraConfig: infraConfig,
cpConfig: cpConfig,
cloudProfile: cloudProfile,
cloudProfileConfig: cloudProfileConfig,
shootVersion: k8sVersion,
}, nil
}
39 changes: 0 additions & 39 deletions pkg/apis/openstack/helper/helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,8 @@ import (

api "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack"
"github.com/gardener/gardener-extension-provider-openstack/pkg/utils"
extensionscontroller "github.com/gardener/gardener/extensions/pkg/controller"

"github.com/Masterminds/semver"
)

// K8sVersionV121 represents the Kubernetes v1.21.0 version.
var K8sVersionV121 = semver.MustParse("v1.21.0")

// FindSubnetByPurpose takes a list of subnets and tries to find the first entry
// whose purpose matches with the given purpose. If no such entry is found then an error will be
// returned.
Expand Down Expand Up @@ -182,36 +176,3 @@ func checkFloatingPoolCandidate(floatingPool *api.FloatingPool, floatingPoolName

return nil, 0
}

// FilterLoadBalancerClassByVersionContraints filters a given list of load balancer classes
// to consider the only usable load balancer classes with version compatible features
// based on a given kubernetes version.
func FilterLoadBalancerClassByVersionContraints(lbClasses []api.LoadBalancerClass, version *semver.Version) []api.LoadBalancerClass {
var (
usableLoadBalancerClasses = []api.LoadBalancerClass{}
lessThenK8sV121 = version.LessThan(K8sVersionV121)
)

for _, lbClass := range lbClasses {
if lessThenK8sV121 && (lbClass.FloatingSubnetName != nil || lbClass.FloatingSubnetTags != nil) {
continue
}
lbClassRef := lbClass
usableLoadBalancerClasses = append(usableLoadBalancerClasses, lbClassRef)
}

return usableLoadBalancerClasses
}

// DetermineShootVersionFromCluster receives a cluster resource and determine the
// Shoot version out of it. If it can't be determined an error will be returned.
func DetermineShootVersionFromCluster(cluster *extensionscontroller.Cluster) (*semver.Version, error) {
if cluster == nil || cluster.Shoot == nil {
return nil, fmt.Errorf("cluster or shoot object in cluster is empty")
}
version, err := semver.NewVersion(cluster.Shoot.Spec.Kubernetes.Version)
if err != nil {
return nil, err
}
return version, nil
}
59 changes: 0 additions & 59 deletions pkg/apis/openstack/helper/helper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,8 @@
package helper_test

import (
"github.com/Masterminds/semver"
api "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack"
. "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack/helper"
extensionscontroller "github.com/gardener/gardener/extensions/pkg/controller"
gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/extensions/table"
Expand Down Expand Up @@ -141,50 +138,6 @@ var _ = Describe("Helper", func() {
Entry("return fip even if there is a non-constraing fip with better score", []api.FloatingPool{{Name: "fip-*", Region: &regionName}, {Name: "fip-1", Region: &regionName, NonConstraining: pointer.BoolPtr(true)}}, "fip-1", regionName, nil, pointer.StringPtr("fip-*")),
Entry("return non-constraing fip as there is no other matching fip", []api.FloatingPool{{Name: "nofip-1", Region: &regionName}, {Name: "fip-1", Region: &regionName, NonConstraining: pointer.BoolPtr(true)}}, "fip-1", regionName, nil, pointer.StringPtr("fip-1")),
)

DescribeTable("#FilterLoadBalancerClassByVersionContraints",
func(k8sVersion string, lbClasses, expectedLbClasses []api.LoadBalancerClass) {
version, err := semver.NewVersion(k8sVersion)
Expect(err).NotTo(HaveOccurred())

filterLbClassList := FilterLoadBalancerClassByVersionContraints(lbClasses, version)

Expect(filterLbClassList).To(Equal(expectedLbClasses))
},
Entry("should return input list for k8s version >= v1.21", "v1.21.0",
[]api.LoadBalancerClass{{Name: "default", FloatingSubnetTags: pointer.StringPtr("test1,test2"), FloatingSubnetName: pointer.StringPtr("*pattern*")}},
[]api.LoadBalancerClass{{Name: "default", FloatingSubnetTags: pointer.StringPtr("test1,test2"), FloatingSubnetName: pointer.StringPtr("*pattern*")}},
),
Entry("should return input list for k8s version < v1.21 as there are no entries with unsupporeted fields", "v1.20.0",
[]api.LoadBalancerClass{{Name: "default", FloatingNetworkID: pointer.StringPtr("fip-1")}},
[]api.LoadBalancerClass{{Name: "default", FloatingNetworkID: pointer.StringPtr("fip-1")}},
),
Entry("should return empty list for k8s version < v1.21 as entries with not supported field floatingSubnetName are filtered", "v1.20.0",
[]api.LoadBalancerClass{
{Name: "default", FloatingSubnetName: pointer.StringPtr("*pattern*")}},
[]api.LoadBalancerClass{},
),
Entry("should return empty list for k8s version < v1.21 as entries with not supported field floatingSubnetTags are filtered", "v1.20.0",
[]api.LoadBalancerClass{{Name: "default", FloatingSubnetTags: pointer.StringPtr("test1,test2")}},
[]api.LoadBalancerClass{},
),
)

DescribeTable("DetermineShootVersionFromCluster",
func(cluster *extensionscontroller.Cluster, expectError bool, expectedVersion string) {
version, err := DetermineShootVersionFromCluster(cluster)
if expectError {
Expect(err).To(HaveOccurred())
return
}
Expect(err).NotTo(HaveOccurred())
Expect(version.String()).To(Equal(expectedVersion))
},
Entry("should return shoot version", makeCluster("v1.20.0"), false, "1.20.0"),
Entry("should return error as shoot verison is invalid", makeCluster("x.x.x"), true, ""),
Entry("should return error as cluster resource is empty", nil, true, ""),
Entry("should return error as cluster shoot resource is empty", &extensionscontroller.Cluster{}, true, ""),
)
})

func makeProfileMachineImages(name, version, image string) []api.MachineImages {
Expand Down Expand Up @@ -226,18 +179,6 @@ func makeProfileRegionMachineImages(name, version, image, region string) []api.M
}
}

func makeCluster(version string) *extensionscontroller.Cluster {
return &extensionscontroller.Cluster{
Shoot: &gardencorev1beta1.Shoot{
Spec: gardencorev1beta1.ShootSpec{
Kubernetes: gardencorev1beta1.Kubernetes{
Version: version,
},
},
},
}
}

func expectResults(result, expected interface{}, err error, expectErr bool) {
if !expectErr {
Expect(result).To(Equal(expected))
Expand Down
23 changes: 5 additions & 18 deletions pkg/apis/openstack/validation/controlplane.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,13 @@ import (
"fmt"

api "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack"
"github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack/helper"

"github.com/Masterminds/semver"
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/util/validation/field"
)

// ValidateControlPlaneConfig validates a ControlPlaneConfig object.
func ValidateControlPlaneConfig(controlPlaneConfig *api.ControlPlaneConfig, k8sVersion *semver.Version, fldPath *field.Path) field.ErrorList {
func ValidateControlPlaneConfig(controlPlaneConfig *api.ControlPlaneConfig, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if len(controlPlaneConfig.LoadBalancerProvider) == 0 {
Expand All @@ -37,16 +35,6 @@ func ValidateControlPlaneConfig(controlPlaneConfig *api.ControlPlaneConfig, k8sV
lbClassPath := fldPath.Child("loadBalancerClasses").Index(i)
allErrs = append(allErrs, ValidateLoadBalancerClasses(class, lbClassPath)...)

// Selection of the floating subnet via name or by tags is only possible for k8s clusters >= v1.21
if k8sVersion.LessThan(helper.K8sVersionV121) {
if class.FloatingSubnetName != nil {
allErrs = append(allErrs, field.Forbidden(lbClassPath.Child("floatingSubnetName"), "cannot select floating subnet by name for clusters below k8s < v1.21"))
}
if class.FloatingSubnetTags != nil {
allErrs = append(allErrs, field.Forbidden(lbClassPath.Child("floatingSubnetTags"), "cannot select floating subnet by tags for clusters below k8s < v1.21"))
}
}

// Do not allow that the user specify a vpn LoadBalancerClass in the controlplane config.
// It need to come from the CloudProfile.
if class.Purpose != nil && *class.Purpose == api.VPNLoadBalancerClass {
Expand All @@ -66,7 +54,7 @@ func ValidateControlPlaneConfigUpdate(oldConfig, newConfig *api.ControlPlaneConf
}

// ValidateControlPlaneConfigAgainstCloudProfile validates the given ControlPlaneConfig against constraints in the given CloudProfile.
func ValidateControlPlaneConfigAgainstCloudProfile(oldCpConfig, cpConfig *api.ControlPlaneConfig, domain, shootRegion, floatingPoolName string, shootVersion *semver.Version, cloudProfileConfig *api.CloudProfileConfig, fldPath *field.Path) field.ErrorList {
func ValidateControlPlaneConfigAgainstCloudProfile(oldCpConfig, cpConfig *api.ControlPlaneConfig, domain, shootRegion, floatingPoolName string, cloudProfileConfig *api.CloudProfileConfig, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if oldCpConfig == nil || oldCpConfig.LoadBalancerProvider != cpConfig.LoadBalancerProvider {
Expand All @@ -76,7 +64,7 @@ func ValidateControlPlaneConfigAgainstCloudProfile(oldCpConfig, cpConfig *api.Co
}

if oldCpConfig == nil || !equality.Semantic.DeepEqual(oldCpConfig.LoadBalancerClasses, cpConfig.LoadBalancerClasses) {
allErrs = append(allErrs, validateLoadBalancerClassesConstraints(cloudProfileConfig.Constraints.FloatingPools, cpConfig.LoadBalancerClasses, domain, shootRegion, floatingPoolName, shootVersion, fldPath.Child("loadBalancerClasses"))...)
allErrs = append(allErrs, validateLoadBalancerClassesConstraints(cloudProfileConfig.Constraints.FloatingPools, cpConfig.LoadBalancerClasses, domain, shootRegion, floatingPoolName, fldPath.Child("loadBalancerClasses"))...)
}

return allErrs
Expand Down Expand Up @@ -118,7 +106,7 @@ func validateLoadBalancerProviderConstraints(providers []api.LoadBalancerProvide
return false, validValues
}

func validateLoadBalancerClassesConstraints(floatingPools []api.FloatingPool, shootLBClasses []api.LoadBalancerClass, domain, shootRegion, floatingPoolName string, shootVersion *semver.Version, fldPath *field.Path) field.ErrorList {
func validateLoadBalancerClassesConstraints(floatingPools []api.FloatingPool, shootLBClasses []api.LoadBalancerClass, domain, shootRegion, floatingPoolName string, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if len(shootLBClasses) == 0 {
return allErrs
Expand All @@ -134,14 +122,13 @@ func validateLoadBalancerClassesConstraints(floatingPools []api.FloatingPool, sh
return allErrs
}

usableFloatingPoolLbClasses := helper.FilterLoadBalancerClassByVersionContraints(fp.LoadBalancerClasses, shootVersion)
for i, shootLBClass := range shootLBClasses {
var (
valid bool
validValues []string
)

for _, lbClass := range usableFloatingPoolLbClasses {
for _, lbClass := range fp.LoadBalancerClasses {
if equality.Semantic.DeepEqual(shootLBClass, lbClass) {
valid = true
break
Expand Down
Loading