Refactor spinwick logic

This change cleans up core spinwick creation logic in preparation
for adding tests.

internal/cloudtools/installation.go

@@ -1,39 +1,34 @@
 package cloudtools
 
 import (
-	"fmt"
-
-	cloudModel "github.com/mattermost/mattermost-cloud/model"
+	cloud "github.com/mattermost/mattermost-cloud/model"
+	"github.com/pkg/errors"
 )
 
-// GetInstallationIDFromOwnerID returns the ID of an installation that matches
-// a given OwnerID. Multiple matches will return an error. No match will return
+// GetInstallationIDFromOwnerID returns the installation that matches a given
+// OwnerID. Multiple matches will return an error. No match will return
 // an empty ID and no error.
-func GetInstallationIDFromOwnerID(serverURL, awsAPIKey, ownerID string) (string, string, error) {
+func GetInstallationIDFromOwnerID(serverURL, awsAPIKey, ownerID string) (*cloud.InstallationDTO, error) {
 	headers := map[string]string{
 		"x-api-key": awsAPIKey,
 	}
-	cloudClient := cloudModel.NewClientWithHeaders(serverURL, headers)
-	installations, err := cloudClient.GetInstallations(&cloudModel.GetInstallationsRequest{
-		OwnerID: ownerID,
-		Paging: cloudModel.Paging{
-			Page:           0,
-			PerPage:        100,
-			IncludeDeleted: false,
-		},
+	cloudClient := cloud.NewClientWithHeaders(serverURL, headers)
+	installations, err := cloudClient.GetInstallations(&cloud.GetInstallationsRequest{
+		OwnerID:                     ownerID,
+		Paging:                      cloud.AllPagesNotDeleted(),
 		IncludeGroupConfig:          false,
 		IncludeGroupConfigOverrides: false,
 	})
 	if err != nil {
-		return "", "", err
+		return nil, errors.Wrap(err, "failed to retrieve installations from provisioner")
 	}
 
 	if len(installations) == 0 {
-		return "", "", nil
+		return nil, nil
 	}
 	if len(installations) == 1 {
-		return installations[0].ID, installations[0].Image, nil
+		return installations[0], nil
 	}
 
-	return "", "", fmt.Errorf("found %d installations with ownerID %s", len(installations), ownerID)
+	return nil, errors.Errorf("found %d installations with ownerID %s", len(installations), ownerID)
 }

server/builds.go

@@ -19,7 +19,7 @@ type Builds struct{}
 type buildsInterface interface {
 	getInstallationVersion(pr *model.PullRequest) string
 	dockerRegistryClient(s *Server) (*registry.Registry, error)
-	waitForImage(ctx context.Context, s *Server, reg *registry.Registry, pr *model.PullRequest, imageToCheck string, logger logrus.FieldLogger) (*model.PullRequest, error)
+	waitForImage(ctx context.Context, reg *registry.Registry, desiredTag, imageToCheck string, logger logrus.FieldLogger) error
 }
 
 func (b *Builds) getInstallationVersion(pr *model.PullRequest) string {
@@ -39,23 +39,22 @@ func (b *Builds) dockerRegistryClient(s *Server) (reg *registry.Registry, err er
 	return reg, nil
 }
 
-func (b *Builds) waitForImage(ctx context.Context, s *Server, reg *registry.Registry, pr *model.PullRequest, imageToCheck string, logger logrus.FieldLogger) (*model.PullRequest, error) {
-	desiredTag := b.getInstallationVersion(pr)
+func (b *Builds) waitForImage(ctx context.Context, reg *registry.Registry, desiredTag, imageToCheck string, logger logrus.FieldLogger) error {
 	logger = logger.WithFields(logrus.Fields{"image": imageToCheck, "tag": desiredTag})
 
 	for {
 		select {
 		case <-ctx.Done():
-			return pr, errors.New("timed out waiting for image to publish")
+			return errors.New("timed out waiting for image to publish")
 		case <-time.After(30 * time.Second):
 			_, err := reg.ManifestDigest(imageToCheck, desiredTag)
 			if err != nil && !strings.Contains(err.Error(), "status=404") {
-				return pr, errors.Wrap(err, "unable to fetch tag from docker registry")
+				return errors.Wrap(err, "unable to fetch tag from docker registry")
 			}
 
 			if err == nil {
 				logger.Info("Docker tag found!")
-				return pr, nil
+				return nil
 			}
 
 			logger.Debug("Docker tag for the build not found. Waiting...")

server/builds_mocked.go

@@ -23,6 +23,6 @@ func (b *MockedBuilds) dockerRegistryClient(s *Server) (*registry.Registry, erro
 	return nil, nil
 }
 
-func (b *MockedBuilds) waitForImage(ctx context.Context, s *Server, reg *registry.Registry, pr *model.PullRequest, imageToCheck string, logger logrus.FieldLogger) (*model.PullRequest, error) {
-	return pr, nil
+func (b *MockedBuilds) waitForImage(ctx context.Context, reg *registry.Registry, desiredTag, imageToCheck string, logger logrus.FieldLogger) error {
+	return nil
 }

server/pull_request.go

@@ -67,7 +67,6 @@ func (s *Server) handlePullRequestEvent(event *github.PullRequestEvent) {
 	case "synchronize":
 		logger.Info("PR has a new commit")
 		if s.isSpinWickLabelInLabels(pr.Labels) {
-			logger.Info("PR has a SpinWick label, starting upgrade")
 			if s.isSpinWickHALabel(pr.Labels) {
 				s.handleUpdateSpinWick(pr, true, false)
 			} else if s.isSpinWickCloudWithCWSLabel(pr.Labels) {