From 3e758c6d869a2ff5757dfe47f372b9adcdd41c78 Mon Sep 17 00:00:00 2001 From: Rob Van Mieghem Date: Fri, 8 Jan 2016 21:17:54 +0100 Subject: [PATCH] FIX #2411 Do not default to aufs on a btrfs filesystem Signed-off-by: Rob Van Mieghem --- libmachine/provision/arch.go | 6 ++- libmachine/provision/arch_test.go | 20 +++++++ libmachine/provision/debian.go | 6 ++- libmachine/provision/debian_test.go | 20 +++++++ .../provision/provisiontest/sshcommander.go | 22 ++++++++ .../provisiontest/sshcommander_test.go | 11 ++++ libmachine/provision/redhat.go | 6 ++- libmachine/provision/redhat_test.go | 15 ++++++ libmachine/provision/ubuntu_systemd.go | 6 ++- libmachine/provision/ubuntu_systemd_test.go | 15 ++++++ libmachine/provision/ubuntu_upstart.go | 6 ++- libmachine/provision/ubuntu_upstart_test.go | 15 ++++++ libmachine/provision/utils.go | 40 ++++++++++++++ libmachine/provision/utils_test.go | 54 +++++++++++++++++++ 14 files changed, 232 insertions(+), 10 deletions(-) create mode 100644 libmachine/provision/arch_test.go create mode 100644 libmachine/provision/debian_test.go create mode 100644 libmachine/provision/provisiontest/sshcommander.go create mode 100644 libmachine/provision/provisiontest/sshcommander_test.go diff --git a/libmachine/provision/arch.go b/libmachine/provision/arch.go index f0d46ce0bc..03436f99d0 100644 --- a/libmachine/provision/arch.go +++ b/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 == "" { - provisioner.EngineOptions.StorageDriver = "overlay" + storageDriver, err := decideStorageDriver(provisioner, "overlay", engineOptions.StorageDriver) + 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") diff --git a/libmachine/provision/arch_test.go b/libmachine/provision/arch_test.go new file mode 100644 index 0000000000..b40ce1aa8b --- /dev/null +++ b/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.FakeSSHCommander{FilesystemType: "ext4"} + p.Provision(swarm.Options{}, auth.Options{}, engine.Options{}) + if p.EngineOptions.StorageDriver != "overlay" { + t.Fatal("Default storage driver should be overlay") + } +} diff --git a/libmachine/provision/debian.go b/libmachine/provision/debian.go index 9ad9302d85..fe9f58d3f5 100644 --- a/libmachine/provision/debian.go +++ b/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 == "" { - provisioner.EngineOptions.StorageDriver = "aufs" + storageDriver, err := decideStorageDriver(provisioner, "aufs", engineOptions.StorageDriver) + 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") diff --git a/libmachine/provision/debian_test.go b/libmachine/provision/debian_test.go new file mode 100644 index 0000000000..740b6450a4 --- /dev/null +++ b/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.FakeSSHCommander{FilesystemType: "ext4"} + p.Provision(swarm.Options{}, auth.Options{}, engine.Options{}) + if p.EngineOptions.StorageDriver != "aufs" { + t.Fatal("Default storage driver should be aufs") + } +} diff --git a/libmachine/provision/provisiontest/sshcommander.go b/libmachine/provision/provisiontest/sshcommander.go new file mode 100644 index 0000000000..7a3bc9efe5 --- /dev/null +++ b/libmachine/provision/provisiontest/sshcommander.go @@ -0,0 +1,22 @@ +//Package provisiontest provides utilities for testing provisioners +package provisiontest + +import ( + "errors" + "strings" +) + +//FakeSSHCommander is an implementation of provision.SSHCommander to provide predictable responses set by testing code +//Extend it when needed +type FakeSSHCommander struct { + //Result of the ssh command to look up the FilesystemType + FilesystemType string +} + +//SSHCommand is an implementation of provision.SSHCommander.SSHCommand to provide predictable responses set by testing code +func (sshCmder FakeSSHCommander) SSHCommand(args string) (string, error) { + if !strings.HasPrefix(args, "stat -f") { + return "", errors.New("Not implemented by FakeSSHCommander") + } + return sshCmder.FilesystemType + "\n", nil +} diff --git a/libmachine/provision/provisiontest/sshcommander_test.go b/libmachine/provision/provisiontest/sshcommander_test.go new file mode 100644 index 0000000000..c695c8e6cb --- /dev/null +++ b/libmachine/provision/provisiontest/sshcommander_test.go @@ -0,0 +1,11 @@ +package provisiontest + +import "testing" + +func TestStatSSHCommand(t *testing.T) { + sshCmder := FakeSSHCommander{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) + } +} diff --git a/libmachine/provision/redhat.go b/libmachine/provision/redhat.go index 9d0dae1bf8..b4feb11649 100644 --- a/libmachine/provision/redhat.go +++ b/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 == "" { - provisioner.EngineOptions.StorageDriver = "devicemapper" + storageDriver, err := decideStorageDriver(provisioner, "devicemapper", engineOptions.StorageDriver) + if err != nil { + return err } + provisioner.EngineOptions.StorageDriver = storageDriver if err := provisioner.SetHostname(provisioner.Driver.GetMachineName()); err != nil { return err diff --git a/libmachine/provision/redhat_test.go b/libmachine/provision/redhat_test.go index 551e951bdc..a11b9faa42 100644 --- a/libmachine/provision/redhat_test.go +++ b/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.FakeSSHCommander{FilesystemType: "ext4"} + p.Provision(swarm.Options{}, auth.Options{}, engine.Options{}) + if p.EngineOptions.StorageDriver != "devicemapper" { + t.Fatal("Default storage driver should be devicemapper") + } +} diff --git a/libmachine/provision/ubuntu_systemd.go b/libmachine/provision/ubuntu_systemd.go index e6dd716786..775dc1fe9d 100644 --- a/libmachine/provision/ubuntu_systemd.go +++ b/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 == "" { - provisioner.EngineOptions.StorageDriver = "aufs" + storageDriver, err := decideStorageDriver(provisioner, "aufs", engineOptions.StorageDriver) + if err != nil { + return err } + provisioner.EngineOptions.StorageDriver = storageDriver log.Debug("setting hostname") if err := provisioner.SetHostname(provisioner.Driver.GetMachineName()); err != nil { diff --git a/libmachine/provision/ubuntu_systemd_test.go b/libmachine/provision/ubuntu_systemd_test.go index 9c3e076008..b3422a2f80 100644 --- a/libmachine/provision/ubuntu_systemd_test.go +++ b/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.FakeSSHCommander{FilesystemType: "ext4"} + p.Provision(swarm.Options{}, auth.Options{}, engine.Options{}) + if p.EngineOptions.StorageDriver != "aufs" { + t.Fatal("Default storage driver should be aufs") + } +} diff --git a/libmachine/provision/ubuntu_upstart.go b/libmachine/provision/ubuntu_upstart.go index 97860f2548..fe4e5ff70c 100644 --- a/libmachine/provision/ubuntu_upstart.go +++ b/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 == "" { - provisioner.EngineOptions.StorageDriver = "aufs" + storageDriver, err := decideStorageDriver(provisioner, "aufs", engineOptions.StorageDriver) + if err != nil { + return err } + provisioner.EngineOptions.StorageDriver = storageDriver if err := provisioner.SetHostname(provisioner.Driver.GetMachineName()); err != nil { return err diff --git a/libmachine/provision/ubuntu_upstart_test.go b/libmachine/provision/ubuntu_upstart_test.go index be98d1e162..9622d0cf14 100644 --- a/libmachine/provision/ubuntu_upstart_test.go +++ b/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.FakeSSHCommander{FilesystemType: "ext4"} + p.Provision(swarm.Options{}, auth.Options{}, engine.Options{}) + if p.EngineOptions.StorageDriver != "aufs" { + t.Fatal("Default storage driver should be aufs") + } +} diff --git a/libmachine/provision/utils.go b/libmachine/provision/utils.go index 318542a17b..57b52324d6 100644 --- a/libmachine/provision/utils.go +++ b/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 { diff --git a/libmachine/provision/utils_test.go b/libmachine/provision/utils_test.go index 1585035c48..2164840051 100644 --- a/libmachine/provision/utils_test.go +++ b/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,51 @@ 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{}, + }} + engineOptions := engine.Options{} + for _, test := range tests { + engineOptions.StorageDriver = test.suppliedDriver + p.SSHCommander = provisiontest.FakeSSHCommander{FilesystemType: test.remoteFilesystemType} + storageDriver, err := decideStorageDriver(p, test.defaultDriver, test.suppliedDriver) + assert.NoError(t, err) + assert.Equal(t, test.expectedDriver, storageDriver) + } + +}