Fixes

- added the master option back the protokube, updating the nodeup model and protokube code
- removed any comments no related to the PR as suggested
- reverted the ordering of the mutex in the AWSVolumes in protokube

nodeup/pkg/model/protokube.go

@@ -45,34 +45,38 @@ var _ fi.ModelBuilder = &ProtokubeBuilder{}
 
 // Build is responsible for generating the options for protokube
 func (t *ProtokubeBuilder) Build(c *fi.ModelBuilderContext) error {
-	// @check if protokube; we have decided to disable this by default (https://github.com/kubernetes/kops/pull/3091)
-	if !t.IsMaster {
+	useGossip := dns.IsGossipHostname(t.Cluster.Spec.MasterInternalName)
+
+	// check is not a master and we are not using gossip (https://github.com/kubernetes/kops/pull/3091)
+	if !t.IsMaster && !useGossip {
 		glog.V(2).Infof("skipping the provisioning of protokube on the nodes")
 		return nil
 	}
 
-	kubeconfig, err := t.buildPKIKubeconfig("kops")
-	if err != nil {
-		return err
-	}
-
-	c.AddTask(&nodetasks.File{
-		Path:     "/var/lib/kops/kubeconfig",
-		Contents: fi.NewStringResource(kubeconfig),
-		Type:     nodetasks.FileType_File,
-		Mode:     s("0400"),
-	})
+	if t.IsMaster {
+		kubeconfig, err := t.buildPKIKubeconfig("kops")
+		if err != nil {
+			return err
+		}
 
-	// retrieve the etcd peer certificates and private keys from the keystore
-	if t.Cluster.Spec.EnableEtcdTLS {
-		for _, x := range []string{"etcd", "etcd-client"} {
-			if err = t.buildCeritificateTask(c, x, fmt.Sprintf("%s.pem", x)); err != nil {
-				return err
+		c.AddTask(&nodetasks.File{
+			Path:     "/var/lib/kops/kubeconfig",
+			Contents: fi.NewStringResource(kubeconfig),
+			Type:     nodetasks.FileType_File,
+			Mode:     s("0400"),
+		})
+
+		// retrieve the etcd peer certificates and private keys from the keystore
+		if t.Cluster.Spec.EnableEtcdTLS {
+			for _, x := range []string{"etcd", "etcd-client"} {
+				if err := t.buildCeritificateTask(c, x, fmt.Sprintf("%s.pem", x)); err != nil {
+					return err
+				}
 			}
-		}
-		for _, x := range []string{"etcd", "etcd-client"} {
-			if err = t.buildPrivateTask(c, x, fmt.Sprintf("%s-key.pem", x)); err != nil {
-				return err
+			for _, x := range []string{"etcd", "etcd-client"} {
+				if err := t.buildPrivateTask(c, x, fmt.Sprintf("%s-key.pem", x)); err != nil {
+					return err
+				}
 			}
 		}
 	}
@@ -179,6 +183,7 @@ type ProtokubeFlags struct {
 	DNSServer         *string  `json:"dns-server,omitempty" flag:"dns-server"`
 	InitializeRBAC    *bool    `json:"initializeRBAC,omitempty" flag:"initialize-rbac"`
 	LogLevel          *int32   `json:"logLevel,omitempty" flag:"v"`
+	Master            *bool    `json:"master,omitempty" flag:"master"`
 	PeerTLSCaFile     *string  `json:"peer-ca,omitempty" flag:"peer-ca"`
 	PeerTLSCertFile   *string  `json:"peer-cert,omitempty" flag:"peer-cert"`
 	PeerTLSKeyFile    *string  `json:"peer-key,omitempty" flag:"peer-key"`
@@ -194,6 +199,7 @@ func (t *ProtokubeBuilder) ProtokubeFlags(k8sVersion semver.Version) *ProtokubeF
 		Channels:      t.NodeupConfig.Channels,
 		Containerized: fi.Bool(true),
 		LogLevel:      fi.Int32(4),
+		Master:        b(t.IsMaster),
 	}
 	useTLS := t.Cluster.Spec.EnableEtcdTLS
 
@@ -202,7 +208,7 @@ func (t *ProtokubeBuilder) ProtokubeFlags(k8sVersion semver.Version) *ProtokubeF
 		f.InitializeRBAC = fi.Bool(true)
 	}
 
-	// @check if we are using tls and add the options
+	// check if we are using tls and add the options to protokube
 	if useTLS {
 		f.PeerTLSCaFile = s(filepath.Join(t.PathSrvKubernetes(), "ca.crt"))
 		f.PeerTLSCertFile = s(filepath.Join(t.PathSrvKubernetes(), "etcd.pem"))

pkg/apis/kops/cluster.go

@@ -261,23 +261,27 @@ type EtcdMemberSpec struct {
 type SubnetType string
 
 const (
-	SubnetTypePublic  SubnetType = "Public"
+	// SubnetTypePublic means the subnet is public
+	SubnetTypePublic SubnetType = "Public"
+	// SubnetTypePrivate means the subnet has no public address or is natted
 	SubnetTypePrivate SubnetType = "Private"
+	// SubnetTypeUtility mean the subnet is used for utility services, such as the bastion
 	SubnetTypeUtility SubnetType = "Utility"
 )
 
+// ClusterSubnetSpec defines a subnet
 type ClusterSubnetSpec struct {
+	// Name is the name of the subnet
 	Name string `json:"name,omitempty"`
-
-	Zone string `json:"zone,omitempty"`
-
+	// CIDR is the network cidr of the subnet
 	CIDR string `json:"cidr,omitempty"`
-
+	// Zone is the zone the subnet resides
+	Zone string `json:"zone,omitempty"`
 	// ProviderID is the cloud provider id for the objects associated with the zone (the subnet on AWS)
 	ProviderID string `json:"id,omitempty"`
-
+	// Egress
 	Egress string `json:"egress,omitempty"`
-
+	// Type define which one if the internal types (public, utility, private) the network is
 	Type SubnetType `json:"type,omitempty"`
 }
 

pkg/apis/kops/componentconfig.go

@@ -187,7 +187,7 @@ type KubeAPIServerConfig struct {
 	EtcdCAFile string `json:"etcdCaFile,omitempty" flag:"etcd-cafile"`
 	// EtcdCertFile is the path to a certificate
 	EtcdCertFile string `json:"etcdCertFile,omitempty" flag:"etcd-certfile"`
-	// EtcdKeyFile is the path to a orivate key
+	// EtcdKeyFile is the path to a private key
 	EtcdKeyFile string `json:"etcdKeyFile,omitempty" flag:"etcd-keyfile"`
 	// TODO: Remove unused BasicAuthFile
 	BasicAuthFile string `json:"basicAuthFile,omitempty" flag:"basic-auth-file"`

pkg/model/firewall.go

@@ -134,7 +134,7 @@ func (b *FirewallModelBuilder) applyNodeToMasterAllowSpecificPorts(c *fi.ModelBu
 
 		if b.Cluster.Spec.Networking.Calico != nil {
 			// Calico needs to access etcd
-			// TODO: Remove, replace with etcd in calico manifest: note this port is deprecated and we should use the 2379
+			// TODO: Remove, replace with etcd in calico manifest
 			// https://coreos.com/etcd/docs/latest/v2/configuration.html
 			glog.Warningf("Opening etcd port on masters for access from the nodes, for calico.  This is unsafe in untrusted environments.")
 			tcpPorts = append(tcpPorts, 4001)

pkg/model/pki.go

@@ -74,17 +74,16 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error {
 		c.AddTask(t)
 	}
 
-	// @check if we need to generate certificates for etcd peers certificates from a different CA
+	// check if we need to generate certificates for etcd peers certificates from a different CA?
 	// @question i think we should use another KeyStore for this, perhaps registering a EtcdKeyStore given
-	// that mutual tls used to verify between the peers we don't was
-	// For clients assuming we are using etcdv3 is can switch on user authentication and map the common names for auth
+	// that mutual tls used to verify between the peers we don't want certificates for kubernetes able to act as a peer.
+	// For clients assuming we are using etcdv3 is can switch on user authentication and map the common names for auth.
 	if b.Cluster.Spec.EnableEtcdTLS {
 		alternativeNames := []string{
 			fmt.Sprintf("*.internal.%s", b.ClusterName()),
-			fmt.Sprintf("*.internal.%s", b.Cluster.Spec.DNSZone),
 			"localhost", "127.0.0.1"}
 		{
-			// @question should wildcard here instead of generating per node. If we ever provide the
+			// @question should wildcard's be here instead of generating per node. If we ever provide the
 			// ability to resize the master, this will become a blocker
 			c.AddTask(&fitasks.Keypair{
 				AlternateNames: alternativeNames,
@@ -94,7 +93,6 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error {
 				Type:           "server",
 			})
 		}
-		// @TODO when we move to etcdv3 we should generate a client cert for Calico as well
 		{
 			c.AddTask(&fitasks.Keypair{
 				Name:      fi.String("etcd-client"),

protokube/cmd/protokube/main.go

@@ -60,16 +60,14 @@ func main() {
 // run is responsible for running the protokube service controller
 func run() error {
 	var zones []string
-	var applyTaints, initializeRBAC, containerized bool
+	var applyTaints, initializeRBAC, containerized, master bool
 	var cloud, clusterID, dnsServer, dnsProviderID, dnsInternalSuffix, gossipSecret, gossipListen string
 	var flagChannels, tlsCert, tlsKey, tlsCA, peerCert, peerKey, peerCA, etcdImageSource string
 
 	flag.BoolVar(&applyTaints, "apply-taints", applyTaints, "Apply taints to nodes based on the role")
 	flag.BoolVar(&containerized, "containerized", containerized, "Set if we are running containerized.")
 	flag.BoolVar(&initializeRBAC, "initialize-rbac", initializeRBAC, "Set if we should initialize RBAC")
-	flag.StringVar(&tlsCA, "tls-ca", tlsCA, "Path to a file containing the ca for client certificates")
-	flag.StringVar(&tlsCert, "tls-cert", tlsCert, "Path to a file containing the certificate for etcd server")
-	flag.StringVar(&tlsKey, "tls-key", tlsKey, "Path to a file containing the key certificate for etcd server")
+	flag.BoolVar(&master, "master", master, "Whether or not this node is a master")
 	flag.StringVar(&cloud, "cloud", "aws", "CloudProvider we are using (aws,gce)")
 	flag.StringVar(&clusterID, "cluster-id", clusterID, "Cluster ID")
 	flag.StringVar(&dnsInternalSuffix, "dns-internal-suffix", dnsInternalSuffix, "DNS suffix for internal domain names")
@@ -79,6 +77,9 @@ func run() error {
 	flag.StringVar(&peerCA, "peer-ca", peerCA, "Path to a file containing the peer ca in PEM format")
 	flag.StringVar(&peerCert, "peer-cert", peerCert, "Path to a file containing the peer certificate")
 	flag.StringVar(&peerKey, "peer-key", peerKey, "Path to a file containing the private key for the peers")
+	flag.StringVar(&tlsCA, "tls-ca", tlsCA, "Path to a file containing the ca for client certificates")
+	flag.StringVar(&tlsCert, "tls-cert", tlsCert, "Path to a file containing the certificate for etcd server")
+	flag.StringVar(&tlsKey, "tls-key", tlsKey, "Path to a file containing the private key for etcd server")
 	flags.StringSliceVarP(&zones, "zone", "z", []string{}, "Configure permitted zones and their mappings")
 	flags.StringVar(&dnsProviderID, "dns", "aws-route53", "DNS provider we should use (aws-route53, google-clouddns, coredns)")
 	flags.StringVar(&etcdImageSource, "etcd-image-source", etcdImageSource, "Etcd Source Container Registry")
@@ -284,19 +285,20 @@ func run() error {
 	k := &protokube.KubeBoot{
 		ApplyTaints:       applyTaints,
 		Channels:          channels,
-		TLSCA:             tlsCA,
-		TLSCert:           tlsCert,
-		TLSKey:            tlsKey,
 		DNS:               dnsProvider,
 		EtcdImageSource:   etcdImageSource,
 		InitializeRBAC:    initializeRBAC,
 		InternalDNSSuffix: dnsInternalSuffix,
 		InternalIP:        internalIP,
 		Kubernetes:        protokube.NewKubernetesContext(),
+		Master:            master,
 		ModelDir:          modelDir,
 		PeerCA:            peerCA,
 		PeerCert:          peerCert,
 		PeerKey:           peerKey,
+		TLSCA:             tlsCA,
+		TLSCert:           tlsCert,
+		TLSKey:            tlsKey,
 	}
 
 	k.Init(volumes)

protokube/pkg/protokube/aws_volume.go

@@ -39,13 +39,14 @@ var devices = []string{"/dev/xvdu", "/dev/xvdv", "/dev/xvdx", "/dev/xvdx", "/dev
 
 // AWSVolumes defines the aws volume implementation
 type AWSVolumes struct {
+	mutex sync.Mutex
+
 	clusterTag string
 	deviceMap  map[string]string
 	ec2        *ec2.EC2
 	instanceId string
 	internalIP net.IP
 	metadata   *ec2metadata.EC2Metadata
-	mutex      sync.Mutex
 	zone       string
 }
 

protokube/pkg/protokube/etcd_manifest.go

@@ -101,8 +101,8 @@ func BuildEtcdManifest(c *EtcdCluster) *v1.Pod {
 			},
 		})
 
-		// @check if tls is enabled and mount the directory - it might be worth concidering
-		// if we you use our own directory in /srv i.e /srv/etcd
+		// @check if tls is enabled and mount the directory. It might be worth considering
+		// if we you use our own directory in /srv i.e /srv/etcd rather than the default /src/kubernetes
 		if c.isTLS() {
 			for _, dirname := range buildCertificateDirectories(c) {
 				normalized := strings.Replace(dirname, "/", "", -1)

protokube/pkg/protokube/kube_boot.go

@@ -34,6 +34,8 @@ var (
 
 // KubeBoot is the options for the protokube service
 type KubeBoot struct {
+	// Channels is a list of channel to apply
+	Channels []string
 	// InitializeRBAC should be set to true if we should create the core RBAC roles
 	InitializeRBAC bool
 	// InternalDNSSuffix is the dns zone we are living in
@@ -59,9 +61,11 @@ type KubeBoot struct {
 	// PeerCert is the path to a peer certificate for etcd
 	PeerCert string
 	// PeerKey is the path to a peer private key for etcd
-	PeerKey         string
-	Channels        []string
-	Kubernetes      *KubernetesContext
+	PeerKey string
+	// Kubernetes is the context methods for kubernetes
+	Kubernetes *KubernetesContext
+	// Master indicates we are a master node
+	Master          bool
 	volumeMounter   *VolumeMountController
 	etcdControllers map[string]*EtcdController
 }
@@ -89,6 +93,7 @@ func (k *KubeBoot) syncOnce() error {
 	if err != nil {
 		return err
 	}
+
 	for _, v := range volumes {
 		for _, etcdSpec := range v.Info.EtcdClusters {
 			key := etcdSpec.ClusterKey + "::" + etcdSpec.NodeName
@@ -105,17 +110,18 @@ func (k *KubeBoot) syncOnce() error {
 			}
 		}
 	}
-	// apply the kubernetes taints?
-	if k.ApplyTaints {
-		if err := applyMasterTaints(k.Kubernetes); err != nil {
-			glog.Warningf("error updating master taints: %v", err)
+	if k.Master {
+		if k.ApplyTaints {
+			if err := applyMasterTaints(k.Kubernetes); err != nil {
+				glog.Warningf("error updating master taints: %v", err)
+			}
 		}
-	}
 
-	if k.InitializeRBAC {
-		// @TODO: Idempotency
-		if err := applyRBAC(k.Kubernetes); err != nil {
-			glog.Warningf("error initializing RBAC: %v", err)
+		if k.InitializeRBAC {
+			// @TODO: Idempotency: good question; not sure this should ever be done on the node though
+			if err := applyRBAC(k.Kubernetes); err != nil {
+				glog.Warningf("error initializing rbac: %v", err)
+			}
 		}
 	}