Fix problem where ddev-ssh-agent volume causes error, fixes #1312 (#1327

)

Includes test cleanups, hoping to make the webcaching test more stable.

cmd/ddev/cmd/root_test.go

@@ -1,20 +1,16 @@
 package cmd
 
 import (
-	"fmt"
 	"github.com/drud/ddev/pkg/fileutil"
 	"github.com/drud/ddev/pkg/globalconfig"
 	"os"
 	"testing"
 
-	"github.com/drud/ddev/pkg/dockerutil"
 	"github.com/drud/ddev/pkg/testcommon"
-	"github.com/drud/ddev/pkg/util"
 	log "github.com/sirupsen/logrus"
 
 	"path/filepath"
 
-	"github.com/drud/ddev/pkg/appports"
 	"github.com/drud/ddev/pkg/ddevapp"
 	"github.com/drud/ddev/pkg/exec"
 	"github.com/drud/ddev/pkg/output"
@@ -162,35 +158,6 @@ func addSites() error {
 		if err != nil {
 			log.Fatalln("Error Output from ddev start:", out, "err:", err)
 		}
-
-		app, err := ddevapp.GetActiveApp("")
-		if err != nil {
-			log.Fatalln("Could not find an active ddev configuration:", err)
-		}
-
-		// Warning: assumes web at port 80, will need adjustment in the future.
-		dockerIP, err := dockerutil.GetDockerIP()
-		if err != nil {
-			util.Warning("Unable to GetDockerIP: %v", err)
-		}
-
-		urls := []string{
-			"http://" + dockerIP + "/" + site.HTTPProbeURI,
-			"http://" + dockerIP + ":" + appports.GetPort("mailhog"),
-			"http://" + dockerIP + ":" + appports.GetPort("dba"),
-		}
-
-		for _, url := range urls {
-			o := util.NewHTTPOptions(url)
-			o.ExpectedStatus = 200
-			o.Timeout = 180
-			o.Headers["Host"] = app.HostName()
-			err = util.EnsureHTTPStatus(o)
-			if err != nil {
-				return fmt.Errorf("failed to ensureHTTPStatus on %s url=%s", app.HostName(), url)
-			}
-		}
-
 	}
 	return nil
 }

pkg/ddevapp/config.go

