Skip to content
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 experimental Docker Stack CLI commands #23522

Merged
merged 2 commits into from
Jun 15, 2016
Merged

Add experimental Docker Stack CLI commands #23522

merged 2 commits into from
Jun 15, 2016

Conversation

dnephin
Copy link
Member

@dnephin dnephin commented Jun 14, 2016

Adds experimental stack commands for deploying a bundle

@duglin
Copy link
Contributor

duglin commented Jun 14, 2016

@dnephin where can we read up on this feature?

@icecrime
Copy link
Contributor

@duglin I'm going to try and write a doc page today, sorry for the delay!

@mlaventure
Copy link
Contributor

Can we have // +build experimental added to all the new stack files to avoid compiling them when building the non experimental version?

Signed-off-by: Daniel Nephin <dnephin@docker.com>
@dnephin
Copy link
Member Author

dnephin commented Jun 14, 2016

@mlaventure done

networkSet := make(map[string]bool)
for _, service := range services {
for _, network := range service.Networks {
networkSet[network] = true
Copy link
Contributor

Choose a reason for hiding this comment

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

can't we just append those in the networks []string variable from the get go here?

I'm not sure what is the benefit of going through an intermediate map first.

Copy link
Contributor

Choose a reason for hiding this comment

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

Scratch that, I read the function name afterwards :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ya, I don't know if there is a better way to do this in go.

@mlaventure
Copy link
Contributor

LGTM (IANAM)

Only nitpick I've got is:

func getNetworks(
    ctx context.Context,
    apiclient client.APIClient,
    namespace string,
 ) ([]types.NetworkResource, error) {
    return apiclient.NetworkList(
        ctx,
        types.NetworkListOptions{Filters: getStackFilter(namespace)})
 }

we have no other code with the above style as we don't enforce the 80 columns rule

Signed-off-by: Arnaud Porterie (icecrime) <arnaud.porterie@docker.com>
@icecrime
Copy link
Contributor

Added some docs in #23544.

@GordonTheTurtle GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label Jun 15, 2016
@GordonTheTurtle GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Jun 15, 2016
@icecrime
Copy link
Contributor

LGTM 👍

@vieux
Copy link
Contributor

vieux commented Jun 15, 2016

Mush better with some docs thanks @icecrime

LGTM

@cpuguy83
Copy link
Member

LGTM moving to docs

@icecrime
Copy link
Contributor

So @thaJeztah is on a plane, and this is experimental, soooooooo... 😇

@icecrime icecrime merged commit c067756 into master Jun 15, 2016
dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017
Add experimental Docker Stack CLI commands
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants