Merge pull request #3621 from justinsb/protokube_mount_using_nsenter

Automatic merge from submit-queue.

Simplify protokube mounter using nsenter executor

nodeup/pkg/model/protokube.go

@@ -122,7 +122,8 @@ func (t *ProtokubeBuilder) buildSystemdService() (*nodetasks.Service, error) {
 
 	dockerArgs = append(dockerArgs, []string{
 		"--net=host",
-		"--privileged",
+		"--pid=host",   // Needed for mounting in a container (when using systemd mounting?)
+		"--privileged", // We execute in the host namespace
 		"--env", "KUBECONFIG=/rootfs/var/lib/kops/kubeconfig",
 		t.ProtokubeEnvironmentVariables(),
 		t.ProtokubeImageName(),

protokube/pkg/protokube/kube_boot.go

@@ -19,10 +19,10 @@ package protokube
 import (
 	"fmt"
 	"net"
-	"os/exec"
 	"time"
 
 	"github.com/golang/glog"
+	"k8s.io/kubernetes/pkg/util/mount"
 )
 
 var (
@@ -151,8 +151,17 @@ func startKubeletService() error {
 	// (in particular, we want to avoid kubernetes/kubernetes#40123 )
 	glog.V(2).Infof("ensuring that kubelet systemd service is running")
 
-	cmd := exec.Command("systemctl", "status", "--no-block", "kubelet")
-	output, err := cmd.CombinedOutput()
+	// We run systemctl from the hostfs so we don't need systemd in our image
+	// (and we don't risk version skew)
+
+	exec := mount.NewOsExec()
+	if Containerized {
+		exec = NewNsEnterExec()
+	}
+
+	systemctlCommand := "systemctl"
+
+	output, err := exec.Run(systemctlCommand, "status", "--no-block", "kubelet")
 	glog.V(2).Infof("'systemctl status kubelet' output:\n%s", string(output))
 	if err == nil {
 		glog.V(2).Infof("kubelet systemd service already running")
@@ -160,8 +169,7 @@ func startKubeletService() error {
 	}
 
 	glog.Infof("kubelet systemd service not running. Starting")
-	cmd = exec.Command("systemctl", "start", "--no-block", "kubelet")
-	output, err = cmd.CombinedOutput()
+	output, err = exec.Run(systemctlCommand, "start", "--no-block", "kubelet")
 	if err != nil {
 		return fmt.Errorf("error starting kubelet: %v\nOutput: %s", err, output)
 	}

protokube/pkg/protokube/nsenter_exec.go

@@ -0,0 +1,53 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package protokube
+
+import (
+	"github.com/golang/glog"
+	"k8s.io/kubernetes/pkg/util/mount"
+	"k8s.io/utils/exec"
+)
+
+// Constants from nsenter_mount.go
+const (
+	hostMountNamespacePath = "/rootfs/proc/1/ns/mnt"
+	nsenterPath            = "nsenter"
+)
+
+// NewNsEnterExec builds a mount.Exec implementation that nsenters into the host process
+// It is very similar to mount.NewNsenterMounter, but execs into the host
+func NewNsEnterExec() mount.Exec {
+	return &nsEnterExec{}
+}
+
+// nsEnterExec is an implementation of mount.Exec that runs in the host namespace
+type nsEnterExec struct{}
+
+var _ mount.Exec = &nsEnterExec{}
+
+// Run implements mount.Exec::Run but runs proceses in the host namespace
+func (e *nsEnterExec) Run(cmd string, args ...string) ([]byte, error) {
+	nsenterArgs := []string{
+		"--mount=" + hostMountNamespacePath,
+		"--",
+		cmd,
+	}
+	nsenterArgs = append(nsenterArgs, args...)
+	glog.V(5).Infof("Running command : %v %v", nsenterPath, nsenterArgs)
+	exe := exec.New()
+	return exe.Command(nsenterPath, nsenterArgs...).CombinedOutput()
+}

protokube/pkg/protokube/volume_mounter.go

@@ -105,54 +105,51 @@ func (k *VolumeMountController) safeFormatAndMount(device string, mountpoint str
 	}
 	glog.Infof("Found device %q", device)
 
-	// If we are containerized, we still first SafeFormatAndMount in our namespace
-	// This is because SafeFormatAndMount doesn't seem to work in a container
-	safeFormatAndMount := &mount.SafeFormatAndMount{Interface: mount.New(""), Exec: mount.NewOsExec()}
+	safeFormatAndMount := &mount.SafeFormatAndMount{}
+
+	if Containerized {
+		// Build mount & exec implementations that execute in the host namespaces
+		safeFormatAndMount.Interface = mount.NewNsenterMounter()
+		safeFormatAndMount.Exec = NewNsEnterExec()
+
+		// Note that we don't use pathFor for operations going through safeFormatAndMount,
+		// because NewNsenterMounter and NewNsEnterExec will operate in the host
+	} else {
+		safeFormatAndMount.Interface = mount.New("")
+		safeFormatAndMount.Exec = mount.NewOsExec()
+	}
 
 	// Check if it is already mounted
+	// TODO: can we now use IsLikelyNotMountPoint or IsMountPointMatch instead here
 	mounts, err := safeFormatAndMount.List()
 	if err != nil {
 		return fmt.Errorf("error listing existing mounts: %v", err)
 	}
 
-	// Note: IsLikelyNotMountPoint is not containerized
-
-	findMountpoint := pathFor(mountpoint)
 	var existing []*mount.MountPoint
 	for i := range mounts {
 		m := &mounts[i]
 		glog.V(8).Infof("found existing mount: %v", m)
-		if m.Path == findMountpoint {
+		// Note: when containerized, we still list mounts in the host, so we don't need to call pathFor(mountpoint)
+		if m.Path == mountpoint {
 			existing = append(existing, m)
 		}
 	}
 
-	options := []string{}
-	//if readOnly {
-	//	options = append(options, "ro")
-	//}
+	// Mount only if isn't mounted already
 	if len(existing) == 0 {
+		options := []string{}
+
 		glog.Infof("Creating mount directory %q", pathFor(mountpoint))
 		if err := os.MkdirAll(pathFor(mountpoint), 0750); err != nil {
 			return err
 		}
 
-		glog.Infof("Mounting device %q on %q", pathFor(device), pathFor(mountpoint))
+		glog.Infof("Mounting device %q on %q", device, mountpoint)
 
-		err = safeFormatAndMount.FormatAndMount(pathFor(device), pathFor(mountpoint), fstype, options)
+		err = safeFormatAndMount.FormatAndMount(device, mountpoint, fstype, options)
 		if err != nil {
-			//os.Remove(mountpoint)
-			return fmt.Errorf("error formatting and mounting disk %q on %q: %v", pathFor(device), pathFor(mountpoint), err)
-		}
-
-		// If we are containerized, we then also mount it into the host
-		if Containerized {
-			hostMounter := mount.NewNsenterMounter()
-			err = hostMounter.Mount(device, mountpoint, fstype, options)
-			if err != nil {
-				//os.Remove(mountpoint)
-				return fmt.Errorf("error formatting and mounting disk %q on %q in host: %v", device, mountpoint, err)
-			}
+			return fmt.Errorf("error formatting and mounting disk %q on %q: %v", device, mountpoint, err)
 		}
 	} else {
 		glog.Infof("Device already mounted on %q, verifying it is our device", mountpoint)
@@ -165,12 +162,38 @@ func (k *VolumeMountController) safeFormatAndMount(device string, mountpoint str
 				glog.Infof("%s\t%s", m.Device, m.Path)
 			}
 
-			return fmt.Errorf("Found multiple existing mounts of %q at %q", device, mountpoint)
+			return fmt.Errorf("found multiple existing mounts of %q at %q", device, mountpoint)
 		} else {
 			glog.Infof("Found existing mount of %q at %q", device, mountpoint)
 		}
-
 	}
+
+	// If we're containerized we also want to mount the device (again) into our container
+	// We could also do this with mount propagation, but this is simple
+	if Containerized {
+		source := pathFor(device)
+		target := pathFor(mountpoint)
+		options := []string{}
+
+		mounter := mount.New("")
+
+		mountedDevice, _, err := mount.GetDeviceNameFromMount(mounter, target)
+		if err != nil {
+			return fmt.Errorf("error checking for mounts of %s inside container: %v", target, err)
+		}
+
+		if mountedDevice != "" {
+			if mountedDevice != source {
+				return fmt.Errorf("device already mounted at %s, but is %s and we want %s", target, mountedDevice, source)
+			}
+		} else {
+			glog.Infof("mounting inside container: %s -> %s", source, target)
+			if err := mounter.Mount(source, target, fstype, options); err != nil {
+				return fmt.Errorf("error mounting %s inside container at %s: %v", source, target, err)
+			}
+		}
+	}
+
 	return nil
 }