@@ -3,13 +3,14 @@ package ddevapp
 import (
 	"bytes"
 	"fmt"
-	"github.com/drud/ddev/pkg/globalconfig"
 	"html/template"
 	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strings"
 
+	"github.com/drud/ddev/pkg/globalconfig"
+
 	"regexp"
 
 	"runtime"
@@ -456,6 +457,24 @@ func (app *DdevApp) CheckCustomConfig() {
 
 }
 
+type composeYAMLVars struct {
+	Name            string
+	Plugin          string
+	AppType         string
+	MailhogPort     string
+	DBAPort         string
+	DBPort          string
+	DdevGenerated   string
+	ExtraHost       string
+	ComposeVersion  string
+	MountType       string
+	WebMount        string
+	OmitDBA         bool
+	OmitSSHAgent    bool
+	WebcacheEnabled bool
+	IsWindowsFS     bool
+}
+
 // RenderComposeYAML renders the contents of docker-compose.yaml.
 func (app *DdevApp) RenderComposeYAML() (string, error) {
 	var doc bytes.Buffer
@@ -467,6 +486,7 @@ func (app *DdevApp) RenderComposeYAML() (string, error) {
 	if err != nil {
 		return "", err
 	}
+
 	// Docker 18.03 on linux doesn't define host.docker.internal
 	// so we need to go get the ip address of docker0
 	// We would hope to be able to remove this when
@@ -484,38 +504,26 @@ func (app *DdevApp) RenderComposeYAML() (string, error) {
 		}
 	}
 
-	var includeDBA = "includeItByDefault"
-	if util.ArrayContainsString(app.OmitContainers, "dba") {
-		includeDBA = ""
-	}
-	var includeBGSYNC = ""
-	var mountType = "bind"
-	webMount := "../"
-	if app.WebcacheEnabled {
-		includeBGSYNC = "true"
-		webMount = "webcachevol"
-		mountType = "volume"
-	}
-
-	isWindowsFS := "false"
-	if runtime.GOOS == "windows" {
-		isWindowsFS = "true"
-	}
-	templateVars := map[string]string{
-		"name":            app.Name,
-		"plugin":          "ddev",
-		"appType":         app.Type,
-		"webMount":        webMount,
-		"mailhogport":     appports.GetPort("mailhog"),
-		"dbaport":         appports.GetPort("dba"),
-		"dbport":          appports.GetPort("db"),
-		"ddevgenerated":   DdevFileSignature,
-		"extra_host":      docker0Hostname + `:` + docker0Addr,
-		"compose_version": version.DockerComposeFileFormatVersion,
-		"IncludeDBA":      includeDBA,
-		"IncludeBGSYNC":   includeBGSYNC,
-		"IsWindowsFS":     isWindowsFS,
-		"mountType":       mountType,
+	templateVars := composeYAMLVars{
+		Name:            app.Name,
+		Plugin:          "ddev",
+		AppType:         app.Type,
+		MailhogPort:     appports.GetPort("mailhog"),
+		DBAPort:         appports.GetPort("dba"),
+		DBPort:          appports.GetPort("db"),
+		DdevGenerated:   DdevFileSignature,
+		ExtraHost:       docker0Hostname + `:` + docker0Addr,
+		ComposeVersion:  version.DockerComposeFileFormatVersion,
+		OmitDBA:         util.ArrayContainsString(app.OmitContainers, "dba"),
+		OmitSSHAgent:    util.ArrayContainsString(app.OmitContainers, "ddev-ssh-agent"),
+		WebcacheEnabled: app.WebcacheEnabled,
+		IsWindowsFS:     runtime.GOOS == "windows",
+		MountType:       "bind",
+		WebMount:        "../",
+	}
+	if templateVars.WebcacheEnabled {
+		templateVars.MountType = "volume"
+		templateVars.WebMount = "webcachevol"
 	}
 
 	err = templ.Execute(&doc, templateVars)

pkg/ddevapp/config_test.go

@@ -556,6 +556,8 @@ func TestConfigOverrideDetection(t *testing.T) {
 
 	site := TestSites[0]
 	switchDir := site.Chdir()
+	defer switchDir()
+
 	runTime := testcommon.TimeTrack(time.Now(), fmt.Sprintf("%s ConfigOverrideDetection", site.Name))
 
 	// Copy test overrides into the project .ddev directory
@@ -577,7 +579,7 @@ func TestConfigOverrideDetection(t *testing.T) {
 	assert.NoError(err)
 
 	restoreOutput := util.CaptureUserOut()
-	startErr := app.Start()
+	startErr := app.StartAndWaitForSync(2)
 	out := restoreOutput()
 	//nolint: errcheck
 	defer app.Down(true, false)
@@ -587,7 +589,7 @@ func TestConfigOverrideDetection(t *testing.T) {
 		logs, _ = GetErrLogsFromApp(app, startErr)
 	}
 
-	require.NoError(t, startErr, "app.Start() did not succeed: output:\n=====\n%s\n===== logs:\n========= logs =======\n%s\n========\n", out, logs)
+	require.NoError(t, startErr, "app.StartAndWaitForSync() did not succeed: output:\n=====\n%s\n===== logs:\n========= logs =======\n%s\n========\n", out, logs)
 
 	assert.Contains(out, "utf.cnf")
 	assert.Contains(out, "my-php.ini")
@@ -601,6 +603,5 @@ func TestConfigOverrideDetection(t *testing.T) {
 		assert.NotContains(out, "nginx-site.conf")
 	}
 	assert.Contains(out, "Custom configuration takes effect")
-	switchDir()
 	runTime()
 }

pkg/ddevapp/ddevapp_test.go

@@ -580,6 +580,9 @@ func TestDdevImportDB(t *testing.T) {
 			path := filepath.Join(testDir, "testdata", file)
 			err = app.ImportDB(path, "")
 			assert.NoError(err, "Failed to app.ImportDB path: %s err: %v", path, err)
+			if err != nil {
+				continue
+			}
 
 			// Test that a settings file has correct hash_salt format
 			switch app.Type {
@@ -665,7 +668,7 @@ func TestDdevOldMariaDB(t *testing.T) {
 	assert.NoError(err)
 	app.MariaDBVersion = ddevapp.MariaDB101
 	app.DBImage = version.GetDBImage(app.MariaDBVersion)
-	err = app.Start()
+	err = app.StartAndWaitForSync(2)
 	//nolint: errcheck
 	defer app.Down(true, false)
 
@@ -746,7 +749,7 @@ func TestDdevExportDB(t *testing.T) {
 	testcommon.ClearDockerEnv()
 	err := app.Init(site.Dir)
 	assert.NoError(err)
-	err = app.Start()
+	err = app.StartAndWaitForSync(0)
 	assert.NoError(err)
 	//nolint: errcheck
 	defer app.Down(true, false)
@@ -1650,12 +1653,19 @@ func TestCleanupWithoutCompose(t *testing.T) {
 	assert.NoError(err)
 
 	// Ensure we have a site started so we have something to cleanup
-	err = app.Start()
+	err = app.StartAndWaitForSync(2)
 	assert.NoError(err)
 	// Setup by creating temp directory and nesting a folder for our site.
 	tempPath := testcommon.CreateTmpDir("site-copy")
 	siteCopyDest := filepath.Join(tempPath, "site")
-	defer removeAllErrCheck(tempPath, assert)
+
+	//nolint: errcheck
+	defer os.RemoveAll(tempPath)
+	//nolint: errcheck
+	defer revertDir()
+	// Move the site directory back to its original location.
+	//nolint: errcheck
+	defer os.Rename(siteCopyDest, site.Dir)
 
 	// Move site directory to a temp directory to mimick a missing directory.
 	err = os.Rename(site.Dir, siteCopyDest)
@@ -1681,10 +1691,6 @@ func TestCleanupWithoutCompose(t *testing.T) {
 		assert.False(volume.Labels["com.docker.compose.project"] == "ddev"+strings.ToLower(app.GetName()))
 	}
 
-	revertDir()
-	// Move the site directory back to its original location.
-	err = os.Rename(siteCopyDest, site.Dir)
-	assert.NoError(err)
 }
 
 // TestGetappsEmpty ensures that GetApps returns an empty list when no applications are running.
@@ -1841,7 +1847,7 @@ func TestHttpsRedirection(t *testing.T) {
 		// Do a start on the configured site.
 		app, err = ddevapp.GetActiveApp("")
 		assert.NoError(err)
-		err = app.StartAndWaitForSync(2)
+		err = app.StartAndWaitForSync(5)
 		assert.NoError(err, "failed to StartAndWaitForSync on project %s webserverType=%s: %v", app.Name, webserverType, err)
 		if err != nil {
 			appLogs, getLogsErr := ddevapp.GetErrLogsFromApp(app, err)
@@ -1965,8 +1971,8 @@ func TestGetAllURLs(t *testing.T) {
 		err = app.WriteConfig()
 		assert.NoError(err)
 
-		err = app.Start()
-		assert.NoError(err)
+		err = app.StartAndWaitForSync(0)
+		require.NoError(t, err)
 
 		urls := app.GetAllURLs()
 
@@ -1983,9 +1989,10 @@ func TestGetAllURLs(t *testing.T) {
 		// Ensure urlMap contains direct address of the web container
 		webContainer, err := app.FindContainerByType("web")
 		assert.NoError(err)
+		require.NotEmpty(t, webContainer)
 
 		dockerIP, err := dockerutil.GetDockerIP()
-		assert.NoError(err)
+		require.NoError(t, err)
 
 		// Find HTTP port of web container
 		var port docker.APIPort