Skip to content

Notify user if deployment will not be cancelled when terminating. #875

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Notify user if deployment will not be cancelled when terminating. #875

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
28d8ad9
share a context between deploy and tail
jordanstephens Nov 19, 2024
8d0b9c0
notify user that deployment will not be cancelled
jordanstephens Nov 19, 2024
148ccf7
remove redundant messages
jordanstephens Nov 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/cmd/cli/command/commands.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func Execute(ctx context.Context) error {

var cerr *cli.CancelError
if errors.As(err, &cerr) {
printDefangHint("Detached. The process will keep running.\nTo continue the logs from where you left off, do:", cerr.Error())
printDefangHint("To continue the logs from where you left off, do:", cerr.Error())
}

code := connect.CodeOf(err)
Expand Down
32 changes: 14 additions & 18 deletions src/cmd/cli/command/compose.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,11 @@ func makeComposeUpCmd() *cobra.Command {
if err != nil {
return err
}
deploy, project, err := cli.ComposeUp(cmd.Context(), loader, client, provider, upload, mode.Value())

ctx, cancelTail := context.WithCancelCause(cmd.Context())
defer cancelTail(nil) // to cancel WaitServiceState and clean-up context

deploy, project, err := cli.ComposeUp(ctx, loader, client, provider, upload, mode.Value())
if err != nil {
if !nonInteractive && strings.Contains(err.Error(), "maximum number of projects") {
if resp, err2 := provider.GetServices(cmd.Context(), &defangv1.GetServicesRequest{Project: project.Name}); err2 == nil {
Expand Down Expand Up @@ -102,20 +105,17 @@ func makeComposeUpCmd() *cobra.Command {
return nil
}

tailCtx, cancelTail := context.WithCancelCause(cmd.Context())
defer cancelTail(nil) // to cancel WaitServiceState and clean-up context

if waitTimeout >= 0 {
var cancelTimeout context.CancelFunc
tailCtx, cancelTimeout = context.WithTimeout(tailCtx, time.Duration(waitTimeout)*time.Second)
ctx, cancelTimeout = context.WithTimeout(ctx, time.Duration(waitTimeout)*time.Second)
defer cancelTimeout()
}

errCompleted := errors.New("deployment succeeded") // tail canceled because of deployment completion
const targetState = defangv1.ServiceState_DEPLOYMENT_COMPLETED

go func() {
if err := cli.WaitServiceState(tailCtx, provider, targetState, deploy.Etag, unmanagedServices); err != nil {
if err := cli.WaitServiceState(ctx, provider, targetState, deploy.Etag, unmanagedServices); err != nil {
var errDeploymentFailed cli.ErrDeploymentFailed
if errors.As(err, &errDeploymentFailed) {
cancelTail(err)
Expand Down Expand Up @@ -143,46 +143,42 @@ func makeComposeUpCmd() *cobra.Command {
}

// blocking call to tail
if err := cli.Tail(tailCtx, loader, provider, tailParams); err != nil {
if err := cli.Tail(ctx, loader, provider, tailParams); err != nil {
term.Debug("Tail stopped with", err)

if connect.CodeOf(err) == connect.CodePermissionDenied {
// If tail fails because of missing permission, we wait for the deployment to finish
term.Warn("Unable to tail logs. Waiting for the deployment to finish.")
<-tailCtx.Done()
<-ctx.Done()
// Get the actual error from the context so we won't print "Error: missing tail permission"
err = context.Cause(tailCtx)
} else if !(errors.Is(tailCtx.Err(), context.Canceled) || errors.Is(tailCtx.Err(), context.DeadlineExceeded)) {
err = context.Cause(ctx)
} else if !(errors.Is(ctx.Err(), context.Canceled) || errors.Is(ctx.Err(), context.DeadlineExceeded)) {
return err // any error other than cancelation
}

// The tail was canceled; check if it was because of deployment failure or explicit cancelation or wait-timeout reached
if errors.Is(context.Cause(tailCtx), context.Canceled) {
// Tail was canceled by the user before deployment completion/failure; show a warning and exit with an error
term.Warn("Deployment is not finished. Service(s) might not be running.")
return err
Comment on lines -160 to -163
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this removed?

} else if errors.Is(context.Cause(tailCtx), context.DeadlineExceeded) {
if errors.Is(context.Cause(ctx), context.DeadlineExceeded) {
// Tail was canceled when wait-timeout is reached; show a warning and exit with an error
term.Warn("Wait-timeout exceeded, detaching from logs. Deployment still in progress.")
return err
}

var errDeploymentFailed cli.ErrDeploymentFailed
if errors.As(context.Cause(tailCtx), &errDeploymentFailed) {
if errors.As(context.Cause(ctx), &errDeploymentFailed) {
// Tail got canceled because of deployment failure: prompt to show the debugger
term.Warn(errDeploymentFailed)
if !nonInteractive {
failedServices := []string{errDeploymentFailed.Service}
track.Evt("Debug Prompted", P("failedServices", failedServices), P("etag", deploy.Etag), P("reason", errDeploymentFailed))
// Call the AI debug endpoint using the original command context (not the tailCtx which is canceled)
// Call the AI debug endpoint using the original command context (not the ctx which is canceled)
_ = cli.InteractiveDebug(cmd.Context(), loader, client, provider, deploy.Etag, project, failedServices)
}
return err
}
}

// Print the current service states of the deployment
if errors.Is(context.Cause(tailCtx), errCompleted) {
if errors.Is(context.Cause(ctx), errCompleted) {
for _, service := range deploy.Services {
service.State = targetState
}
Expand Down
8 changes: 8 additions & 0 deletions src/pkg/cli/composeUp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cli

import (
"context"
"errors"
"fmt"

"github.com/DefangLabs/defang/src/pkg/cli/client"
Expand Down Expand Up @@ -96,6 +97,13 @@ func ComposeUp(ctx context.Context, loader client.Loader, c client.FabricClient,
}
deployRequest.DelegationSetId = delegation.DelegationSetId
}
go func() {
<-ctx.Done()

if errors.Is(ctx.Err(), context.Canceled) || errors.Is(ctx.Err(), context.DeadlineExceeded) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not enough to exclude the natural cancellation of the context from success :(

defang up
 * Using Defang Playground provider from stored preference
 * Packaging the project files for app at /Users/jordan/wk/samples/samples/nodejs-http/app
 * Uploading the project files for app
 * Monitor your services' status in the defang portal
   - https://portal.defang.dev/service/app
 * Tailing logs for deployment ID qqut567fztmx ; press Ctrl+C to detach:
 * Showing only build logs and runtime errors. Press V to toggle verbose mode.
2024-11-19T15:52:44.499-08:00 cd Update started for stack jordanstephens-prod1
2024-11-19T15:52:48.671-08:00 cd  ** Building image for "app"...
2024-11-19T15:52:51.878-08:00 cd  ** Build succeeded for "app"
2024-11-19T15:52:52.356-08:00 cd  ** Updated service "app" to revision 50
2024-11-19T15:52:56.752-08:00 cd Update succeeded in 12.284256189s ; provisioning...
2024-11-19T15:53:39.389-08:00 app Server running at http://127.0.0.1:3000/
errCompleted
 ! Deployment will not be cancelled.
 * Service app is in state DEPLOYMENT_COMPLETED and will be available at:
   - https://jordanstephens-app--3000.prod1a.defang.dev
 * Done.
For help with warnings, check our FAQ at https://docs.defang.io/docs/faq

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So does that mean this PR is not ready to be merged? I wouldn't want the happy path to show "Deployment will not be cancelled".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right this PR is not ready to merge. It's still in draft.

term.Warn("Deployment will not be cancelled.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This message could be less curt.

}
}()
resp, err = p.Deploy(ctx, deployRequest)
if err != nil {
return nil, project, err
Expand Down
Loading