Fix failed override of provider introduced in #495 (#520)

* Fix failed override of provider introduced in #495

* Fix broken count provided in site count when aborting tests

* Improve complex if/else to decide on provider override

* Add TestPantheonPull to do an actual app.Import()

* Rename App.Config() to App.CreateSettingsFile()

* Refactor EnsureDdevNetwork, make sure we have ddev_default at start

cmd/ddev/cmd/import-db.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"os"
 
+	"github.com/drud/ddev/pkg/dockerutil"
 	"github.com/drud/ddev/pkg/plugins/platform"
 	"github.com/drud/ddev/pkg/util"
 	"github.com/spf13/cobra"
@@ -25,7 +26,7 @@ can be provided if it is not located at the top-level of the archive.`,
 			util.CheckErr(err)
 			os.Exit(0)
 		}
-		dockerNetworkPreRun()
+		dockerutil.EnsureDdevNetwork()
 	},
 	Run: func(cmd *cobra.Command, args []string) {
 		app, err := platform.GetActiveApp("")

cmd/ddev/cmd/import-files.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"os"
 
+	"github.com/drud/ddev/pkg/dockerutil"
 	"github.com/drud/ddev/pkg/plugins/platform"
 	"github.com/drud/ddev/pkg/util"
 	"github.com/spf13/cobra"
@@ -27,7 +28,7 @@ the assets being imported.`,
 			util.CheckErr(err)
 			os.Exit(0)
 		}
