Skip to content

Commit

Permalink
Merge pull request #8826 from johngmyers/remove-legacy-etcd-provider
Browse files Browse the repository at this point in the history 
Remove support for the legacy etcd provider as of k8s 1.18
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored May 28, 2020
2 parents 9a87998 + 3f66e09 commit 4b4dbd4
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 122 deletions.
2 changes: 2 additions & 0 deletions docs/releases/1.18-NOTES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@

* Google API client libraries updated from v0.beta to v1.

* Kops does not support the "Legacy" etcd provider for Kubernetes versions 1.18 and higher. Such clusters will need to use the default "Manager" etcd provider.

# Breaking changes

* Support for Docker versions 1.11, 1.12 and 1.13 has been removed because of the [dockerproject.org shut down](https://www.docker.com/blog/changes-dockerproject-org-apt-yum-repositories/). Those affected must upgrade to a newer Docker version.
Expand Down
113 changes: 0 additions & 113 deletions pkg/apis/kops/validation/legacy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,8 @@ import (
"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/apis/kops/util"
"k8s.io/kops/pkg/featureflag"
"k8s.io/kops/pkg/model/components"
"k8s.io/kops/pkg/util/subnet"
"k8s.io/kops/upup/pkg/fi"

"github.com/blang/semver"
)

// legacy contains validation functions that don't match the apimachinery style
Expand Down Expand Up @@ -495,21 +492,6 @@ func ValidateCluster(c *kops.Cluster, strict bool) field.ErrorList {
}
}

// Etcd
{
fieldEtcdClusters := fieldSpec.Child("etcdClusters")

if len(c.Spec.EtcdClusters) == 0 {
allErrs = append(allErrs, field.Required(fieldEtcdClusters, ""))
} else {
for i, x := range c.Spec.EtcdClusters {
allErrs = append(allErrs, validateEtcdClusterSpecLegacy(x, fieldEtcdClusters.Index(i))...)
}
allErrs = append(allErrs, validateEtcdTLS(c.Spec.EtcdClusters, fieldEtcdClusters)...)
allErrs = append(allErrs, validateEtcdStorage(c.Spec.EtcdClusters, fieldEtcdClusters)...)
}
}

allErrs = append(allErrs, newValidateCluster(c)...)

return allErrs
Expand All @@ -530,101 +512,6 @@ func validateSubnetCIDR(networkCIDR *net.IPNet, additionalNetworkCIDRs []*net.IP
return false
}

// validateEtcdClusterSpecLegacy is responsible for validating the etcd cluster spec
func validateEtcdClusterSpecLegacy(spec *kops.EtcdClusterSpec, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if spec.Name == "" {
allErrs = append(allErrs, field.Required(fieldPath.Child("name"), "etcdCluster did not have name"))
}
if len(spec.Members) == 0 {
allErrs = append(allErrs, field.Required(fieldPath.Child("members"), "No members defined in etcd cluster"))
} else if (len(spec.Members) % 2) == 0 {
// Not technically a requirement, but doesn't really make sense to allow
allErrs = append(allErrs, field.Invalid(fieldPath.Child("members"), len(spec.Members), "Should be an odd number of master-zones for quorum. Use --zones and --master-zones to declare node zones and master zones separately"))
}
allErrs = append(allErrs, validateEtcdVersion(spec, fieldPath, nil)...)
for _, m := range spec.Members {
allErrs = append(allErrs, validateEtcdMemberSpec(m, fieldPath)...)
}

return allErrs
}

// validateEtcdTLS checks the TLS settings for etcd are valid
func validateEtcdTLS(specs []*kops.EtcdClusterSpec, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
var usingTLS int
for _, x := range specs {
if x.EnableEtcdTLS {
usingTLS++
}
}
// check both clusters are using tls if one is enabled
if usingTLS > 0 && usingTLS != len(specs) {
allErrs = append(allErrs, field.Forbidden(fieldPath.Index(0).Child("enableEtcdTLS"), "both etcd clusters must have TLS enabled or none at all"))
}

return allErrs
}

// validateEtcdStorage is responsible for checking versions are identical.
func validateEtcdStorage(specs []*kops.EtcdClusterSpec, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
version := specs[0].Version
for i, x := range specs {
if x.Version != "" && x.Version != version {
allErrs = append(allErrs, field.Forbidden(fieldPath.Index(i).Child("version"), fmt.Sprintf("cluster: %q, has a different storage version: %q, both must be the same", x.Name, x.Version)))
}
}

return allErrs
}

