-
Notifications
You must be signed in to change notification settings - Fork 11
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.
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
Add more logs to tail and service status monitor #518
Changes from all commits
d1f940b
6b840c7
686c82d
3517f1c
304462d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,7 +11,6 @@ import ( | |
"regexp" | ||
"slices" | ||
"strings" | ||
"sync" | ||
"time" | ||
|
||
"github.com/AlecAivazis/survey/v2" | ||
|
@@ -825,30 +824,25 @@ var composeUpCmd = &cobra.Command{ | |
return nil | ||
} | ||
|
||
tailCtx, cancelTail := context.WithCancel(ctx) | ||
tailCtx, cancelTail := contextWithServiceStatus(ctx, cli.ServiceStarted, serviceInfos) | ||
defer cancelTail() | ||
|
||
var wg sync.WaitGroup | ||
wg.Add(1) | ||
go func() { | ||
defer wg.Done() | ||
// show users the current streaming logs | ||
if err := startTailing(tailCtx, deploy.Etag, since); err != nil { | ||
var cerr *cli.CancelError | ||
if !errors.As(err, &cerr) { | ||
term.Debugf("failed to start tailing: %v", err) | ||
} | ||
} | ||
}() | ||
if err := waitServiceStatus(ctx, cli.ServiceStarted, serviceInfos); err != nil && !errors.Is(err, context.Canceled) { | ||
if !errors.Is(err, cli.ErrDryRun) && !errors.As(err, new(cliClient.ErrNotImplemented)) { | ||
term.Warnf("failed to wait for service status: %v", err) | ||
} | ||
wg.Wait() // Wait until ctrl+c is pressed | ||
// show users the current streaming logs | ||
err = startTailing(tailCtx, deploy.Etag, since) | ||
|
||
// Indicate to the user the log tailing failed | ||
var cerr *cli.CancelError | ||
if !errors.As(err, &cerr) { | ||
term.Warnf("failed to start tailing, you will not be able to see logs: %v", err) | ||
Comment on lines
+835
to
+836
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am on the fence if this warning is needed |
||
} | ||
cancelTail() | ||
wg.Wait() // Wait for tail to finish | ||
|
||
// wait for service status to be ready even if tailing failed | ||
<-tailCtx.Done() | ||
printEndpoints(serviceInfos) | ||
|
||
if err != nil { | ||
return err | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This fixes the detach message regression |
||
} | ||
term.Info("Done.") | ||
return nil | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,39 +2,67 @@ package command | |
|
||
import ( | ||
"context" | ||
"errors" | ||
"fmt" | ||
|
||
"github.com/DefangLabs/defang/src/pkg/cli" | ||
cliClient "github.com/DefangLabs/defang/src/pkg/cli/client" | ||
"github.com/DefangLabs/defang/src/pkg/term" | ||
defangv1 "github.com/DefangLabs/defang/src/protos/io/defang/v1" | ||
) | ||
|
||
func contextWithServiceStatus(ctx context.Context, targetStatus cli.ServiceStatus, serviceInfos []*defangv1.ServiceInfo) (context.Context, context.CancelFunc) { | ||
// New context to make sure tail is only cancelled when message about service status is printed | ||
newCtx, cancel := context.WithCancel(context.Background()) | ||
go func() { | ||
err := waitServiceStatus(ctx, targetStatus, serviceInfos) | ||
if err == nil { | ||
cancel() | ||
return | ||
} | ||
|
||
if !errors.Is(err, context.Canceled) && !errors.Is(err, cli.ErrDryRun) && !errors.As(err, new(cliClient.ErrNotImplemented)) { | ||
term.Info("Service status monitoring failed, we will continue tailing the logs. Press Ctrl+C to detach.") | ||
} | ||
term.Debugf("failed to wait for service status: %v", err) | ||
Comment on lines
+24
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code would be much simpler if we don't need to log these, but we risk swallow errors for debugging |
||
|
||
<-ctx.Done() // Don't cancel tail until the original context is cancelled | ||
cancel() | ||
}() | ||
return newCtx, cancel | ||
|
||
} | ||
func waitServiceStatus(ctx context.Context, targetStatus cli.ServiceStatus, serviceInfos []*defangv1.ServiceInfo) error { | ||
serviceList := []string{} | ||
for _, serviceInfo := range serviceInfos { | ||
serviceList = append(serviceList, serviceInfo.Service.Name) | ||
} | ||
term.Debugf("Waiting for services %v to reach state: %s", serviceList, targetStatus) | ||
|
||
// set up service status subscription (non-blocking) | ||
subscribeServiceStatusChan, err := cli.Subscribe(ctx, client, serviceList) | ||
serviceStatusUpdateStream, err := cli.Subscribe(ctx, client, serviceList) | ||
if err != nil { | ||
term.Debugf("error subscribing to service status: %v", err) | ||
return err | ||
return fmt.Errorf("error subscribing to service status: %w", err) | ||
} | ||
defer serviceStatusUpdateStream.Close() | ||
|
||
serviceStatus := make(map[string]string, len(serviceList)) | ||
for _, name := range serviceList { | ||
serviceStatus[name] = string(cli.ServiceUnknown) | ||
} | ||
|
||
// monitor for when all services are completed to end this command | ||
for newStatus := range subscribeServiceStatusChan { | ||
if _, ok := serviceStatus[newStatus.Name]; !ok { | ||
term.Debugf("unexpected service %s update", newStatus.Name) | ||
continue | ||
for { | ||
statusUpdate, err := serviceStatusUpdateStream.ServerStatus() | ||
if err != nil { | ||
return fmt.Errorf("service state monitoring terminated without all services reaching desired state %q: %w", targetStatus, err) | ||
} | ||
|
||
serviceStatus[newStatus.Name] = newStatus.Status | ||
if _, ok := serviceStatus[statusUpdate.Name]; !ok { | ||
term.Debugf("unexpected service %s update", statusUpdate.Name) | ||
continue | ||
} | ||
serviceStatus[statusUpdate.Name] = statusUpdate.Status | ||
|
||
if allInStatus(targetStatus, serviceStatus) { | ||
for _, sInfo := range serviceInfos { | ||
|
@@ -43,8 +71,6 @@ func waitServiceStatus(ctx context.Context, targetStatus cli.ServiceStatus, serv | |
return nil | ||
} | ||
} | ||
|
||
return fmt.Errorf("service state monitoring terminated without all services reaching desired state: %s", targetStatus) | ||
} | ||
|
||
func allInStatus(targetStatus cli.ServiceStatus, serviceStatuses map[string]string) bool { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how can this happen? startTailing blocks? You do
<-Done()
so this cancel is just for cleanup, in which case the name is wrong.