-		dockerNetworkPreRun()
+		dockerutil.EnsureDdevNetwork()
 	},
 	Run: func(cmd *cobra.Command, args []string) {
 		app, err := platform.GetActiveApp("")

cmd/ddev/cmd/pull.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"os"
 
+	"github.com/drud/ddev/pkg/dockerutil"
 	"github.com/drud/ddev/pkg/plugins/platform"
 	"github.com/drud/ddev/pkg/util"
 	"github.com/fatih/color"
@@ -24,7 +25,7 @@ var PullCmd = &cobra.Command{
 			util.CheckErr(err)
 			os.Exit(0)
 		}
-		dockerNetworkPreRun()
+		dockerutil.EnsureDdevNetwork()
 	},
 	Run: func(cmd *cobra.Command, args []string) {
 		appImport(skipConfirmation)
@@ -35,7 +36,7 @@ func appImport(skipConfirmation bool) {
 	app, err := platform.GetActiveApp("")
 
 	if err != nil {
-		util.Failed("%v", err)
+		util.Failed("appImport failed: %v", err)
 	}
 
 	if !skipConfirmation {

cmd/ddev/cmd/restart.go

@@ -3,14 +3,15 @@ package cmd
 import (
 	"os"
 
+	"github.com/drud/ddev/pkg/dockerutil"
 	"github.com/drud/ddev/pkg/output"
 	"github.com/drud/ddev/pkg/plugins/platform"
 	"github.com/drud/ddev/pkg/util"
 	"github.com/spf13/cobra"
 )
 
-// LocalDevReconfigCmd rebuilds an apps settings
-var LocalDevReconfigCmd = &cobra.Command{
+// LocalDevRestartCmd rebuilds an apps settings
+var LocalDevRestartCmd = &cobra.Command{
 	Use:   "restart",
 	Short: "Restart the local development environment for a site.",
 	Long:  `Restart stops the containers for site's environment and starts them back up again.`,
@@ -21,7 +22,7 @@ var LocalDevReconfigCmd = &cobra.Command{
 			os.Exit(0)
 		}
 
-		dockerNetworkPreRun()
+		dockerutil.EnsureDdevNetwork()
 	},
 	Run: func(cmd *cobra.Command, args []string) {
 		app, err := platform.GetActiveApp("")
@@ -46,6 +47,6 @@ var LocalDevReconfigCmd = &cobra.Command{
 }
 
 func init() {
-	RootCmd.AddCommand(LocalDevReconfigCmd)
+	RootCmd.AddCommand(LocalDevRestartCmd)
 
 }

cmd/ddev/cmd/root.go

@@ -23,8 +23,6 @@ var (
 	serviceType    string
 )
 
-const netName = "ddev_default"
-
 // RootCmd represents the base command when called without any subcommands
 var RootCmd = &cobra.Command{
 	Use:   "ddev",
@@ -110,15 +108,6 @@ func Execute() {
 
 }
 
-func dockerNetworkPreRun() {
-	client := dockerutil.GetDockerClient()
-
-	err := dockerutil.EnsureNetwork(client, netName)
-	if err != nil {
-		util.Failed("Unable to create/ensure docker network %s, error: %v", netName, err)
-	}
-}
-
 func init() {
 	RootCmd.PersistentFlags().BoolVarP(&output.JSONOutput, "json-output", "j", false, "If true, user-oriented output will be in JSON format.")
 }

cmd/ddev/cmd/start.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"os"
 
+	"github.com/drud/ddev/pkg/dockerutil"
 	"github.com/drud/ddev/pkg/output"
 	"github.com/drud/ddev/pkg/plugins/platform"
 	"github.com/drud/ddev/pkg/util"
@@ -23,7 +24,7 @@ provide a working environment for development.`,
 			os.Exit(0)
 		}
 
-		dockerNetworkPreRun()
+		dockerutil.EnsureDdevNetwork()
 	},
 	Run: func(cmd *cobra.Command, args []string) {
 		appStart()

pkg/ddevapp/config.go

@@ -107,12 +107,13 @@ func NewConfig(AppRoot string, provider string) (*Config, error) {
 
 	// Allow override with "pantheon" from function provider arg, but nothing else.
 	// Otherwise we accept whatever might have been in config file if there was anything.
-	switch {
-	case provider == "" || provider == DefaultProviderName:
-		c.Provider = DefaultProviderName
-	case provider == "pantheon":
-		c.Provider = "pantheon"
-	default:
+	if provider == "" && c.Provider != "" {
+		// Do nothing. This is the case where the config has a provider and no override is provided. Config wins.
+	} else if provider == "pantheon" || provider == DefaultProviderName {
+		c.Provider = provider // Use the provider passed-in. Function argument wins.
+	} else if provider == "" && c.Provider == "" {
+		c.Provider = DefaultProviderName // Nothing passed in, nothing configured. Set c.Provider to default
+	} else {
 		return c, fmt.Errorf("Provider '%s' is not implemented", provider)
 	}
 
@@ -190,7 +191,7 @@ func (c *Config) Read() error {
 
 	source, err := ioutil.ReadFile(c.ConfigPath)
 	if err != nil {
-		return fmt.Errorf("could not find an active ddev configuration, have you run 'ddev config'? %v", err)
+		return fmt.Errorf("could not find an active ddev configuration at %s have you run 'ddev config'? %v", c.ConfigPath, err)
 	}
 
 	// validate extend command keys
@@ -233,8 +234,8 @@ func (c *Config) WarnIfConfigReplace() {
 	if c.ConfigExists() {
 		util.Warning("You are reconfiguring the app at %s. \nThe existing configuration will be updated and replaced.", c.AppRoot)
 	} else {
-		output.UserOut.Printf("Creating a new ddev project config in the current directory (%s)", c.AppRoot)
-		output.UserOut.Printf("Once completed, your configuration will be written to %s", c.ConfigPath)
+		util.Success("Creating a new ddev project config in the current directory (%s)", c.AppRoot)
+		util.Success("Once completed, your configuration will be written to %s\n", c.ConfigPath)
 	}
 }
 
@@ -290,7 +291,7 @@ func (c *Config) Validate() error {
 	// validate apptype
 	match = IsAllowedAppType(c.AppType)
 	if !match {
-		return fmt.Errorf("%s is not a valid apptype", c.AppType)
+		return fmt.Errorf("'%s' is not a valid apptype", c.AppType)
 	}
 
 	return nil
@@ -441,7 +442,7 @@ func (c *Config) appTypePrompt() error {
 		appType = strings.ToLower(util.GetInput(c.AppType))
 
 		if IsAllowedAppType(appType) != true {
-			output.UserOut.Errorf("%s is not a valid application type. Allowed application types are: %s\n", appType, strings.Join(AllowedAppTypes, ", "))
+			output.UserOut.Errorf("'%s' is not a valid application type. Allowed application types are: %s\n", appType, strings.Join(AllowedAppTypes, ", "))
 		}
 		c.AppType = appType
 	}
@@ -458,7 +459,7 @@ func IsAllowedAppType(appType string) bool {
 	return false
 }
 
-// PrepDdevDirectory creates a .ddev directory in the current working
+// PrepDdevDirectory creates a .ddev directory in the current working directory
 func PrepDdevDirectory(dir string) error {
 	if _, err := os.Stat(dir); os.IsNotExist(err) {
 

pkg/ddevapp/config_test.go

@@ -10,6 +10,7 @@ import (
 	"testing"
 
 	. "github.com/drud/ddev/pkg/ddevapp"
+	"github.com/drud/ddev/pkg/dockerutil"
 	"github.com/drud/ddev/pkg/output"
 	"github.com/drud/ddev/pkg/testcommon"
 	"github.com/drud/ddev/pkg/util"
@@ -22,6 +23,16 @@ func TestMain(m *testing.M) {
 
 	// Ensure the ddev directory is created before tests run.
 	_ = util.GetGlobalDdevDir()
+
+	// Since this test may be first time ddev has been used, we need the
+	// ddev_default network available. This would normally be done in a
+	// TestMain, so can be moved to one when we need one.
+	dockerutil.EnsureDdevNetwork()
+
+	// Avoid having sudo try to add to /etc/hosts.
+	// This is normally done by Testsite.Prepare()
+	_ = os.Setenv("DRUD_NONINTERACTIVE", "true")
+
 	os.Exit(m.Run())
 }
 
@@ -181,7 +192,7 @@ func TestConfigCommand(t *testing.T) {
 	// Ensure we have expected vales in output.
 	assert.Contains(out, testDir)
 	assert.Contains(out, fmt.Sprintf("No directory could be found at %s", filepath.Join(testDir, invalidDir)))
-	assert.Contains(out, fmt.Sprintf("%s is not a valid application type", invalidAppType))
+	assert.Contains(out, fmt.Sprintf("'%s' is not a valid application type", invalidAppType))
 
 	// Ensure values were properly set on the config struct.
 	assert.Equal(name, config.Name)
@@ -210,7 +221,9 @@ func TestRead(t *testing.T) {
 	}
 
 	err := c.Read()
-	assert.NoError(err)
+	if err != nil {
+		t.Fatalf("Unable to c.Read(), err: %v", err)
+	}
 
 	// Values not defined in file, we should still have default values
 	assert.Equal(c.Name, "TestRead")
@@ -236,7 +249,9 @@ func TestValidate(t *testing.T) {
 	}
 
 	err = c.Validate()
-	assert.NoError(err)
+	if err != nil {
+		t.Fatalf("Failed to c.Validate(), err=%v", err)
+	}
 
 	c.Name = "Invalid!"
 	err = c.Validate()
@@ -250,7 +265,7 @@ func TestValidate(t *testing.T) {
 	c.Docroot = "testing"
 	c.AppType = "potato"
 	err = c.Validate()
-	assert.EqualError(err, fmt.Sprintf("%s is not a valid apptype", c.AppType))
+	assert.EqualError(err, fmt.Sprintf("'%s' is not a valid apptype", c.AppType))
 }
 
 // TestWrite tests writing config values to file

pkg/ddevapp/pantheon_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 
 	. "github.com/drud/ddev/pkg/ddevapp"
+	"github.com/drud/ddev/pkg/plugins/platform"
 	"github.com/drud/ddev/pkg/testcommon"
 	"github.com/drud/ddev/pkg/util"
 	asrt "github.com/stretchr/testify/assert"
@@ -128,3 +129,51 @@ func TestPantheonBackupLinks(t *testing.T) {
 	assert.Contains(backupLink, "database.sql.gz")
 	assert.NoError(err)
 }
+
+// TestPantheonPull ensures we can pull backups from pantheon for a configured environment.
+func TestPantheonPull(t *testing.T) {
+	if os.Getenv("DDEV_PANTHEON_API_TOKEN") == "" {
+		t.Skip("No DDEV_PANTHEON_API_TOKEN env var has been set. Skipping Pantheon specific test.")
+	}
+
+	// Set up tests and give ourselves a working directory.
+	assert := asrt.New(t)
+	testDir := testcommon.CreateTmpDir("TestPantheonPull")
+
+	// testcommon.Chdir()() and CleanupDir() checks their own errors (and exit)
+	defer testcommon.CleanupDir(testDir)
+	defer testcommon.Chdir(testDir)()
+
+	// Move into the properly named pantheon site (must match pantheon sitename)
+	siteDir := testDir + "/" + pantheonTestSiteName
+	err := os.MkdirAll(siteDir+"/sites/default", 0777)
+	assert.NoError(err)
+	err = os.Chdir(siteDir)
+	assert.NoError(err)
+
+	config, err := NewConfig(siteDir, "pantheon")
+	assert.NoError(err)
+	config.Name = pantheonTestSiteName
+	config.AppType = "drupal8"
+	err = config.Write()
+	assert.NoError(err)
+
+	testcommon.ClearDockerEnv()
+
+	provider := PantheonProvider{}
+	err = provider.Init(config)
+	assert.NoError(err)
+
+	provider.Sitename = pantheonTestSiteName
+	provider.EnvironmentName = pantheonTestEnvName
+	err = provider.Write(config.GetPath("import.yaml"))
+	assert.NoError(err)
+
+	// Ensure we can do a pull on the configured site.
+	app, err := platform.GetActiveApp("")
+	assert.NoError(err)
+	err = app.Import()
+	assert.NoError(err)
+	err = app.Down(true)
+	assert.NoError(err)
+}

pkg/dockerutil/dockerutils.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"log"
 	"os"
 	"os/exec"
 	"regexp"
@@ -39,6 +40,17 @@ func EnsureNetwork(client *docker.Client, name string) error {
 	return nil
 }
 
+// EnsureDdevNetwork just creates or ensures the ddev_default network exists or
+// exits with fatal.
+func EnsureDdevNetwork() {
+	// ensure we have docker network
+	client := GetDockerClient()
+	err := EnsureNetwork(client, NetName)
+	if err != nil {
+		log.Fatalf("Failed to ensure docker network %s: %v", NetName, err)
+	}
+}
+
 // GetDockerClient returns a docker client for a docker-machine.
 func GetDockerClient() *docker.Client {
 	client, err := docker.NewClientFromEnv()