// validateEtcdVersion is responsible for validating the storage version of etcd
// @TODO semvar package doesn't appear to ignore a 'v' in v1.1.1; could be a problem later down the line
func validateEtcdVersion(spec *kops.EtcdClusterSpec, fieldPath *field.Path, minimalVersion *semver.Version) field.ErrorList {
// @check if the storage is specified that it's valid

if minimalVersion == nil {
v := semver.MustParse("0.0.0")
minimalVersion = &v
}

version := spec.Version
if spec.Version == "" {
version = components.DefaultEtcd2Version
}

sem, err := semver.Parse(strings.TrimPrefix(version, "v"))
if err != nil {
return field.ErrorList{field.Invalid(fieldPath.Child("version"), version, "the storage version is invalid")}
}

// we only support v3 and v2 for now
if sem.Major == 3 || sem.Major == 2 {
if sem.LT(*minimalVersion) {
return field.ErrorList{field.Invalid(fieldPath.Child("version"), version, fmt.Sprintf("minimum version required is %s", minimalVersion.String()))}
}
return nil
}

return field.ErrorList{field.Invalid(fieldPath.Child("version"), version, "unsupported storage version, we only support major versions 2 and 3")}
}

// validateEtcdMemberSpec is responsible for validate the cluster member
func validateEtcdMemberSpec(spec *kops.EtcdMemberSpec, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if spec.Name == "" {
allErrs = append(allErrs, field.Required(fieldPath.Child("name"), "etcdMember did not have name"))
}

if fi.StringValue(spec.InstanceGroup) == "" {
allErrs = append(allErrs, field.Required(fieldPath.Child("instanceGroup"), "etcdMember did not have instanceGroup"))
}

return allErrs
}

