Exit when CD task is done #1056
Conversation
|
@lionello I've made updates here, please have a look at the changes. |
src/pkg/cli/getDeploymentStatus.go
Outdated
| if DoDryRun { | ||
| return ErrDryRun | ||
| } |
There was a problem hiding this comment.
Should we push this up into the caller? If we know it's a dry run, we probably don't even need to invoke WaitCdTaskState, (and print the debug message above) right?
src/pkg/cli/composeUp.go
Outdated
| @@ -189,20 +190,27 @@ func WaitAndTail(ctx context.Context, project *compose.Project, client client.Fa | |||
| ctx, cancelTail := context.WithCancelCause(ctx) | |||
| defer cancelTail(nil) // to cancel WaitServiceState and clean-up context | |||
|
|
|||
| deploymentStatusCh := make(chan error, 2) | |||
There was a problem hiding this comment.
This is last referenced about 50 lines later. Do you think we can factor out a function to set up the channel, the wait group, the listener goroutines, and maybe return a cleanup function that we can call at the end?
src/pkg/cli/client/byoc/aws/byoc.go
Outdated
| var cancel context.CancelCauseFunc | ||
| ctx, cancel = context.WithCancelCause(ctx) | ||
| go func() { | ||
| if err := ecs.WaitForTask(ctx, taskArn, 2*time.Second); err != nil { | ||
| if stopWhenCDTaskDone || errors.As(err, &ecs.TaskFailure{}) { | ||
| isTaskFailure := !errors.Is(err, io.EOF) |
There was a problem hiding this comment.
I'm confused to read that the name isTaskFailure now means notEOF. It seems like errors.As(err, &ecs.TaskFailure{}) is stronger assertion than !errors.Is(err, io.EOF). So, why was this change made?
* Revert "Revert "Exit when CD task is done (#1056)" (#1073)" This reverts commit 5d5e61a. * review update * review updates * clean up workgroup go code fix DO so the serverstream will handle cancel * use testdata dockerignore from main * clean and refactor * review update * remove warning, cancel of tail does not always mean deployment not completed * review updates * review updates * Stream tests * fix CD deployment failure case --------- Co-authored-by: Eric Liu <[email protected]>
Description
Include wait for CD service to complete as well as the services health check. Resolves an issues where the CD service fails (i.e. setting up ALBs, S3, etc..) but services are still running.
Linked Issues
fixes #1058
Checklist