Merge pull request #3176 from gambol99/etcv3

Automatic merge from submit-queue

Etcd v3 Support

Etcd V3 Support
    
The current implementation is running v2.2.1 which is two years old and end of life. This PR adds the ability to use etcd v3 and set the versions if required. Note at the moment the image is still using the gcr.io registry image and much like Etcd TLS PR there presently is no 'automated' migration path from v2 to v3.
    
- the feature is gated behind the version of the etcd cluster, both clusters events and main must use the same storage type
- the version for v2 is unchanged and pinned at v2.2.1 with v3 using v3.0.17
- @question: we should consider allowing the user to override the images though I think this should be addressed generically, than one offs here and then. I know @chrislovecnm is working on a asset registry??

nodeup/pkg/model/protokube.go

@@ -181,6 +181,7 @@ type ProtokubeFlags struct {
 	DNSInternalSuffix *string  `json:"dnsInternalSuffix,omitempty" flag:"dns-internal-suffix"`
 	DNSProvider       *string  `json:"dnsProvider,omitempty" flag:"dns"`
 	DNSServer         *string  `json:"dns-server,omitempty" flag:"dns-server"`
+	EtcdImage         *string  `json:"etcd-image,omitempty" flag:"etcd-image"`
 	InitializeRBAC    *bool    `json:"initializeRBAC,omitempty" flag:"initialize-rbac"`
 	LogLevel          *int32   `json:"logLevel,omitempty" flag:"v"`
 	Master            *bool    `json:"master,omitempty" flag:"master"`
@@ -195,9 +196,14 @@ type ProtokubeFlags struct {
 
 // ProtokubeFlags is responsible for building the command line flags for protokube
 func (t *ProtokubeBuilder) ProtokubeFlags(k8sVersion semver.Version) *ProtokubeFlags {
+	// @todo: i think we should allow the user to override the source of the image, but for now
+	// lets keep that for another PR and allow the version change
+	imageVersion := t.Cluster.Spec.EtcdClusters[0].Version
+
 	f := &ProtokubeFlags{
 		Channels:      t.NodeupConfig.Channels,
 		Containerized: fi.Bool(true),
+		EtcdImage:     s(fmt.Sprintf("gcr.io/google_containers/etcd:%s", imageVersion)),
 		LogLevel:      fi.Int32(4),
 		Master:        b(t.IsMaster),
 	}

pkg/apis/kops/cluster.go

@@ -224,24 +224,16 @@ type KubeDNSConfig struct {
 	ServerIP string `json:"serverIP,omitempty"`
 }
 
-// EtcdStorageType defined the etcd storage backend
-type EtcdStorageType string
-
-const (
-	// EtcdStorageTypeV2 is the old v2 storage
-	EtcdStorageTypeV2 EtcdStorageType = "etcd2"
-	// EtcdStorageTypeV3 is the new v3 storage
-	EtcdStorageTypeV3 EtcdStorageType = "etcd3"
-)
-
 // EtcdClusterSpec is the etcd cluster specification
 type EtcdClusterSpec struct {
 	// Name is the name of the etcd cluster (main, events etc)
 	Name string `json:"name,omitempty"`
-	// EnableEtcdTLS indicates the etcd service should use TLS between peers and clients
-	EnableEtcdTLS bool `json:"enableEtcdTLS,omitempty"`
 	// Members stores the configurations for each member of the cluster (including the data volume)
 	Members []*EtcdMemberSpec `json:"etcdMembers,omitempty"`
+	// EnableEtcdTLS indicates the etcd service should use TLS between peers and clients
+	EnableEtcdTLS bool `json:"enableEtcdTLS,omitempty"`
+	// Version is the version of etcd to run i.e. 2.1.2, 3.0.17 etcd
+	Version string `json:"version,omitempty"`
 }
 
 // EtcdMemberSpec is a specification for a etcd member

pkg/apis/kops/v1alpha1/cluster.go

@@ -327,32 +327,32 @@ type KubeDNSConfig struct {
 	ServerIP string `json:"serverIP,omitempty"`
 }
 
-//type MasterConfig struct {
-//	Name string `json:",omitempty"`
-//
-//	Image       string `json:",omitempty"`
-//	Zone        string `json:",omitempty"`
-//	MachineType string `json:",omitempty"`
-//}
-
+// EtcdClusterSpec is the etcd cluster specification
 type EtcdClusterSpec struct {
 	// Name is the name of the etcd cluster (main, events etc)
 	Name string `json:"name,omitempty"`
+	// Members stores the configurations for each member of the cluster (including the data volume)
+	Members []*EtcdMemberSpec `json:"etcdMembers,omitempty"`
 	// EnableEtcdTLS indicates the etcd service should use TLS between peers and clients
 	EnableEtcdTLS bool `json:"enableEtcdTLS,omitempty"`
-	// EtcdMember stores the configurations for each member of the cluster (including the data volume)
-	Members []*EtcdMemberSpec `json:"etcdMembers,omitempty"`
+	// Version is the version of etcd to run i.e. 2.1.2, 3.0.17 etcd
+	Version string `json:"version,omitempty"`
 }
 
+// EtcdMemberSpec is a specification for a etcd member
 type EtcdMemberSpec struct {
 	// Name is the name of the member within the etcd cluster
-	Name string  `json:"name,omitempty"`
+	Name string `json:"name,omitempty"`
+	// Zone is the zone the member lives
 	Zone *string `json:"zone,omitempty"`
-
-	VolumeType      *string `json:"volumeType,omitempty"`
-	VolumeSize      *int32  `json:"volumeSize,omitempty"`
-	KmsKeyId        *string `json:"kmsKeyId,omitempty"`
-	EncryptedVolume *bool   `json:"encryptedVolume,omitempty"`
+	// VolumeType is the underlining cloud storage class
+	VolumeType *string `json:"volumeType,omitempty"`
+	// VolumeSize is the underlining cloud volume size
+	VolumeSize *int32 `json:"volumeSize,omitempty"`
+	// KmsKeyId is a AWS KMS ID used to encrypt the volume
+	KmsKeyId *string `json:"kmsKeyId,omitempty"`
+	// EncryptedVolume indicates you want to encrypt the volume
+	EncryptedVolume *bool `json:"encryptedVolume,omitempty"`
 }
 
 type ClusterZoneSpec struct {

pkg/apis/kops/v1alpha1/zz_generated.conversion.go

@@ -996,7 +996,6 @@ func Convert_kops_EgressProxySpec_To_v1alpha1_EgressProxySpec(in *kops.EgressPro
 
 func autoConvert_v1alpha1_EtcdClusterSpec_To_kops_EtcdClusterSpec(in *EtcdClusterSpec, out *kops.EtcdClusterSpec, s conversion.Scope) error {
 	out.Name = in.Name
-	out.EnableEtcdTLS = in.EnableEtcdTLS
 	if in.Members != nil {
 		in, out := &in.Members, &out.Members
 		*out = make([]*kops.EtcdMemberSpec, len(*in))
@@ -1009,6 +1008,8 @@ func autoConvert_v1alpha1_EtcdClusterSpec_To_kops_EtcdClusterSpec(in *EtcdCluste
 	} else {
 		out.Members = nil
 	}
+	out.EnableEtcdTLS = in.EnableEtcdTLS
+	out.Version = in.Version
 	return nil
 }
 
@@ -1019,7 +1020,6 @@ func Convert_v1alpha1_EtcdClusterSpec_To_kops_EtcdClusterSpec(in *EtcdClusterSpe
 
 func autoConvert_kops_EtcdClusterSpec_To_v1alpha1_EtcdClusterSpec(in *kops.EtcdClusterSpec, out *EtcdClusterSpec, s conversion.Scope) error {
 	out.Name = in.Name
-	out.EnableEtcdTLS = in.EnableEtcdTLS
 	if in.Members != nil {
 		in, out := &in.Members, &out.Members
 		*out = make([]*EtcdMemberSpec, len(*in))
@@ -1032,6 +1032,8 @@ func autoConvert_kops_EtcdClusterSpec_To_v1alpha1_EtcdClusterSpec(in *kops.EtcdC
 	} else {
 		out.Members = nil
 	}
+	out.EnableEtcdTLS = in.EnableEtcdTLS
+	out.Version = in.Version
 	return nil
 }
 

pkg/apis/kops/v1alpha2/cluster.go

@@ -253,24 +253,32 @@ type KubeDNSConfig struct {
 	ServerIP string `json:"serverIP,omitempty"`
 }
 
+// EtcdClusterSpec is the etcd cluster specification
 type EtcdClusterSpec struct {
 	// Name is the name of the etcd cluster (main, events etc)
 	Name string `json:"name,omitempty"`
+	// Members stores the configurations for each member of the cluster (including the data volume)
+	Members []*EtcdMemberSpec `json:"etcdMembers,omitempty"`
 	// EnableEtcdTLS indicates the etcd service should use TLS between peers and clients
 	EnableEtcdTLS bool `json:"enableEtcdTLS,omitempty"`
-	// EtcdMember stores the configurations for each member of the cluster (including the data volume)
-	Members []*EtcdMemberSpec `json:"etcdMembers,omitempty"`
+	// Version is the version of etcd to run i.e. 2.1.2, 3.0.17 etcd
+	Version string `json:"version,omitempty"`
 }
 
+// EtcdMemberSpec is a specification for a etcd member
 type EtcdMemberSpec struct {
 	// Name is the name of the member within the etcd cluster
-	Name          string  `json:"name,omitempty"`
+	Name string `json:"name,omitempty"`
+	// InstanceGroup is the instanceGroup this volume is associated
 	InstanceGroup *string `json:"instanceGroup,omitempty"`
-
-	VolumeType      *string `json:"volumeType,omitempty"`
-	VolumeSize      *int32  `json:"volumeSize,omitempty"`
-	KmsKeyId        *string `json:"kmsKeyId,omitempty"`
-	EncryptedVolume *bool   `json:"encryptedVolume,omitempty"`
+	// VolumeType is the underlining cloud storage class
+	VolumeType *string `json:"volumeType,omitempty"`
+	// VolumeSize is the underlining cloud volume size
+	VolumeSize *int32 `json:"volumeSize,omitempty"`
+	// KmsKeyId is a AWS KMS ID used to encrypt the volume
+	KmsKeyId *string `json:"kmsKeyId,omitempty"`
+	// EncryptedVolume indicates you want to encrypt the volume
+	EncryptedVolume *bool `json:"encryptedVolume,omitempty"`
 }
 
 // SubnetType string describes subnet types (public, private, utility)

pkg/apis/kops/v1alpha2/zz_generated.conversion.go

@@ -1084,7 +1084,6 @@ func Convert_kops_EgressProxySpec_To_v1alpha2_EgressProxySpec(in *kops.EgressPro
 
 func autoConvert_v1alpha2_EtcdClusterSpec_To_kops_EtcdClusterSpec(in *EtcdClusterSpec, out *kops.EtcdClusterSpec, s conversion.Scope) error {
 	out.Name = in.Name
-	out.EnableEtcdTLS = in.EnableEtcdTLS
 	if in.Members != nil {
 		in, out := &in.Members, &out.Members
 		*out = make([]*kops.EtcdMemberSpec, len(*in))
@@ -1097,6 +1096,8 @@ func autoConvert_v1alpha2_EtcdClusterSpec_To_kops_EtcdClusterSpec(in *EtcdCluste
 	} else {
 		out.Members = nil
 	}
+	out.EnableEtcdTLS = in.EnableEtcdTLS
+	out.Version = in.Version
 	return nil
 }
 
@@ -1107,7 +1108,6 @@ func Convert_v1alpha2_EtcdClusterSpec_To_kops_EtcdClusterSpec(in *EtcdClusterSpe
 
 func autoConvert_kops_EtcdClusterSpec_To_v1alpha2_EtcdClusterSpec(in *kops.EtcdClusterSpec, out *EtcdClusterSpec, s conversion.Scope) error {
 	out.Name = in.Name
-	out.EnableEtcdTLS = in.EnableEtcdTLS
 	if in.Members != nil {
 		in, out := &in.Members, &out.Members
 		*out = make([]*EtcdMemberSpec, len(*in))
@@ -1120,6 +1120,8 @@ func autoConvert_kops_EtcdClusterSpec_To_v1alpha2_EtcdClusterSpec(in *kops.EtcdC
 	} else {
 		out.Members = nil
 	}
+	out.EnableEtcdTLS = in.EnableEtcdTLS
+	out.Version = in.Version
 	return nil
 }
 

pkg/apis/kops/validation/legacy.go

@@ -21,12 +21,13 @@ import (
 	"net"
 	"strings"
 
-	"github.com/blang/semver"
 	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/apis/kops/util"
 	"k8s.io/kops/upup/pkg/fi"
+
+	"github.com/blang/semver"
 )
 
 // legacy contains validation functions that don't match the apimachinery style
@@ -45,14 +46,13 @@ func ValidateCluster(c *kops.Cluster, strict bool) error {
 	// KubernetesVersion
 	if c.Spec.KubernetesVersion == "" {
 		return field.Required(specField.Child("KubernetesVersion"), "")
-	} else {
-		sv, err := util.ParseKubernetesVersion(c.Spec.KubernetesVersion)
-		if err != nil {
-			return field.Invalid(specField.Child("KubernetesVersion"), c.Spec.KubernetesVersion, "unable to determine kubernetes version")
-		}
+	}
 
-		kubernetesRelease = semver.Version{Major: sv.Major, Minor: sv.Minor}
+	sv, err := util.ParseKubernetesVersion(c.Spec.KubernetesVersion)
+	if err != nil {
+		return field.Invalid(specField.Child("KubernetesVersion"), c.Spec.KubernetesVersion, "unable to determine kubernetes version")
 	}
+	kubernetesRelease = semver.Version{Major: sv.Major, Minor: sv.Minor}
 
 	if c.ObjectMeta.Name == "" {
 		return field.Required(field.NewPath("Name"), "Cluster Name is required (e.g. --name=mycluster.myzone.com)")
@@ -373,33 +373,16 @@ func ValidateCluster(c *kops.Cluster, strict bool) error {
 		if len(c.Spec.EtcdClusters) == 0 {
 			return field.Required(specField.Child("EtcdClusters"), "")
 		}
-		var usingTLS int
-		for _, etcd := range c.Spec.EtcdClusters {
-			if etcd.Name == "" {
-				return fmt.Errorf("EtcdCluster did not have name")
-			}
-			if len(etcd.Members) == 0 {
-				return fmt.Errorf("No members defined in etcd cluster %q", etcd.Name)
-			}
-			if (len(etcd.Members) % 2) == 0 {
-				// Not technically a requirement, but doesn't really make sense to allow
-				return fmt.Errorf("There should be an odd number of master-zones, for etcd's quorum.  Hint: Use --zones and --master-zones to declare node zones and master zones separately.")
-			}
-			if etcd.EnableEtcdTLS {
-				usingTLS++
-			}
-			for _, m := range etcd.Members {
-				if m.Name == "" {
-					return fmt.Errorf("EtcdMember did not have Name in cluster %q", etcd.Name)
-				}
-				if fi.StringValue(m.InstanceGroup) == "" {
-					return fmt.Errorf("EtcdMember did not have InstanceGroup in cluster %q", etcd.Name)
-				}
+		for _, x := range c.Spec.EtcdClusters {
+			if err := validateEtcdClusterSpec(x); err != nil {
+				return err
 			}
 		}
-		// check both clusters are using tls if one us enabled
-		if usingTLS > 0 && usingTLS != len(c.Spec.EtcdClusters) {
-			return fmt.Errorf("Both etcd clusters must have TLS enabled or none at all")
+		if err := validateEtcdTLS(c.Spec.EtcdClusters); err != nil {
+			return err
+		}
+		if err := validateEtcdStorage(c.Spec.EtcdClusters); err != nil {
+			return err
 		}
 	}
 
@@ -416,6 +399,92 @@ func ValidateCluster(c *kops.Cluster, strict bool) error {
 	return nil
 }
 
+// validateEtcdClusterSpec is responsible for validating the etcd cluster spec
+func validateEtcdClusterSpec(spec *kops.EtcdClusterSpec) error {
+	if spec.Name == "" {
+		return fmt.Errorf("EtcdCluster did not have name")
+	}
+	if len(spec.Members) == 0 {
+		return fmt.Errorf("No members defined in spec cluster %q", spec.Name)
+	}
+	if (len(spec.Members) % 2) == 0 {
+		// Not technically a requirement, but doesn't really make sense to allow
+		return fmt.Errorf("Should be an odd number of master-zones for quorum. Use --zones and --master-zones to declare node zones and master zones separately")
+	}
+	if err := validateEtcdVersion(spec); err != nil {
+		return err
+	}
+	for _, m := range spec.Members {
+		if err := validateEtcdMemberSpec(m); err != nil {
+			return fmt.Errorf("Member in cluster: %q invalid, error: %q", spec.Name, err)
+		}
+	}
+
+	return nil
+}
+
+// validateEtcdTLS checks the TLS settings for etcd are valid
+func validateEtcdTLS(specs []*kops.EtcdClusterSpec) error {
+	var usingTLS int
+	for _, x := range specs {
+		if x.EnableEtcdTLS {
+			usingTLS++
+		}
+	}
+	// check both clusters are using tls if one us enabled
+	if usingTLS > 0 && usingTLS != len(specs) {
+		return fmt.Errorf("Both etcd clusters must have TLS enabled or none at all")
+	}
+
+	return nil
+}
+
+// validateEtcdStorage is responsible for checks version are identical
+func validateEtcdStorage(specs []*kops.EtcdClusterSpec) error {
+	version := specs[0].Version
+	for _, x := range specs {
+		if x.Version != "" && x.Version != version {
+			return fmt.Errorf("cluster: %q, has a different storage versions: %q, both must be the same", x.Name, x.Version)
+		}
+	}
+
+	return nil
+}
+
+// validateEtcdVersion is responsible for validating the storage version of etcd
+// @TODO semvar package doesn't appear to ignore a 'v' in v1.1.1 should could be a problem later down the line
+func validateEtcdVersion(spec *kops.EtcdClusterSpec) error {
+	// @check if the storage is specified, thats is valid
+	if spec.Version == "" {
+		return nil // as it will be filled in by default for us
+	}
+
+	sem, err := semver.Parse(strings.TrimPrefix(spec.Version, "v"))
+	if err != nil {
+		return fmt.Errorf("the storage version: %q for cluster: %q is invalid", spec.Version, spec.Name)
+	}
+
+	// we only support v3 and v2 for now
+	if sem.Major == 3 || sem.Major == 2 {
+		return nil
+	}
+
+	return fmt.Errorf("unsupported storage version: %q, we only support major versions 2 and 3", spec.Version)
+}
+
+// validateEtcdMemberSpec is responsible for validate the cluster member
+func validateEtcdMemberSpec(spec *kops.EtcdMemberSpec) error {
+	if spec.Name == "" {
+		return fmt.Errorf("does not have a name")
+	}
+
+	if fi.StringValue(spec.InstanceGroup) == "" {
+		return fmt.Errorf("does not have a instance group")
+	}
+
+	return nil
+}
+
 func DeepValidate(c *kops.Cluster, groups []*kops.InstanceGroup, strict bool) error {
 	if err := ValidateCluster(c, strict); err != nil {
 		return err