-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add support for --detach flag in stack deploy #4258
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
Merged
thaJeztah
merged 1 commit into
docker:master
from
gmargaritis:373-support-detach-in-docker-stack-deploy
Mar 4, 2024
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,9 +2,11 @@ package swarm | |
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "fmt" | ||
|
|
||
| "github.com/docker/cli/cli/command" | ||
| servicecli "github.com/docker/cli/cli/command/service" | ||
| "github.com/docker/cli/cli/command/stack/options" | ||
| "github.com/docker/cli/cli/compose/convert" | ||
| composetypes "github.com/docker/cli/cli/compose/types" | ||
|
|
@@ -13,10 +15,9 @@ import ( | |
| "github.com/docker/docker/api/types/swarm" | ||
| apiclient "github.com/docker/docker/client" | ||
| "github.com/docker/docker/errdefs" | ||
| "github.com/pkg/errors" | ||
| ) | ||
|
|
||
| func deployCompose(ctx context.Context, dockerCli command.Cli, opts options.Deploy, config *composetypes.Config) error { | ||
| func deployCompose(ctx context.Context, dockerCli command.Cli, opts *options.Deploy, config *composetypes.Config) error { | ||
| if err := checkDaemonIsSwarmManager(ctx, dockerCli); err != nil { | ||
| return err | ||
| } | ||
|
|
@@ -60,7 +61,17 @@ func deployCompose(ctx context.Context, dockerCli command.Cli, opts options.Depl | |
| if err != nil { | ||
| return err | ||
| } | ||
| return deployServices(ctx, dockerCli, services, namespace, opts.SendRegistryAuth, opts.ResolveImage) | ||
|
|
||
| serviceIDs, err := deployServices(ctx, dockerCli, services, namespace, opts.SendRegistryAuth, opts.ResolveImage) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if opts.Detach { | ||
| return nil | ||
| } | ||
|
|
||
| return waitOnServices(ctx, dockerCli, serviceIDs, opts.Quiet) | ||
| } | ||
|
|
||
| func getServicesDeclaredNetworks(serviceConfigs []composetypes.ServiceConfig) map[string]struct{} { | ||
|
|
@@ -87,11 +98,11 @@ func validateExternalNetworks(ctx context.Context, client apiclient.NetworkAPICl | |
| network, err := client.NetworkInspect(ctx, networkName, types.NetworkInspectOptions{}) | ||
| switch { | ||
| case errdefs.IsNotFound(err): | ||
| return errors.Errorf("network %q is declared as external, but could not be found. You need to create a swarm-scoped network before the stack is deployed", networkName) | ||
| return fmt.Errorf("network %q is declared as external, but could not be found. You need to create a swarm-scoped network before the stack is deployed", networkName) | ||
| case err != nil: | ||
| return err | ||
| case network.Scope != "swarm": | ||
| return errors.Errorf("network %q is declared as external, but it is not in the right scope: %q instead of \"swarm\"", networkName, network.Scope) | ||
| return fmt.Errorf("network %q is declared as external, but it is not in the right scope: %q instead of \"swarm\"", networkName, network.Scope) | ||
| } | ||
| } | ||
| return nil | ||
|
|
@@ -106,13 +117,13 @@ func createSecrets(ctx context.Context, dockerCli command.Cli, secrets []swarm.S | |
| case err == nil: | ||
| // secret already exists, then we update that | ||
| if err := client.SecretUpdate(ctx, secret.ID, secret.Meta.Version, secretSpec); err != nil { | ||
| return errors.Wrapf(err, "failed to update secret %s", secretSpec.Name) | ||
| return fmt.Errorf("failed to update secret %s: %w", secretSpec.Name, err) | ||
| } | ||
| case errdefs.IsNotFound(err): | ||
| // secret does not exist, then we create a new one. | ||
| fmt.Fprintf(dockerCli.Out(), "Creating secret %s\n", secretSpec.Name) | ||
| if _, err := client.SecretCreate(ctx, secretSpec); err != nil { | ||
| return errors.Wrapf(err, "failed to create secret %s", secretSpec.Name) | ||
| return fmt.Errorf("failed to create secret %s: %w", secretSpec.Name, err) | ||
| } | ||
| default: | ||
| return err | ||
|
|
@@ -130,13 +141,13 @@ func createConfigs(ctx context.Context, dockerCli command.Cli, configs []swarm.C | |
| case err == nil: | ||
| // config already exists, then we update that | ||
| if err := client.ConfigUpdate(ctx, config.ID, config.Meta.Version, configSpec); err != nil { | ||
| return errors.Wrapf(err, "failed to update config %s", configSpec.Name) | ||
| return fmt.Errorf("failed to update config %s: %w", configSpec.Name, err) | ||
| } | ||
| case errdefs.IsNotFound(err): | ||
| // config does not exist, then we create a new one. | ||
| fmt.Fprintf(dockerCli.Out(), "Creating config %s\n", configSpec.Name) | ||
| if _, err := client.ConfigCreate(ctx, configSpec); err != nil { | ||
| return errors.Wrapf(err, "failed to create config %s", configSpec.Name) | ||
| return fmt.Errorf("failed to create config %s: %w", configSpec.Name, err) | ||
| } | ||
| default: | ||
| return err | ||
|
|
@@ -169,26 +180,28 @@ func createNetworks(ctx context.Context, dockerCli command.Cli, namespace conver | |
|
|
||
| fmt.Fprintf(dockerCli.Out(), "Creating network %s\n", name) | ||
| if _, err := client.NetworkCreate(ctx, name, createOpts); err != nil { | ||
| return errors.Wrapf(err, "failed to create network %s", name) | ||
| return fmt.Errorf("failed to create network %s: %w", name, err) | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func deployServices(ctx context.Context, dockerCli command.Cli, services map[string]swarm.ServiceSpec, namespace convert.Namespace, sendAuth bool, resolveImage string) error { | ||
| func deployServices(ctx context.Context, dockerCli command.Cli, services map[string]swarm.ServiceSpec, namespace convert.Namespace, sendAuth bool, resolveImage string) ([]string, error) { | ||
| apiClient := dockerCli.Client() | ||
| out := dockerCli.Out() | ||
|
|
||
| existingServices, err := getStackServices(ctx, apiClient, namespace.Name()) | ||
| if err != nil { | ||
| return err | ||
| return nil, err | ||
| } | ||
|
|
||
| existingServiceMap := make(map[string]swarm.Service) | ||
| for _, service := range existingServices { | ||
| existingServiceMap[service.Spec.Name] = service | ||
| } | ||
|
|
||
| var serviceIDs []string | ||
|
|
||
| for internalName, serviceSpec := range services { | ||
| var ( | ||
| name = namespace.Scope(internalName) | ||
|
|
@@ -200,7 +213,7 @@ func deployServices(ctx context.Context, dockerCli command.Cli, services map[str | |
| // Retrieve encoded auth token from the image reference | ||
| encodedAuth, err = command.RetrieveAuthTokenFromImage(dockerCli.ConfigFile(), image) | ||
| if err != nil { | ||
| return err | ||
| return nil, err | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -241,12 +254,14 @@ func deployServices(ctx context.Context, dockerCli command.Cli, services map[str | |
|
|
||
| response, err := apiClient.ServiceUpdate(ctx, service.ID, service.Version, serviceSpec, updateOpts) | ||
| if err != nil { | ||
| return errors.Wrapf(err, "failed to update service %s", name) | ||
| return nil, fmt.Errorf("failed to update service %s: %w", name, err) | ||
| } | ||
|
|
||
| for _, warning := range response.Warnings { | ||
| fmt.Fprintln(dockerCli.Err(), warning) | ||
| } | ||
|
|
||
| serviceIDs = append(serviceIDs, service.ID) | ||
| } else { | ||
| fmt.Fprintf(out, "Creating service %s\n", name) | ||
|
|
||
|
|
@@ -257,10 +272,29 @@ func deployServices(ctx context.Context, dockerCli command.Cli, services map[str | |
| createOpts.QueryRegistry = true | ||
| } | ||
|
|
||
| if _, err := apiClient.ServiceCreate(ctx, serviceSpec, createOpts); err != nil { | ||
| return errors.Wrapf(err, "failed to create service %s", name) | ||
| response, err := apiClient.ServiceCreate(ctx, serviceSpec, createOpts) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to create service %s: %w", name, err) | ||
| } | ||
|
|
||
| serviceIDs = append(serviceIDs, response.ID) | ||
| } | ||
| } | ||
|
|
||
| return serviceIDs, nil | ||
| } | ||
|
|
||
| func waitOnServices(ctx context.Context, dockerCli command.Cli, serviceIDs []string, quiet bool) error { | ||
| var errs []error | ||
| for _, serviceID := range serviceIDs { | ||
| if err := servicecli.WaitOnService(ctx, dockerCli, serviceID, quiet); err != nil { | ||
| errs = append(errs, fmt.Errorf("%s: %w", serviceID, err)) | ||
| } | ||
| } | ||
|
|
||
| if len(errs) > 0 { | ||
| return errors.Join(errs...) | ||
| } | ||
|
Comment on lines
+295
to
+297
Collaborator
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 think you can just return |
||
|
|
||
| return nil | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
It looks like this PR (currently at least) keeps the old behavior (
docker stack deploybeing non-interactive / detached as default).If the plan is to eventually make "interactive" (
--detach=false) the default, I also went looking how we handled the transition fordocker service create; moby/moby@21ec12b (moby/moby#31144)For that we chose to print a warning; wondering if (and if: when? / which release?) we want to do the same for
docker stack; inform users that we plan to switch non-detach as default, with the option to return to the old (--detach[=true]) behavior.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.
I've added a message just like in moby/moby@21ec12b (moby/moby#31144) as you said, indicating that we plan to switch non-detach as default.