Merge pull request #1637 from porter-dev/nafees/preview-env-improvements

[POR-315] Do not build an image if one already exists in the registry

cli/cmd/apply.go

@@ -104,19 +104,53 @@ func apply(_ *types.GetAuthenticatedUserResponse, client *api.Client, args []str
 		return fmt.Errorf("namespace must be set by PORTER_NAMESPACE")
 	}
 
-	deploymentHook, err := NewDeploymentHook(client, resGroup, deplNamespace)
+	if hasDeploymentHookEnvVars() {
+		deploymentHook, err := NewDeploymentHook(client, resGroup, deplNamespace)
 
-	if err != nil {
-		return err
-	}
+		if err != nil {
+			return err
+		}
 
-	worker.RegisterHook("deployment", deploymentHook)
+		worker.RegisterHook("deployment", deploymentHook)
+	}
 
 	return worker.Apply(resGroup, &switchboardTypes.ApplyOpts{
 		BasePath: basePath,
 	})
 }
 
+func hasDeploymentHookEnvVars() bool {
+	if ghIDStr := os.Getenv("PORTER_GIT_INSTALLATION_ID"); ghIDStr == "" {
+		return false
+	}
+
+	if prIDStr := os.Getenv("PORTER_PULL_REQUEST_ID"); prIDStr == "" {
+		return false
+	}
+
+	if branchName := os.Getenv("PORTER_BRANCH_NAME"); branchName == "" {
+		return false
+	}
+
+	if actionIDStr := os.Getenv("PORTER_ACTION_ID"); actionIDStr == "" {
+		return false
+	}
+
+	if repoName := os.Getenv("PORTER_REPO_NAME"); repoName == "" {
+		return false
+	}
+
+	if repoOwner := os.Getenv("PORTER_REPO_OWNER"); repoOwner == "" {
+		return false
+	}
+
+	if prName := os.Getenv("PORTER_PR_NAME"); prName == "" {
+		return false
+	}
+
+	return true
+}
+
 type Source struct {
 	Name          string
 	Repo          string
@@ -135,6 +169,7 @@ type ApplicationConfig struct {
 	WaitForJob bool
 
 	Build struct {
+		ForceBuild bool
 		Method     string
 		Context    string
 		Dockerfile string
@@ -408,7 +443,7 @@ func (d *Driver) createApplication(resource *models.Resource, client *api.Client
 	if appConf.Build.Method == "registry" {
 		subdomain, err = createAgent.CreateFromRegistry(appConf.Build.Image, appConf.Values)
 	} else {
-		subdomain, err = createAgent.CreateFromDocker(appConf.Values, sharedOpts.OverrideTag, buildConfig)
+		subdomain, err = createAgent.CreateFromDocker(appConf.Values, sharedOpts.OverrideTag, buildConfig, appConf.Build.ForceBuild)
 	}
 
 	if err != nil {
@@ -456,7 +491,7 @@ func (d *Driver) updateApplication(resource *models.Resource, client *api.Client
 			}
 		}
 
-		err = updateAgent.Build(buildConfig)
+		err = updateAgent.Build(buildConfig, appConf.Build.ForceBuild)
 
 		if err != nil {
 			return nil, err
@@ -702,30 +737,24 @@ func NewDeploymentHook(client *api.Client, resourceGroup *switchboardTypes.Resou
 		namespace:     namespace,
 	}
 
-	if ghIDStr := os.Getenv("PORTER_GIT_INSTALLATION_ID"); ghIDStr != "" {
-		ghID, err := strconv.Atoi(ghIDStr)
+	ghIDStr := os.Getenv("PORTER_GIT_INSTALLATION_ID")
+	ghID, err := strconv.Atoi(ghIDStr)
 
-		if err != nil {
-			return nil, err
-		}
-
-		res.gitInstallationID = uint(ghID)
-	} else if ghIDStr == "" {
-		return nil, fmt.Errorf("Git installation ID must be defined, set by PORTER_GIT_INSTALLATION_ID")
+	if err != nil {
+		return nil, err
 	}
 
-	if prIDStr := os.Getenv("PORTER_PULL_REQUEST_ID"); prIDStr != "" {
-		prID, err := strconv.Atoi(prIDStr)
+	res.gitInstallationID = uint(ghID)
 
-		if err != nil {
-			return nil, err
-		}
+	prIDStr := os.Getenv("PORTER_PULL_REQUEST_ID")
+	prID, err := strconv.Atoi(prIDStr)
 
-		res.prID = uint(prID)
-	} else if prIDStr == "" {
-		return nil, fmt.Errorf("Pull request ID must be defined, set by PORTER_PULL_REQUEST_ID")
+	if err != nil {
+		return nil, err
 	}
 
+	res.prID = uint(prID)
+
 	res.projectID = config.Project
 
 	if res.projectID == 0 {
@@ -738,41 +767,26 @@ func NewDeploymentHook(client *api.Client, resourceGroup *switchboardTypes.Resou
 		return nil, fmt.Errorf("cluster id must be set")
 	}
 
-	if branchName := os.Getenv("PORTER_BRANCH_NAME"); branchName != "" {
-		res.branch = branchName
-	} else if branchName == "" {
-		return nil, fmt.Errorf("Branch name must be defined, set by PORTER_BRANCH_NAME")
-	}
-
-	if actionIDStr := os.Getenv("PORTER_ACTION_ID"); actionIDStr != "" {
-		actionID, err := strconv.Atoi(actionIDStr)
+	branchName := os.Getenv("PORTER_BRANCH_NAME")
+	res.branch = branchName
 
-		if err != nil {
-			return nil, err
-		}
+	actionIDStr := os.Getenv("PORTER_ACTION_ID")
+	actionID, err := strconv.Atoi(actionIDStr)
 
-		res.actionID = uint(actionID)
-	} else if actionIDStr == "" {
-		return nil, fmt.Errorf("Action Run ID must be defined, set by PORTER_ACTION_ID")
+	if err != nil {
+		return nil, err
 	}
 
-	if repoName := os.Getenv("PORTER_REPO_NAME"); repoName != "" {
-		res.repoName = repoName
-	} else if repoName == "" {
-		return nil, fmt.Errorf("Repo name must be defined, set by PORTER_REPO_NAME")
-	}
+	res.actionID = uint(actionID)
 
-	if repoOwner := os.Getenv("PORTER_REPO_OWNER"); repoOwner != "" {
-		res.repoOwner = repoOwner
-	} else if repoOwner == "" {
-		return nil, fmt.Errorf("Repo owner must be defined, set by PORTER_REPO_OWNER")
-	}
+	repoName := os.Getenv("PORTER_REPO_NAME")
+	res.repoName = repoName
 
-	if prName := os.Getenv("PORTER_PR_NAME"); prName != "" {
-		res.prName = prName
-	} else if prName == "" {
-		return nil, fmt.Errorf("PR Name must be supplied, set by PORTER_PR_NAME")
-	}
+	repoOwner := os.Getenv("PORTER_REPO_OWNER")
+	res.repoOwner = repoOwner
+
+	prName := os.Getenv("PORTER_PR_NAME")
+	res.prName = prName
 
 	commit, err := git.LastCommit()
 

cli/cmd/create.go

@@ -77,6 +77,7 @@ var values string
 var source string
 var image string
 var registryURL string
+var forceBuild bool
 
 func init() {
 	rootCmd.AddCommand(createCmd)
@@ -155,6 +156,13 @@ func init() {
 		"",
 		"the registry URL to use (must exist in \"porter registries list\")",
 	)
+
+	createCmd.PersistentFlags().BoolVar(
+		&forceBuild,
+		"force-build",
+		false,
+		"set this to force build an image",
+	)
 }
 
 var supportedKinds = map[string]string{"web": "", "job": "", "worker": ""}
@@ -217,7 +225,7 @@ func createFull(_ *types.GetAuthenticatedUserResponse, client *api.Client, args
 	}
 
 	if source == "local" {
-		subdomain, err := createAgent.CreateFromDocker(valuesObj, "default", nil)
+		subdomain, err := createAgent.CreateFromDocker(valuesObj, "default", nil, forceBuild)
 
 		return handleSubdomainCreate(subdomain, err)
 	} else if source == "github" {

cli/cmd/deploy.go

@@ -297,6 +297,13 @@ func init() {
 		"file destination for .env files",
 	)
 
+	updateCmd.PersistentFlags().BoolVar(
+		&forceBuild,
+		"force-build",
+		false,
+		"set this to force build an image",
+	)
+
 	updateCmd.AddCommand(updateBuildCmd)
 	updateCmd.AddCommand(updatePushCmd)
 	updateCmd.AddCommand(updateConfigCmd)
@@ -476,7 +483,7 @@ func updateBuildWithAgent(updateAgent *deploy.DeployAgent) error {
 		return err
 	}
 
-	if err := updateAgent.Build(nil); err != nil {
+	if err := updateAgent.Build(nil, forceBuild); err != nil {
 		if stream {
 			updateAgent.StreamEvent(types.SubEvent{
 				EventID: "build",

cli/cmd/deploy/create.go

@@ -219,6 +219,7 @@ func (c *CreateAgent) CreateFromDocker(
 	overrideValues map[string]interface{},
 	imageTag string,
 	extraBuildConfig *types.BuildConfig,
+	forceBuild bool,
 ) (string, error) {
 	opts := c.CreateOpts
 
@@ -272,59 +273,69 @@ func (c *CreateAgent) CreateFromDocker(
 		return "", err
 	}
 
-	env, err := GetEnvForRelease(c.Client, mergedValues, opts.ProjectID, opts.ClusterID, opts.Namespace)
+	imageExists, err := agent.CheckIfImageExists(fmt.Sprintf("%s:%s", imageURL, imageTag))
 
 	if err != nil {
-		env = map[string]string{}
+		return "", err
 	}
 
-	// add additional env based on options
-	for key, val := range opts.SharedOpts.AdditionalEnv {
-		env[key] = val
-	}
+	if imageExists && imageTag != "default" && !forceBuild {
+		fmt.Printf("%s:%s already exists in the registry, so skipping build\n", imageURL, imageTag)
+	} else { // image does not exist or has tag default so we (re)build one
+		env, err := GetEnvForRelease(c.Client, mergedValues, opts.ProjectID, opts.ClusterID, opts.Namespace)
 
-	buildAgent := &BuildAgent{
-		SharedOpts:  opts.SharedOpts,
-		client:      c.Client,
-		imageRepo:   imageURL,
-		env:         env,
-		imageExists: false,
-	}
+		if err != nil {
+			env = map[string]string{}
+		}
 
-	if opts.Method == DeployBuildTypeDocker {
-		basePath, err := filepath.Abs(".")
+		// add additional env based on options
+		for key, val := range opts.SharedOpts.AdditionalEnv {
+			env[key] = val
+		}
 
-		if err != nil {
-			return "", err
+		buildAgent := &BuildAgent{
+			SharedOpts:  opts.SharedOpts,
+			client:      c.Client,
+			imageRepo:   imageURL,
+			env:         env,
+			imageExists: false,
 		}
 
-		err = buildAgent.BuildDocker(agent, basePath, opts.LocalPath, opts.LocalDockerfile, imageTag, "")
-	} else {
-		err = buildAgent.BuildPack(agent, opts.LocalPath, imageTag, "", extraBuildConfig)
-	}
+		if opts.Method == DeployBuildTypeDocker {
+			basePath, err := filepath.Abs(".")
 
-	if err != nil {
-		return "", err
-	}
+			if err != nil {
+				return "", err
+			}
 
-	// create repository
-	err = c.Client.CreateRepository(
-		context.Background(),
-		opts.ProjectID,
-		regID,
-		&types.CreateRegistryRepositoryRequest{
-			ImageRepoURI: imageURL,
-		},
-	)
+			err = buildAgent.BuildDocker(agent, basePath, opts.LocalPath, opts.LocalDockerfile, imageTag, "")
+		} else {
+			err = buildAgent.BuildPack(agent, opts.LocalPath, imageTag, "", extraBuildConfig)
+		}
 
-	if err != nil {
-		return "", err
-	}
+		if err != nil {
+			return "", err
+		}
+
+		// create repository
+		err = c.Client.CreateRepository(
+			context.Background(),
+			opts.ProjectID,
+			regID,
+			&types.CreateRegistryRepositoryRequest{
+				ImageRepoURI: imageURL,
+			},
+		)
 
-	err = agent.PushImage(fmt.Sprintf("%s:%s", imageURL, imageTag))
+		if err != nil {
+			return "", err
+		}
 
-	if err != nil {
-		return "", err
+		err = agent.PushImage(fmt.Sprintf("%s:%s", imageURL, imageTag))
+
+		if err != nil {
+			return "", err
+		}
 	}
 
 	subdomain, err := c.CreateSubdomainIfRequired(mergedValues)

cli/cmd/deploy/deploy.go

@@ -219,11 +219,33 @@ func (d *DeployAgent) WriteBuildEnv(fileDest string) error {
 
 // Build uses the deploy agent options to build a new container image from either
 // buildpack or docker.
-func (d *DeployAgent) Build(overrideBuildConfig *types.BuildConfig) error {
+func (d *DeployAgent) Build(overrideBuildConfig *types.BuildConfig, forceBuild bool) error {
+	// retrieve current image to use for cache
+	currImageSection := d.release.Config["image"].(map[string]interface{})
+	currentTag := currImageSection["tag"].(string)
+
+	if d.tag == "" {
+		d.tag = currentTag
+	}
+
+	imageExists, err := d.agent.CheckIfImageExists(fmt.Sprintf("%s:%s", d.imageRepo, d.tag))
+
+	if err != nil {
+		return err
+	}
+
+	d.imageExists = imageExists
+
+	// we do not want to re-build an image
+	// FIXME: what if overrideBuildConfig == nil but the image stays the same?
+	if overrideBuildConfig == nil && d.imageExists && d.tag != "latest" && !forceBuild {
+		fmt.Printf("%s:%s already exists in the registry, so skipping build\n", d.imageRepo, d.tag)
+		return nil
+	}
+
 	// if build is not local, fetch remote source
 	var basePath string
 	buildCtx := d.opts.LocalPath
-	var err error
 
 	if !d.opts.Local {
 		repoSplit := strings.Split(d.release.GitActionConfig.GitRepo, "/")
@@ -265,24 +287,11 @@ func (d *DeployAgent) Build(overrideBuildConfig *types.BuildConfig) error {
 		}
 	}
 
-	// retrieve current image to use for cache
-	currImageSection := d.release.Config["image"].(map[string]interface{})
-	currentTag := currImageSection["tag"].(string)
-
-	if d.tag == "" {
-		d.tag = currentTag
-	}
-
 	currTag, err := d.pullCurrentReleaseImage()
 
 	// if image is not found, don't return an error
 	if err != nil && err != docker.PullImageErrNotFound {
 		return err
-	} else if err != nil && err == docker.PullImageErrNotFound {
-		fmt.Println("could not find image, moving to build step")
-		d.imageExists = false
-	} else if err == nil {
-		d.imageExists = true
 	}
 
 	buildAgent := &BuildAgent{

cli/cmd/docker/agent.go

@@ -158,6 +158,26 @@ var PullImageErrNotFound = fmt.Errorf("Requested image not found")
 
 var PullImageErrUnauthorized = fmt.Errorf("Could not pull image: unauthorized")
 
+// CheckIfImageExists checks if the image exists in the registry
+func (a *Agent) CheckIfImageExists(image string) (bool, error) {
+	encodedRegistryAuth, err := a.getEncodedRegistryAuth(image)
+
+	if err != nil {
+		return false, err
+	}
+
+	_, err = a.client.DistributionInspect(context.Background(), image, encodedRegistryAuth)
+
+	if err == nil {
+		return true, nil
+	} else if strings.Contains(err.Error(), "image not found") ||
+		strings.Contains(err.Error(), "does not exist in the registry") {
+		return false, nil
+	}
+
+	return false, err
+}
+
 // PullImage pulls an image specified by the image string
 func (a *Agent) PullImage(image string) error {
 	opts, err := a.getPullOptions(image)
@@ -219,17 +239,30 @@ func (a *Agent) getPullOptions(image string) (types.ImagePullOptions, error) {
 		return types.ImagePullOptions{}, nil
 	}
 
+	authConfigEncoded, err := a.getEncodedRegistryAuth(image)
+
+	if err != nil {
+		return types.ImagePullOptions{}, err
+	}
+
+	return types.ImagePullOptions{
+		RegistryAuth: authConfigEncoded,
+		Platform:     "linux/amd64",
+	}, nil
+}
+
+func (a *Agent) getEncodedRegistryAuth(image string) (string, error) {
 	// get using server url
 	serverURL, err := GetServerURLFromTag(image)
 
 	if err != nil {
-		return types.ImagePullOptions{}, err
+		return "", err
 	}
 
 	user, secret, err := a.authGetter.GetCredentials(serverURL)
 
 	if err != nil {
-		return types.ImagePullOptions{}, err
+		return "", err
 	}
 
 	var authConfig = types.AuthConfig{
@@ -239,12 +272,8 @@ func (a *Agent) getPullOptions(image string) (types.ImagePullOptions, error) {
 	}
 
 	authConfigBytes, _ := json.Marshal(authConfig)
-	authConfigEncoded := base64.URLEncoding.EncodeToString(authConfigBytes)
 
-	return types.ImagePullOptions{
-		RegistryAuth: authConfigEncoded,
-		Platform:     "linux/amd64",
-	}, nil
+	return base64.URLEncoding.EncodeToString(authConfigBytes), nil
 }
 
 func (a *Agent) getPushOptions(image string) (types.ImagePushOptions, error) {