// DeepValidate is responsible for validating the instancegroups within the cluster spec
func DeepValidate(c *kops.Cluster, groups []*kops.InstanceGroup, strict bool) error {
if errs := ValidateCluster(c, strict); len(errs) != 0 {
Expand Down
116 changes: 108 additions & 8 deletions pkg/apis/kops/validation/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

"github.com/blang/semver"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/kops/upup/pkg/fi"

"k8s.io/apimachinery/pkg/api/validation"
utilnet "k8s.io/apimachinery/pkg/util/net"
Expand All @@ -35,7 +36,7 @@ import (

func newValidateCluster(cluster *kops.Cluster) field.ErrorList {
allErrs := validation.ValidateObjectMeta(&cluster.ObjectMeta, false, validation.NameIsDNSSubdomain, field.NewPath("metadata"))
allErrs = append(allErrs, validateClusterSpec(&cluster.Spec, field.NewPath("spec"))...)
allErrs = append(allErrs, validateClusterSpec(&cluster.Spec, cluster, field.NewPath("spec"))...)

// Additional cloud-specific validation rules
switch kops.CloudProviderID(cluster.Spec.CloudProvider) {
Expand All @@ -48,7 +49,7 @@ func newValidateCluster(cluster *kops.Cluster) field.ErrorList {
return allErrs
}

func validateClusterSpec(spec *kops.ClusterSpec, fieldPath *field.Path) field.ErrorList {
func validateClusterSpec(spec *kops.ClusterSpec, c *kops.Cluster, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

allErrs = append(allErrs, validateSubnets(spec.Subnets, fieldPath.Child("subnets"))...)
Expand Down Expand Up @@ -104,8 +105,16 @@ func validateClusterSpec(spec *kops.ClusterSpec, fieldPath *field.Path) field.Er

// EtcdClusters
{
for i, etcdCluster := range spec.EtcdClusters {
allErrs = append(allErrs, validateEtcdClusterSpec(etcdCluster, fieldPath.Child("etcdClusters").Index(i))...)
fieldEtcdClusters := fieldPath.Child("etcdClusters")

if len(spec.EtcdClusters) == 0 {
allErrs = append(allErrs, field.Required(fieldEtcdClusters, ""))
} else {
for i, etcdCluster := range spec.EtcdClusters {
allErrs = append(allErrs, validateEtcdClusterSpec(etcdCluster, c, fieldEtcdClusters.Index(i))...)
}
allErrs = append(allErrs, validateEtcdTLS(spec.EtcdClusters, fieldEtcdClusters)...)
allErrs = append(allErrs, validateEtcdStorage(spec.EtcdClusters, fieldEtcdClusters)...)
}
}

Expand Down Expand Up @@ -539,15 +548,106 @@ func validateAdditionalPolicy(role string, policy string, fldPath *field.Path) f
return errs
}

func validateEtcdClusterSpec(spec *kops.EtcdClusterSpec, fieldPath *field.Path) field.ErrorList {
errs := field.ErrorList{}
func validateEtcdClusterSpec(spec *kops.EtcdClusterSpec, c *kops.Cluster, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if spec.Name == "" {
allErrs = append(allErrs, field.Required(fieldPath.Child("name"), "etcdCluster did not have name"))
}
if spec.Provider != "" {
value := string(spec.Provider)
errs = append(errs, IsValidValue(fieldPath.Child("provider"), &value, kops.SupportedEtcdProviderTypes)...)
allErrs = append(allErrs, IsValidValue(fieldPath.Child("provider"), &value, kops.SupportedEtcdProviderTypes)...)
if spec.Provider == kops.EtcdProviderTypeLegacy && c.IsKubernetesGTE("1.18") {
allErrs = append(allErrs, field.Forbidden(fieldPath.Child("provider"), "support for Legacy mode removed as of Kubernetes 1.18"))
}
}
if len(spec.Members) == 0 {
allErrs = append(allErrs, field.Required(fieldPath.Child("etcdMembers"), "No members defined in etcd cluster"))
} else if (len(spec.Members) % 2) == 0 {
// Not technically a requirement, but doesn't really make sense to allow
allErrs = append(allErrs, field.Invalid(fieldPath.Child("etcdMembers"), len(spec.Members), "Should be an odd number of master-zones for quorum. Use --zones and --master-zones to declare node zones and master zones separately"))
}
allErrs = append(allErrs, validateEtcdVersion(spec, fieldPath, nil)...)
for i, m := range spec.Members {
allErrs = append(allErrs, validateEtcdMemberSpec(m, fieldPath.Child("etcdMembers").Index(i))...)
}

return errs
return allErrs
}

// validateEtcdTLS checks the TLS settings for etcd are valid
func validateEtcdTLS(specs []*kops.EtcdClusterSpec, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
var usingTLS int
for _, x := range specs {
if x.EnableEtcdTLS {
usingTLS++
}
}
// check both clusters are using tls if one is enabled
if usingTLS > 0 && usingTLS != len(specs) {
allErrs = append(allErrs, field.Forbidden(fieldPath.Index(0).Child("enableEtcdTLS"), "both etcd clusters must have TLS enabled or none at all"))
}

return allErrs
}

// validateEtcdStorage is responsible for checking versions are identical.
func validateEtcdStorage(specs []*kops.EtcdClusterSpec, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
version := specs[0].Version
for i, x := range specs {
if x.Version != "" && x.Version != version {
allErrs = append(allErrs, field.Forbidden(fieldPath.Index(i).Child("version"), fmt.Sprintf("cluster: %q, has a different storage version: %q, both must be the same", x.Name, x.Version)))
}
}

return allErrs
}

// validateEtcdVersion is responsible for validating the storage version of etcd
// @TODO semvar package doesn't appear to ignore a 'v' in v1.1.1; could be a problem later down the line
func validateEtcdVersion(spec *kops.EtcdClusterSpec, fieldPath *field.Path, minimalVersion *semver.Version) field.ErrorList {
// @check if the storage is specified that it's valid

if minimalVersion == nil {
v := semver.MustParse("0.0.0")
minimalVersion = &v
}

version := spec.Version
if spec.Version == "" {
version = components.DefaultEtcd2Version
}

sem, err := semver.Parse(strings.TrimPrefix(version, "v"))
if err != nil {
return field.ErrorList{field.Invalid(fieldPath.Child("version"), version, "the storage version is invalid")}
}

// we only support v3 and v2 for now
if sem.Major == 3 || sem.Major == 2 {
if sem.LT(*minimalVersion) {
return field.ErrorList{field.Invalid(fieldPath.Child("version"), version, fmt.Sprintf("minimum version required is %s", minimalVersion.String()))}
}
return nil
}

return field.ErrorList{field.Invalid(fieldPath.Child("version"), version, "unsupported storage version, we only support major versions 2 and 3")}
}

// validateEtcdMemberSpec is responsible for validate the cluster member
func validateEtcdMemberSpec(spec *kops.EtcdMemberSpec, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if spec.Name == "" {
allErrs = append(allErrs, field.Required(fieldPath.Child("name"), "etcdMember did not have name"))
}

if fi.StringValue(spec.InstanceGroup) == "" {
allErrs = append(allErrs, field.Required(fieldPath.Child("instanceGroup"), "etcdMember did not have instanceGroup"))
}

return allErrs
}

func ValidateEtcdVersionForCalicoV3(e *kops.EtcdClusterSpec, majorVersion string, fldPath *field.Path) field.ErrorList {
Expand Down
14 changes: 13 additions & 1 deletion pkg/apis/kops/validation/validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,12 +332,24 @@ func Test_Validate_AdditionalPolicies(t *testing.T) {
}
for _, g := range grid {
clusterSpec := &kops.ClusterSpec{
KubernetesVersion: "1.17.0",
AdditionalPolicies: &g.Input,
Subnets: []kops.ClusterSubnetSpec{
{Name: "subnet1"},
},
EtcdClusters: []*kops.EtcdClusterSpec{
{
Name: "main",
Members: []*kops.EtcdMemberSpec{
{
Name: "us-test-1a",
InstanceGroup: fi.String("master-us-test-1a"),
},
},
},
},
}
errs := validateClusterSpec(clusterSpec, field.NewPath("spec"))
errs := validateClusterSpec(clusterSpec, &kops.Cluster{Spec: *clusterSpec}, field.NewPath("spec"))
testErrors(t, g.Input, errs, g.ExpectedErrors)
}
}
Expand Down

0 comments on commit 4b4dbd4

Please sign in to comment.