Do not re-deploy first version of the app when preflights succeed

kotsadm/pkg/downstream/downstream.go

@@ -154,6 +154,9 @@ func GetDownstreamVersionStatus(appID string, sequence int64) (string, error) {
 	var status sql.NullString
 	err := row.Scan(&status)
 	if err != nil {
+		if err == sql.ErrNoRows {
+			return "", nil
+		}
 		return "", errors.Wrap(err, "failed to get downstream version")
 	}
 

kotsadm/pkg/preflight/execute.go

@@ -7,7 +7,6 @@ import (
 	"github.com/pkg/errors"
 	"github.com/replicatedhq/kots/kotsadm/pkg/logger"
 	"github.com/replicatedhq/kots/kotsadm/pkg/store"
-	"github.com/replicatedhq/kots/kotsadm/pkg/version"
 	troubleshootv1beta1 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta1"
 	"github.com/replicatedhq/troubleshoot/pkg/preflight"
 	troubleshootpreflight "github.com/replicatedhq/troubleshoot/pkg/preflight"
@@ -17,7 +16,7 @@ import (
 
 // execute will execute the preflights using spec in preflightSpec.
 // This spec should be rendered, no template functions remaining
-func execute(appID string, sequence int64, preflightSpec *troubleshootv1beta1.Preflight, ignorePermissionErrors bool) error {
+func execute(appID string, sequence int64, preflightSpec *troubleshootv1beta1.Preflight, ignorePermissionErrors bool) (*troubleshootpreflight.UploadPreflightResults, error) {
 	logger.Debug("executing preflight checks",
 		zap.String("appID", appID),
 		zap.Int64("sequence", sequence))
@@ -38,7 +37,7 @@ func execute(appID string, sequence int64, preflightSpec *troubleshootv1beta1.Pr
 
 	restConfig, err := rest.InClusterConfig()
 	if err != nil {
-		return errors.Wrap(err, "failed to read in cluster config")
+		return nil, errors.Wrap(err, "failed to read in cluster config")
 	}
 
 	collectOpts := troubleshootpreflight.CollectOpts{
@@ -51,7 +50,7 @@ func execute(appID string, sequence int64, preflightSpec *troubleshootv1beta1.Pr
 	logger.Debug("preflight collect phase")
 	collectResults, err := troubleshootpreflight.Collect(collectOpts, preflightSpec)
 	if err != nil && !isPermissionsError(err) {
-		return errors.Wrap(err, "failed to collect")
+		return nil, errors.Wrap(err, "failed to collect")
 	}
 
 	uploadPreflightResults := &troubleshootpreflight.UploadPreflightResults{}
@@ -92,54 +91,14 @@ func execute(appID string, sequence int64, preflightSpec *troubleshootv1beta1.Pr
 	logger.Debug("preflight marshalling")
 	b, err := json.Marshal(uploadPreflightResults)
 	if err != nil {
-		return errors.Wrap(err, "failed to marshal results")
+		return uploadPreflightResults, errors.Wrap(err, "failed to marshal results")
 	}
 
 	if err := store.GetStore().SetPreflightResults(appID, sequence, b); err != nil {
-		return errors.Wrap(err, "failed to set preflight results")
+		return uploadPreflightResults, errors.Wrap(err, "failed to set preflight results")
 	}
 
-	// deploy first version if preflight checks passed
-	err = maybeDeployFirstVersion(appID, sequence, uploadPreflightResults)
-	if err != nil {
-		return errors.Wrap(err, "failed to deploy first version")
-	}
-
-	return nil
-}
-
-func maybeDeployFirstVersion(appID string, sequence int64, preflightResults *troubleshootpreflight.UploadPreflightResults) error {
-	if sequence != 0 {
-		return nil
-	}
-
-	preflightState := getPreflightState(preflightResults)
-	if preflightState != "pass" {
-		return nil
-	}
-
-	return version.DeployVersion(appID, sequence)
-}
-
-func getPreflightState(preflightResults *troubleshootpreflight.UploadPreflightResults) string {
-	if len(preflightResults.Errors) > 0 {
-		return "fail"
-	}
-
-	if len(preflightResults.Results) == 0 {
-		return "pass"
-	}
-
-	state := "pass"
-	for _, result := range preflightResults.Results {
-		if result.IsFail {
-			return "fail"
-		} else if result.IsWarn {
-			state = "warn"
-		}
-	}
-
-	return state
+	return uploadPreflightResults, nil
 }
 
 func isPermissionsError(err error) bool {

kotsadm/pkg/preflight/preflight.go

@@ -8,6 +8,7 @@ import (
 	"github.com/replicatedhq/kots/kotsadm/pkg/render"
 	"github.com/replicatedhq/kots/kotsadm/pkg/store"
 	"github.com/replicatedhq/kots/kotsadm/pkg/version"
+	troubleshootpreflight "github.com/replicatedhq/troubleshoot/pkg/preflight"
 	"go.uber.org/zap"
 )
 
@@ -17,7 +18,7 @@ func Run(appID string, sequence int64, archiveDir string) error {
 		return errors.Wrap(err, "failed to load rendered kots kinds")
 	}
 
-	status, err := downstream.GetDownstreamVersionStatus(appID, int64(sequence))
+	status, err := downstream.GetDownstreamVersionStatus(appID, sequence)
 	if err != nil {
 		return errors.Wrapf(err, "failed to check downstream version %d status", sequence)
 	}
@@ -32,11 +33,11 @@ func Run(appID string, sequence int64, archiveDir string) error {
 
 	if renderedKotsKinds.Preflight != nil {
 		// set the status to pending_preflights
-		if err := downstream.SetDownstreamVersionPendingPreflight(appID, int64(sequence)); err != nil {
+		if err := downstream.SetDownstreamVersionPendingPreflight(appID, sequence); err != nil {
 			return errors.Wrapf(err, "failed to set downstream version %d pending preflight", sequence)
 		}
 
-		ignoreRBAC, err := downstream.GetIgnoreRBACErrors(appID, int64(sequence))
+		ignoreRBAC, err := downstream.GetIgnoreRBACErrors(appID, sequence)
 		if err != nil {
 			return errors.Wrap(err, "failed to get ignore rbac flag")
 		}
@@ -64,22 +65,89 @@ func Run(appID string, sequence int64, archiveDir string) error {
 
 		go func() {
 			logger.Debug("preflight checks beginning")
-			if err := execute(appID, sequence, p, ignoreRBAC); err != nil {
+			uploadPreflightResults, err := execute(appID, sequence, p, ignoreRBAC)
+			if err != nil {
+				err = errors.Wrap(err, "failed to run preflight checks")
 				logger.Error(err)
 				return
 			}
-
 			logger.Debug("preflight checks completed")
+
+			err = maybeDeployFirstVersion(appID, sequence, uploadPreflightResults)
+			if err != nil {
+				err = errors.Wrap(err, "failed to deploy first version")
+				logger.Error(err)
+				return
+			}
 		}()
 	} else if sequence == 0 {
-		if err := version.DeployVersion(appID, int64(sequence)); err != nil {
+		err := maybeDeployFirstVersion(appID, sequence, &troubleshootpreflight.UploadPreflightResults{})
+		if err != nil {
 			return errors.Wrap(err, "failed to deploy first version")
 		}
 	} else {
-		if err := downstream.SetDownstreamVersionReady(appID, int64(sequence)); err != nil {
-			return errors.Wrap(err, "failed to set downstream version ready")
+		status, err := downstream.GetDownstreamVersionStatus(appID, sequence)
+		if err != nil {
+			return errors.Wrap(err, "failed to get version status")
+		}
+		if status != "deployed" {
+			if err := downstream.SetDownstreamVersionReady(appID, sequence); err != nil {
+				return errors.Wrap(err, "failed to set downstream version ready")
+			}
 		}
 	}
 
 	return nil
 }
+
+// maybeDeployFirstVersion will deploy the first version if
+// 1. preflight checks pass
+// 2. we have not already deployed it
+func maybeDeployFirstVersion(appID string, sequence int64, preflightResults *troubleshootpreflight.UploadPreflightResults) error {
+	if sequence != 0 {
+		return nil
+	}
+
+	app, err := store.GetStore().GetApp(appID)
+	if err != nil {
+		return errors.Wrap(err, "failed to get app")
+	}
+
+	// do not revert to first version
+	if app.CurrentSequence != 0 {
+		return nil
+	}
+
+	preflightState := getPreflightState(preflightResults)
+	if preflightState != "pass" {
+		return nil
+	}
+
+	logger.Debug("automatically deploying first app version")
+
+	// note: this may attempt to re-deploy the first version but the operator will take care of
+	// comparing the version to current
+
+	return version.DeployVersion(appID, sequence)
+}
+
+func getPreflightState(preflightResults *troubleshootpreflight.UploadPreflightResults) string {
+	if len(preflightResults.Errors) > 0 {
+		return "fail"
+	}
+
+	if len(preflightResults.Results) == 0 {
+		return "pass"
+	}
+
+	state := "pass"
+	for _, result := range preflightResults.Results {
+		if result.IsFail {
+			return "fail"
+		} else if result.IsWarn {
+			state = "warn"
+		}
+	}
+
+	return state
+}

kotsadm/pkg/preflight/preflight_test.go

@@ -0,0 +1,75 @@
+package preflight
+
+import (
+	"testing"
+
+	troubleshootpreflight "github.com/replicatedhq/troubleshoot/pkg/preflight"
+)
+
+func Test_getPreflightState(t *testing.T) {
+	tests := []struct {
+		name             string
+		preflightResults *troubleshootpreflight.UploadPreflightResults
+		want             string
+	}{
+		{
+			name: "pass",
+			preflightResults: &troubleshootpreflight.UploadPreflightResults{
+				Results: []*troubleshootpreflight.UploadPreflightResult{
+					{},
+					{},
+					{},
+				},
+			},
+			want: "pass",
+		},
+		{
+			name: "warn",
+			preflightResults: &troubleshootpreflight.UploadPreflightResults{
+				Results: []*troubleshootpreflight.UploadPreflightResult{
+					{},
+					{IsWarn: true},
+					{},
+				},
+			},
+			want: "warn",
+		},
+		{
+			name: "fail",
+			preflightResults: &troubleshootpreflight.UploadPreflightResults{
+				Results: []*troubleshootpreflight.UploadPreflightResult{
+					{},
+					{IsFail: true},
+					{},
+				},
+			},
+			want: "fail",
+		},
+		{
+			name: "error",
+			preflightResults: &troubleshootpreflight.UploadPreflightResults{
+				Results: []*troubleshootpreflight.UploadPreflightResult{
+					{},
+					{IsWarn: true},
+					{},
+				},
+				Errors: []*troubleshootpreflight.UploadPreflightError{
+					{},
+				},
+			},
+			want: "fail",
+		},
+		{
+			name:             "empty",
+			preflightResults: &troubleshootpreflight.UploadPreflightResults{},
+			want:             "pass",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := getPreflightState(tt.preflightResults); got != tt.want {
+				t.Errorf("getPreflightState() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}