Merge pull request #2789 from robvanmieghem/master

FIX #2411 On a btrfs system, don't default to aufs

libmachine/provision/arch.go

@@ -94,9 +94,11 @@ func (provisioner *ArchProvisioner) Provision(swarmOptions swarm.Options, authOp
 	provisioner.EngineOptions = engineOptions
 	swarmOptions.Env = engineOptions.Env
 
-	if provisioner.EngineOptions.StorageDriver == "" {
+	storageDriver, err := decideStorageDriver(provisioner, "overlay", engineOptions.StorageDriver)
-		provisioner.EngineOptions.StorageDriver = "overlay"
+	if err != nil {
+		return err
 	}
+	provisioner.EngineOptions.StorageDriver = storageDriver
 
 	// HACK: since Arch does not come with sudo by default we install
 	log.Debug("Installing sudo")

libmachine/provision/arch_test.go

@@ -0,0 +1,20 @@
+package provision
+
+import (
+	"testing"
+
+	"github.com/docker/machine/drivers/fakedriver"
+	"github.com/docker/machine/libmachine/auth"
+	"github.com/docker/machine/libmachine/engine"
+	"github.com/docker/machine/libmachine/provision/provisiontest"
+	"github.com/docker/machine/libmachine/swarm"
+)
+
+func TestArchDefaultStorageDriver(t *testing.T) {
+	p := NewArchProvisioner(&fakedriver.Driver{}).(*ArchProvisioner)
+	p.SSHCommander = provisiontest.NewFakeSSHCommander(provisiontest.FakeSSHCommanderOptions{})
+	p.Provision(swarm.Options{}, auth.Options{}, engine.Options{})
+	if p.EngineOptions.StorageDriver != "overlay" {
+		t.Fatal("Default storage driver should be overlay")
+	}
+}

libmachine/provision/debian.go

@@ -106,9 +106,11 @@ func (provisioner *DebianProvisioner) Provision(swarmOptions swarm.Options, auth
 	provisioner.EngineOptions = engineOptions
 	swarmOptions.Env = engineOptions.Env
 
-	if provisioner.EngineOptions.StorageDriver == "" {
+	storageDriver, err := decideStorageDriver(provisioner, "aufs", engineOptions.StorageDriver)
-		provisioner.EngineOptions.StorageDriver = "aufs"
+	if err != nil {
+		return err
 	}
+	provisioner.EngineOptions.StorageDriver = storageDriver
 
 	// HACK: since debian does not come with sudo by default we install
 	log.Debug("installing sudo")

libmachine/provision/debian_test.go

@@ -0,0 +1,20 @@
+package provision
+
+import (
+	"testing"
+
+	"github.com/docker/machine/drivers/fakedriver"
+	"github.com/docker/machine/libmachine/auth"
+	"github.com/docker/machine/libmachine/engine"
+	"github.com/docker/machine/libmachine/provision/provisiontest"
+	"github.com/docker/machine/libmachine/swarm"
+)
+
+func TestDebianDefaultStorageDriver(t *testing.T) {
+	p := NewDebianProvisioner(&fakedriver.Driver{}).(*DebianProvisioner)
+	p.SSHCommander = provisiontest.NewFakeSSHCommander(provisiontest.FakeSSHCommanderOptions{})
+	p.Provision(swarm.Options{}, auth.Options{}, engine.Options{})
+	if p.EngineOptions.StorageDriver != "aufs" {
+		t.Fatal("Default storage driver should be aufs")
+	}
+}

libmachine/provision/provisiontest/sshcommander.go

@@ -0,0 +1,39 @@
+//Package provisiontest provides utilities for testing provisioners
+package provisiontest
+
+import "errors"
+
+//FakeSSHCommanderOptions is intended to create a FakeSSHCommander without actually knowing the underlying sshcommands by passing it to NewSSHCommander
+type FakeSSHCommanderOptions struct {
+	//Result of the ssh command to look up the FilesystemType
+	FilesystemType string
+}
+
+//FakeSSHCommander is an implementation of provision.SSHCommander to provide predictable responses set by testing code
+//Extend it when needed
+type FakeSSHCommander struct {
+	Responses map[string]string
+}
+
+//NewFakeSSHCommander creates a FakeSSHCommander without actually knowing the underlying sshcommands
+func NewFakeSSHCommander(options FakeSSHCommanderOptions) *FakeSSHCommander {
+	if options.FilesystemType == "" {
+		options.FilesystemType = "ext4"
+	}
+	sshCmder := &FakeSSHCommander{
+		Responses: map[string]string{
+			"stat -f -c %T /var/lib": options.FilesystemType + "\n",
+		},
+	}
+
+	return sshCmder
+}
+
+//SSHCommand is an implementation of provision.SSHCommander.SSHCommand to provide predictable responses set by testing code
+func (sshCmder *FakeSSHCommander) SSHCommand(args string) (string, error) {
+	response, commandRegistered := sshCmder.Responses[args]
+	if !commandRegistered {
+		return "", errors.New("Command not registered in FakeSSHCommander")
+	}
+	return response, nil
+}

libmachine/provision/provisiontest/sshcommander_test.go

@@ -0,0 +1,28 @@
+package provisiontest
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestCreateFakeSSHCommander(t *testing.T) {
+	sshCmder := NewFakeSSHCommander(FakeSSHCommanderOptions{FilesystemType: "btrfs"})
+	output, err := sshCmder.SSHCommand("stat -f -c %T /var/lib")
+	if err != nil || output != "btrfs\n" {
+		t.Fatal("FakeSSHCommander should have returned btrfs and no error but returned '", output, "' and error", err)
+	}
+}
+
+func TestStatSSHCommand(t *testing.T) {
+	sshCmder := FakeSSHCommander{
+		Responses: map[string]string{"sshcommand": "sshcommandresponse"},
+	}
+
+	output, err := sshCmder.SSHCommand("sshcommand")
+	assert.NoError(t, err)
+	assert.Equal(t, "sshcommandresponse", output)
+
+	output, err = sshCmder.SSHCommand("errorcommand")
+	assert.Error(t, err)
+}

libmachine/provision/redhat.go

@@ -159,9 +159,11 @@ func (provisioner *RedHatProvisioner) Provision(swarmOptions swarm.Options, auth
 	swarmOptions.Env = engineOptions.Env
 
 	// set default storage driver for redhat
-	if provisioner.EngineOptions.StorageDriver == "" {
+	storageDriver, err := decideStorageDriver(provisioner, "devicemapper", engineOptions.StorageDriver)
-		provisioner.EngineOptions.StorageDriver = "devicemapper"
+	if err != nil {
+		return err
 	}
+	provisioner.EngineOptions.StorageDriver = storageDriver
 
 	if err := provisioner.SetHostname(provisioner.Driver.GetMachineName()); err != nil {
 		return err

libmachine/provision/redhat_test.go

@@ -3,6 +3,12 @@ package provision
 import (
 	"regexp"
 	"testing"
+
+	"github.com/docker/machine/drivers/fakedriver"
+	"github.com/docker/machine/libmachine/auth"
+	"github.com/docker/machine/libmachine/engine"
+	"github.com/docker/machine/libmachine/provision/provisiontest"
+	"github.com/docker/machine/libmachine/swarm"
 )
 
 func TestRedHatGenerateYumRepoList(t *testing.T) {
@@ -26,3 +32,12 @@ func TestRedHatGenerateYumRepoList(t *testing.T) {
 		t.Fatalf("expected match for centos/7")
 	}
 }
+
+func TestRedHatDefaultStorageDriver(t *testing.T) {
+	p := NewRedHatProvisioner("", &fakedriver.Driver{})
+	p.SSHCommander = provisiontest.NewFakeSSHCommander(provisiontest.FakeSSHCommanderOptions{})
+	p.Provision(swarm.Options{}, auth.Options{}, engine.Options{})
+	if p.EngineOptions.StorageDriver != "devicemapper" {
+		t.Fatal("Default storage driver should be devicemapper")
+	}
+}

libmachine/provision/ubuntu_systemd.go

@@ -122,9 +122,11 @@ func (provisioner *UbuntuSystemdProvisioner) Provision(swarmOptions swarm.Option
 	provisioner.EngineOptions = engineOptions
 	swarmOptions.Env = engineOptions.Env
 
-	if provisioner.EngineOptions.StorageDriver == "" {
+	storageDriver, err := decideStorageDriver(provisioner, "aufs", engineOptions.StorageDriver)
-		provisioner.EngineOptions.StorageDriver = "aufs"
+	if err != nil {
+		return err
 	}
+	provisioner.EngineOptions.StorageDriver = storageDriver
 
 	log.Debug("setting hostname")
 	if err := provisioner.SetHostname(provisioner.Driver.GetMachineName()); err != nil {

libmachine/provision/ubuntu_systemd_test.go

@@ -2,6 +2,12 @@ package provision
 
 import (
 	"testing"
+
+	"github.com/docker/machine/drivers/fakedriver"
+	"github.com/docker/machine/libmachine/auth"
+	"github.com/docker/machine/libmachine/engine"
+	"github.com/docker/machine/libmachine/provision/provisiontest"
+	"github.com/docker/machine/libmachine/swarm"
 )
 
 func TestUbuntuSystemdCompatibleWithHost(t *testing.T) {
@@ -27,3 +33,12 @@ func TestUbuntuSystemdCompatibleWithHost(t *testing.T) {
 	}
 
 }
+
+func TestUbuntuSystemdDefaultStorageDriver(t *testing.T) {
+	p := NewUbuntuSystemdProvisioner(&fakedriver.Driver{}).(*UbuntuSystemdProvisioner)
+	p.SSHCommander = provisiontest.NewFakeSSHCommander(provisiontest.FakeSSHCommanderOptions{})
+	p.Provision(swarm.Options{}, auth.Options{}, engine.Options{})
+	if p.EngineOptions.StorageDriver != "aufs" {
+		t.Fatal("Default storage driver should be aufs")
+	}
+}

libmachine/provision/ubuntu_upstart.go

@@ -141,9 +141,11 @@ func (provisioner *UbuntuProvisioner) Provision(swarmOptions swarm.Options, auth
 	provisioner.EngineOptions = engineOptions
 	swarmOptions.Env = engineOptions.Env
 
-	if provisioner.EngineOptions.StorageDriver == "" {
+	storageDriver, err := decideStorageDriver(provisioner, "aufs", engineOptions.StorageDriver)
-		provisioner.EngineOptions.StorageDriver = "aufs"
+	if err != nil {
+		return err
 	}
+	provisioner.EngineOptions.StorageDriver = storageDriver
 
 	if err := provisioner.SetHostname(provisioner.Driver.GetMachineName()); err != nil {
 		return err

libmachine/provision/ubuntu_upstart_test.go

@@ -2,6 +2,12 @@ package provision
 
 import (
 	"testing"
+
+	"github.com/docker/machine/drivers/fakedriver"
+	"github.com/docker/machine/libmachine/auth"
+	"github.com/docker/machine/libmachine/engine"
+	"github.com/docker/machine/libmachine/provision/provisiontest"
+	"github.com/docker/machine/libmachine/swarm"
 )
 
 func TestUbuntuCompatibleWithHost(t *testing.T) {
@@ -27,3 +33,12 @@ func TestUbuntuCompatibleWithHost(t *testing.T) {
 	}
 
 }
+
+func TestUbuntuDefaultStorageDriver(t *testing.T) {
+	p := NewUbuntuProvisioner(&fakedriver.Driver{}).(*UbuntuProvisioner)
+	p.SSHCommander = provisiontest.NewFakeSSHCommander(provisiontest.FakeSSHCommanderOptions{})
+	p.Provision(swarm.Options{}, auth.Options{}, engine.Options{})
+	if p.EngineOptions.StorageDriver != "aufs" {
+		t.Fatal("Default storage driver should be aufs")
+	}
+}

libmachine/provision/utils.go

@@ -208,6 +208,46 @@ func matchNetstatOut(reDaemonListening, netstatOut string) bool {
 	return false
 }
 
+func decideStorageDriver(p Provisioner, defaultDriver, suppliedDriver string) (string, error) {
+	if suppliedDriver != "" {
+		return suppliedDriver, nil
+	}
+	bestSuitedDriver := ""
+
+	defer func() {
+		if bestSuitedDriver != "" {
+			log.Debugf("No storagedriver specified, using %s\n", bestSuitedDriver)
+		}
+	}()
+
+	if defaultDriver != "aufs" {
+		bestSuitedDriver = defaultDriver
+	} else {
+		remoteFilesystemType, err := getFilesystemType(p, "/var/lib")
+		if err != nil {
+			return "", err
+		}
+		if remoteFilesystemType == "btrfs" {
+			bestSuitedDriver = "btrfs"
+		} else {
+			bestSuitedDriver = "aufs"
+		}
+	}
+	return bestSuitedDriver, nil
+
+}
+
+func getFilesystemType(p Provisioner, directory string) (string, error) {
+	statCommandOutput, err := p.SSHCommand("stat -f -c %T " + directory)
+	if err != nil {
+		err = fmt.Errorf("Error looking up filesystem type: %s", err)
+		return "", err
+	}
+
+	fstype := strings.TrimSpace(statCommandOutput)
+	return fstype, nil
+}
+
 func checkDaemonUp(p Provisioner, dockerPort int) func() bool {
 	reDaemonListening := fmt.Sprintf(":%d.*LISTEN", dockerPort)
 	return func() bool {

libmachine/provision/utils_test.go

@@ -8,6 +8,12 @@ import (
 
 	"github.com/docker/machine/drivers/fakedriver"
 	"github.com/docker/machine/libmachine/auth"
+	"github.com/docker/machine/libmachine/engine"
+	"github.com/docker/machine/libmachine/provision/pkgaction"
+	"github.com/docker/machine/libmachine/provision/provisiontest"
+	"github.com/docker/machine/libmachine/provision/serviceaction"
+	"github.com/docker/machine/libmachine/swarm"
+	"github.com/stretchr/testify/assert"
 )
 
 var (
@@ -156,3 +162,66 @@ func TestMachineCustomPortBoot2Docker(t *testing.T) {
 		t.Errorf("expected url %s; received %s", bindURL, url)
 	}
 }
+
+type fakeProvisioner struct {
+	GenericProvisioner
+}
+
+func (provisioner *fakeProvisioner) Package(name string, action pkgaction.PackageAction) error {
+	return nil
+}
+
+func (provisioner *fakeProvisioner) Provision(swarmOptions swarm.Options, authOptions auth.Options, engineOptions engine.Options) error {
+	return nil
+}
+
+func (provisioner *fakeProvisioner) Service(name string, action serviceaction.ServiceAction) error {
+	return nil
+}
+
+func (provisioner *fakeProvisioner) String() string {
+	return "fake"
+}
+
+func TestDecideStorageDriver(t *testing.T) {
+	var tests = []struct {
+		suppliedDriver       string
+		defaultDriver        string
+		remoteFilesystemType string
+		expectedDriver       string
+	}{
+		{"", "aufs", "ext4", "aufs"},
+		{"", "aufs", "btrfs", "btrfs"},
+		{"", "overlay", "btrfs", "overlay"},
+		{"devicemapper", "aufs", "ext4", "devicemapper"},
+		{"devicemapper", "aufs", "btrfs", "devicemapper"},
+	}
+
+	p := &fakeProvisioner{GenericProvisioner{
+		Driver: &fakedriver.Driver{},
+	}}
+	for _, test := range tests {
+		p.SSHCommander = provisiontest.NewFakeSSHCommander(
+			provisiontest.FakeSSHCommanderOptions{
+				FilesystemType: test.remoteFilesystemType,
+			},
+		)
+		storageDriver, err := decideStorageDriver(p, test.defaultDriver, test.suppliedDriver)
+		assert.NoError(t, err)
+		assert.Equal(t, test.expectedDriver, storageDriver)
+	}
+}
+
+func TestGetFilesystemType(t *testing.T) {
+	p := &fakeProvisioner{GenericProvisioner{
+		Driver: &fakedriver.Driver{},
+	}}
+	p.SSHCommander = &provisiontest.FakeSSHCommander{
+		Responses: map[string]string{
+			"stat -f -c %T /var/lib": "btrfs\n",
+		},
+	}
+	fsType, err := getFilesystemType(p, "/var/lib")
+	assert.NoError(t, err)
+	assert.Equal(t, "btrfs", fsType)
+}