diff --git a/docs/releases/1.18-NOTES.md b/docs/releases/1.18-NOTES.md index cf5acfc846382..5945d03806319 100644 --- a/docs/releases/1.18-NOTES.md +++ b/docs/releases/1.18-NOTES.md @@ -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. diff --git a/pkg/apis/kops/validation/legacy.go b/pkg/apis/kops/validation/legacy.go index ba82b10c5ab63..c67de9ff07b86 100644 --- a/pkg/apis/kops/validation/legacy.go +++ b/pkg/apis/kops/validation/legacy.go @@ -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 @@ -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 @@ -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 { diff --git a/pkg/apis/kops/validation/validation.go b/pkg/apis/kops/validation/validation.go index 4efe43df8d149..cb9cf371b6aa3 100644 --- a/pkg/apis/kops/validation/validation.go +++ b/pkg/apis/kops/validation/validation.go @@ -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" @@ -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) { @@ -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"))...) @@ -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)...) } } @@ -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 { diff --git a/pkg/apis/kops/validation/validation_test.go b/pkg/apis/kops/validation/validation_test.go index 9071f5a9d3d75..2f7669bf91f21 100644 --- a/pkg/apis/kops/validation/validation_test.go +++ b/pkg/apis/kops/validation/validation_test.go @@ -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) } }