Merge pull request #3208 from chrislovecnm/protokube-volume-mount-fixes

Automatic merge from submit-queue

Improving etcd volume detection logic, ensuring that root volumes are not mounted

Fixes: https://github.com/kubernetes/kops/issues/3167

When an AWS account has functionality that adds an ec2 instance tags to a volume automatically, protokube can attempt to mount the root volume.  This PR tightens the logic for detecting etcd volumes.  Also, the two volumes that AWS defines as root volume devices are never mounted. Added a unit test, which required refactoring of the code into a separate method.

protokube/pkg/protokube/aws_volume.go

@@ -183,6 +183,13 @@ func (a *AWSVolumes) findVolumes(request *ec2.DescribeVolumesInput) ([]*Volume,
 				}
 			}
 
+			// never mount root volumes
+			// these are volumes that aws sets aside for root volumes mount points
+			if vol.LocalDevice == "/dev/sda1" || vol.LocalDevice == "/dev/xvda" {
+				glog.Warningf("Not mounting: %q, since it is a root volume", vol.LocalDevice)
+				continue
+			}
+
 			skipVolume := false
 
 			for _, tag := range v.Tags {

protokube/pkg/protokube/volume_mounter.go

@@ -185,6 +185,10 @@ func (k *VolumeMountController) attachMasterVolumes() ([]*Volume, error) {
 	var tryAttach []*Volume
 	var attached []*Volume
 	for _, v := range volumes {
+		if doNotMountVolume(v) {
+			continue
+		}
+
 		if v.AttachedTo == "" {
 			tryAttach = append(tryAttach, v)
 		}
@@ -245,6 +249,15 @@ func (k *VolumeMountController) attachMasterVolumes() ([]*Volume, error) {
 	return attached, nil
 }
 
+// doNotMountVolume tests that the volume has an Etcd Cluster associated
+func doNotMountVolume(v *Volume) bool {
+	if len(v.Info.EtcdClusters) == 0 {
+		glog.Warningf("Local device: %q, volume id: %q is being skipped and will not mounted, since it does not have a etcd cluster", v.LocalDevice, v.ID)
+		return true
+	}
+	return false
+}
+
 // ByEtcdClusterName sorts volumes so that we mount in a consistent order,
 // and in addition we try to mount the main etcd volume before the events etcd volume
 type ByEtcdClusterName []*Volume

protokube/pkg/protokube/volume_mounter_test.go

@@ -57,3 +57,42 @@ func Test_VolumeSort_ByEtcdClusterName(t *testing.T) {
 	}
 
 }
+
+func Test_Mount_Volumes(t *testing.T) {
+	grid := []struct {
+		volume      *Volume
+		doNotMount  bool
+		description string
+	}{
+		{
+			&Volume{
+				LocalDevice: "/dev/xvda",
+			},
+			true,
+			"xda without a etcd cluster, do not mount",
+		},
+		{
+			&Volume{
+				LocalDevice: "/dev/xvdb",
+				Info: VolumeInfo{
+					EtcdClusters: []*EtcdClusterSpec{
+						{
+							ClusterKey: "foo",
+							NodeName:   "bar",
+						},
+					},
+				},
+			},
+			true,
+			"xdb with a etcd cluster, mount",
+		},
+	}
+
+	for _, g := range grid {
+		d := doNotMountVolume(g.volume)
+		if d && !g.doNotMount {
+			t.Fatalf("volume mount should not have mounted: %s, description: %s", g.volume.LocalDevice, g.description)
+		}
+	}
+
+}