fix: Changes how the deploy version is detected in app deploy command

cli/app/deploy.go

@@ -46,7 +46,8 @@ checkout as-is. Recipe commit hashes are also supported as values for
 	ValidArgsFunction: func(
 		cmd *cobra.Command,
 		args []string,
-		toComplete string) ([]string, cobra.ShellCompDirective) {
+		toComplete string,
+	) ([]string, cobra.ShellCompDirective) {
 		switch l := len(args); l {
 		case 0:
 			return autocomplete.AppNameComplete()
@@ -99,27 +100,9 @@ checkout as-is. Recipe commit hashes are also supported as values for
 			log.Fatalf("%s is already deployed", app.Name)
 		}
 
-		if len(args) == 2 && args[1] != "" {
+		toDeployVersion, toDeployChaosVersion, err = getDeployVersion(args, deployMeta, app)
-			toDeployVersion = args[1]
+		if err != nil {
-		}
+			log.Fatal(err)
-
-		if !deployMeta.IsDeployed &&
-			toDeployVersion == "" &&
-			app.Recipe.EnvVersion != "" && !internal.IgnoreEnvVersion {
-			log.Debugf("new deployment, choosing .env version: %s", app.Recipe.EnvVersion)
-			toDeployVersion = app.Recipe.EnvVersion
-		}
-
-		if !internal.Chaos && toDeployVersion == "" {
-			if err := getLatestVersionOrCommit(app, &toDeployVersion); err != nil {
-				log.Fatal(err)
-			}
-		}
-
-		if internal.Chaos {
-			if err := getChaosVersion(app, &toDeployVersion, &toDeployChaosVersion); err != nil {
-				log.Fatal(err)
-			}
 		}
 
 		if !internal.Chaos {
@@ -271,32 +254,22 @@ func getChaosVersion(app app.App, toDeployVersion, toDeployChaosVersion *string)
 	return nil
 }
 
-func getLatestVersionOrCommit(app app.App, toDeployVersion *string) error {
+func getLatestVersionOrCommit(app app.App) (string, string, error) {
 	versions, err := app.Recipe.Tags()
 	if err != nil {
-		return err
+		return "", "", err
 	}
 
 	if len(versions) > 0 && !internal.Chaos {
-		*toDeployVersion = versions[len(versions)-1]
+		return versions[len(versions)-1], "", nil
-
-		log.Debugf("choosing %s as version to deploy", *toDeployVersion)
-
-		if _, err := app.Recipe.EnsureVersion(*toDeployVersion); err != nil {
-			return err
-		}
-
-		return nil
 	}
 
 	head, err := app.Recipe.Head()
 	if err != nil {
-		return err
+		return "", "", err
 	}
 
-	*toDeployVersion = formatter.SmallSHA(head.String())
+	return "", formatter.SmallSHA(head.String()), nil
-
-	return nil
 }
 
 // validateArgsAndFlags ensures compatible args/flags.
@@ -323,6 +296,50 @@ func validateSecrets(cl *dockerClient.Client, app app.App) error {
 	return nil
 }
 
+func getDeployVersion(cliArgs []string, deployMeta stack.DeployMeta, app app.App) (string, string, error) {
+	// Chaos mode overrides everything
+	if internal.Chaos {
+		v, err := app.Recipe.ChaosVersion()
+		if err != nil {
+			return "", "", err
+		}
+		cv, err := app.Recipe.GetVersionLabelLocal()
+		if err != nil {
+			return "", "", err
+		}
+		log.Debugf("version: taking chaos version: %s, %s", v, cv)
+		return v, cv, nil
+	}
+
+	// Check if the deploy version is set with a cli argument
+	if len(cliArgs) == 2 && cliArgs[1] != "" {
+		log.Debugf("version: taking version from cli arg: %s", cliArgs[1])
+		return cliArgs[1], "", nil
+	}
+
+	// Check if the recipe has a version in the .env file
+	if app.Recipe.EnvVersion != "" && !internal.IgnoreEnvVersion {
+		log.Debugf("version: taking version from .env file: %s", app.Recipe.EnvVersion)
+		return app.Recipe.EnvVersion, "", nil
+	}
+
+	// Take deployed version
+	if deployMeta.IsDeployed {
+		log.Debugf("version: taking deployed version: %s", deployMeta.Version)
+		return deployMeta.Version, "", nil
+	}
+
+	v, vc, err := getLatestVersionOrCommit(app)
+	log.Debugf("version: taking new recipe versio: %s, %s", v, vc)
+	if err != nil {
+		log.Fatal(err)
+	}
+	if v == "" {
+		return vc, vc, nil
+	}
+	return v, vc, nil
+}
+
 func init() {
 	AppDeployCommand.Flags().BoolVarP(
 		&internal.Chaos,

tests/integration/app_deploy_env_version.bats

@@ -54,13 +54,21 @@ teardown(){
 }
 
 # bats test_tags=slow
-@test "chaos commit written to env" {
+@test "deploy commit written to env and redeploy keeps that version" {
   run $ABRA app deploy "$TEST_APP_DOMAIN" "1e83340e" --no-input --no-converge-checks
   assert_success
 
   run grep -q "TYPE=$TEST_RECIPE:1e83340e" \
     "$ABRA_DIR/servers/$TEST_SERVER/$TEST_APP_DOMAIN.env"
   assert_success
+
+  run $ABRA app deploy "$TEST_APP_DOMAIN" \
+    --force --no-input --no-converge-checks
+  assert_success
+
+  run grep -q "TYPE=$TEST_RECIPE:1e83340e" \
+    "$ABRA_DIR/servers/$TEST_SERVER/$TEST_APP_DOMAIN.env"
+  assert_success
 }
 
 # bats test_tags=slow
@@ -98,12 +106,15 @@ teardown(){
 }
 
 # bats test_tags=slow
-@test "deploy overwrites chaos deploy" {
+@test "takes deployed version when no .env version is present " {
-  run $ABRA app deploy "$TEST_APP_DOMAIN" "1e83340e" \
+  run $ABRA app deploy "$TEST_APP_DOMAIN" "0.1.0+1.20.0" --no-input --no-converge-checks --ignore-env-version
-    --no-input --no-converge-checks
   assert_success
 
-  run grep -q "TYPE=$TEST_RECIPE:1e83340e" \
+  run grep -q "TYPE=$TEST_RECIPE:0.1.0+1.20.0" \
+    "$ABRA_DIR/servers/$TEST_SERVER/$TEST_APP_DOMAIN.env"
+  assert_success
+
+  run sed -i 's/TYPE=abra-test-recipe:.*/TYPE=abra-test-recipe/g' \
     "$ABRA_DIR/servers/$TEST_SERVER/$TEST_APP_DOMAIN.env"
   assert_success
 
@@ -111,7 +122,7 @@ teardown(){
     --force --no-input --no-converge-checks
   assert_success
 
-  run grep -q "TYPE=$TEST_RECIPE:1e83340e" \
+  run grep -q "TYPE=$TEST_RECIPE:0.1.0+1.20.0" \
     "$ABRA_DIR/servers/$TEST_SERVER/$TEST_APP_DOMAIN.env"
-  assert_failure
+  assert_success
 }