Merge pull request #733 from kayac/fix/fail-by-cb

Exit non-zero status when deployment is rolled back.

create.go

@@ -93,7 +93,7 @@ func (d *App) createService(ctx context.Context, opt DeployOption) error {
 		return err
 	}
 
-	doWait, err := d.WaitFunc(sv)
+	doWait, err := d.WaitFunc(sv, nil)
 	if err != nil {
 		return err
 	}

deploy.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"io"
 	"os"
 	"os/exec"
 	"strings"
@@ -95,12 +96,13 @@ func (d *App) Deploy(ctx context.Context, opt DeployOption) error {
 	if err != nil {
 		return err
 	}
-	doWait, err := d.WaitFunc(sv)
+
+	tdArn, err := d.taskDefinitionArnForDeploy(ctx, sv, opt)
 	if err != nil {
 		return err
 	}
 
-	tdArn, err := d.taskDefinitionArnForDeploy(ctx, sv, opt)
+	doWait, err := d.WaitFunc(sv, d.confirmPrimaryTD(tdArn))
 	if err != nil {
 		return err
 	}
@@ -112,7 +114,7 @@ func (d *App) Deploy(ctx context.Context, opt DeployOption) error {
 			return err
 		}
 		addedTags, updatedTags, deletedTags := CompareTags(sv.Tags, newSv.Tags)
-		differ, err := diffServices(ctx, newSv, sv, d.config.ServiceDefinitionPath, &DiffOption{Unified: true, w: os.Stdout})
+		differ, err := diffServices(ctx, newSv, sv, d.config.ServiceDefinitionPath, &DiffOption{Unified: true, w: io.Discard})
 		if err != nil {
 			return fmt.Errorf("failed to diff of service definitions: %w", err)
 		}

ecspresso.go

@@ -68,6 +68,12 @@ func (sv *Service) SetTags(tags []types.Tag) {
 	sv.Tags = tags
 }
 
+func (sv *Service) PrimaryDeployment() (types.Deployment, bool) {
+	return lo.Find(sv.Deployments, func(dp types.Deployment) bool {
+		return aws.ToString(dp.Status) == "PRIMARY"
+	})
+}
+
 func (d *App) newServiceFromTypes(ctx context.Context, in types.Service) (*Service, error) {
 	sv := Service{
 		Service:      in,

rollback.go

@@ -49,14 +49,17 @@ func (d *App) Rollback(ctx context.Context, opt RollbackOption) error {
 	if err != nil {
 		return err
 	}
-
-	doWait, err := d.WaitFunc(sv)
+	targetArn, err := d.FindRollbackTarget(ctx, *sv.TaskDefinition)
+	if err != nil {
+		return err
+	}
+	doWait, err := d.WaitFunc(sv, d.confirmPrimaryTD(targetArn))
 	if err != nil {
 		return err
 	}
 
 	// doRollback returns the task definition arn to be rolled back
-	rollbackedTdArn, err := doRollback(ctx, sv, opt)
+	rollbackedTdArn, err := doRollback(ctx, sv, targetArn, opt)
 	if err != nil {
 		return err
 	}
@@ -111,12 +114,8 @@ func (d *App) rollbackTaskDefinition(ctx context.Context, rollbackedTdArn string
 	return nil
 }
 
-func (d *App) RollbackServiceTasks(ctx context.Context, sv *Service, opt RollbackOption) (string, error) {
+func (d *App) RollbackServiceTasks(ctx context.Context, sv *Service, targetArn string, opt RollbackOption) (string, error) {
 	currentArn := *sv.TaskDefinition
-	targetArn, err := d.FindRollbackTarget(ctx, currentArn)
-	if err != nil {
-		return "", err
-	}
 
 	d.Log("Rolling back to %s %s", arnToName(targetArn), opt.DryRunString())
 	if opt.DryRun {
@@ -138,7 +137,7 @@ func (d *App) RollbackServiceTasks(ctx context.Context, sv *Service, opt Rollbac
 	return currentArn, nil
 }
 
-func (d *App) RollbackByCodeDeploy(ctx context.Context, sv *Service, opt RollbackOption) (string, error) {
+func (d *App) RollbackByCodeDeploy(ctx context.Context, sv *Service, targetArn string, opt RollbackOption) (string, error) {
 	dp, err := d.findDeploymentInfo(ctx)
 	if err != nil {
 		return "", err
@@ -168,10 +167,6 @@ func (d *App) RollbackByCodeDeploy(ctx context.Context, sv *Service, opt Rollbac
 	switch currentDeployment.Status {
 	case cdTypes.DeploymentStatusSucceeded, cdTypes.DeploymentStatusFailed, cdTypes.DeploymentStatusStopped:
 		currentTdArn := *sv.TaskDefinition
-		targetArn, err := d.FindRollbackTarget(ctx, currentTdArn)
-		if err != nil {
-			return "", err
-		}
 		d.Log("the deployment in progress is not found, creating a new deployment with %s %s", targetArn, opt.DryRunString())
 		if opt.DryRun {
 			return currentTdArn, nil
@@ -216,7 +211,7 @@ func (d *App) FindRollbackTarget(ctx context.Context, taskDefinitionArn string)
 			},
 		)
 		if err != nil {
-			return "", fmt.Errorf("failed to list taskdefinitions: %w", err)
+			return "", fmt.Errorf("failed to list task definitions: %w", err)
 		}
 		if len(out.TaskDefinitionArns) == 0 {
 			return "", ErrNotFound(fmt.Sprintf("rollback target is not found: %s", err))
@@ -237,7 +232,7 @@ func (d *App) FindRollbackTarget(ctx context.Context, taskDefinitionArn string)
 	return "", ErrNotFound("rollback target is not found")
 }
 
-type rollbackFunc func(ctx context.Context, sv *Service, opt RollbackOption) (string, error)
+type rollbackFunc func(ctx context.Context, sv *Service, targetArn string, opt RollbackOption) (string, error)
 
 func (d *App) RollbackFunc(sv *Service) (rollbackFunc, error) {
 	defaultFunc := d.RollbackServiceTasks

tests/ci/Makefile

@@ -10,7 +10,7 @@ ECSPRESSO := ecspresso --envfile envfile
 
 help:
 
-test: deploy rollback refresh deploy-no-update-service scale down up delete
+test: deploy deploy rollback refresh deploy-no-update-service scale down up delete
 
 status:
 	$(ECSPRESSO) status --events 10
@@ -58,13 +58,13 @@ run-task:
 		--overrides '{"containerOverrides":[{"name":"nginx", "command":["nginx", "-V"]}]}' \
 		--client-token $(CLIENT_TOKEN)
 	OPTION=-v $(ECSPRESSO) --envfile envfile.override run \
-		--overrides-file=overrides.json \
+		--overrides-file=overrides.jsonnet \
 		--dry-run
 	OPTION=-v $(ECSPRESSO) --envfile envfile.override run \
-		--overrides-file=overrides.json \
+		--overrides-file=overrides.jsonnet \
 		--wait-until=running
 	OPTION=-v $(ECSPRESSO) --envfile envfile.override run \
-		--overrides-file=overrides.json \
+		--overrides-file=overrides.jsonnet \
 		--wait-until=stopped
 
 wait:

tests/ci/ecs-service-def.jsonnet

@@ -16,8 +16,8 @@ local isCodeDeploy = env('DEPLOYMENT_CONTROLLER', 'ECS') == 'CODE_DEPLOY';
   ],
   deploymentConfiguration: {
     deploymentCircuitBreaker: if isCodeDeploy then null else {
-      enable: false,
-      rollback: false,
+      enable: true,
+      rollback: true,
     },
     maximumPercent: 200,
     minimumHealthyPercent: 100,

tests/ci/overrides.jsonnet

@@ -0,0 +1,9 @@
+local must_env = std.native('must_env');
+{
+  containerOverrides: [
+    {
+      name: 'nginx',
+      command: ['nginx', must_env('OPTION')],
+    },
+  ],
+}

wait.go

@@ -19,8 +19,22 @@ import (
 
 type waitFunc func(ctx context.Context, sv *Service) error
 
-func (d *App) WaitFunc(sv *Service) (waitFunc, error) {
-	defaultFunc := d.WaitServiceStable
+type confirmFunc func(ctx context.Context) error
+
+func (confirm confirmFunc) wrap(wait waitFunc) waitFunc {
+	if confirm == nil {
+		return wait
+	}
+	return func(ctx context.Context, sv *Service) error {
+		if err := wait(ctx, sv); err != nil {
+			return err
+		}
+		return confirm(ctx)
+	}
+}
+
+func (d *App) WaitFunc(sv *Service, confirm confirmFunc) (waitFunc, error) {
+	defaultFunc := confirm.wrap(d.WaitServiceStable)
 	if sv == nil || sv.DeploymentController == nil {
 		return defaultFunc, nil
 	}
@@ -29,14 +43,33 @@ func (d *App) WaitFunc(sv *Service) (waitFunc, error) {
 		case types.DeploymentControllerTypeCodeDeploy:
 			return d.WaitForCodeDeploy, nil
 		case types.DeploymentControllerTypeEcs:
-			return d.WaitServiceStable, nil
+			return defaultFunc, nil
 		default:
 			return nil, fmt.Errorf("unsupported deployment controller type: %s", dc.Type)
 		}
 	}
 	return defaultFunc, nil
 }
 
+func (d *App) confirmPrimaryTD(tdArn string) confirmFunc {
+	return func(ctx context.Context) error {
+		sv, err := d.DescribeService(ctx)
+		if err != nil {
+			return err
+		}
+		if dp, ok := sv.PrimaryDeployment(); ok {
+			current := aws.ToString(dp.TaskDefinition)
+			d.Log("[DEBUG] checking primary deployment %s %s == %s", *dp.Id, current, tdArn)
+			if arnToName(current) != arnToName(tdArn) {
+				return fmt.Errorf("task definition %s is not deployed yet. PRIMARY deployment is %s", tdArn, current)
+			}
+			d.Log("[DEBUG] task definition %s is deployed", tdArn)
+			return nil
+		}
+		return fmt.Errorf("no primary deployment found")
+	}
+}
+
 type WaitOption struct {
 }
 
@@ -51,7 +84,7 @@ func (d *App) Wait(ctx context.Context, opt WaitOption) error {
 		return err
 	}
 	d.LogJSON(sv.DeploymentController)
-	doWait, err := d.WaitFunc(sv)
+	doWait, err := d.WaitFunc(sv, nil)
 	if err != nil {
 		return err
 	}