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

Docs: add note about management commands targeting a manager #1590

Merged
merged 1 commit into from
Dec 12, 2019

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Dec 23, 2018

Alternative for #1578

closes #1578 already merged

has to be run targeting a manager node.
Create and update a stack from a `compose` or a `dab` file on the swarm.

> **Note**: This is a cluster management command, and must be executed on a swarm
Copy link
Member Author

Choose a reason for hiding this comment

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

Wasn't sure how to deal with k8s as an orchestrator here; this command can be run both on Swarm or Kubernetes, so we may need a different description to either make it generic enough to fit both use-cases, or extend the description to explicitly call out both use-cases

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a something like: **Note**:If you use swarm as an orchestrator, this is a cluster management command... ?

@thaJeztah
Copy link
Member Author

This is a quick approach for this, but I think we can add annotations for this, so that;

  • it can be used to automatically show/hide commands if the node is not a manager
  • it can be included in the generated YAML docs, so that we can add this information automatically when generating the documentation; similar to the API-version and Orchestrator annotations;

screen shot 2018-12-23 at 12 33 00

@codecov-io
Copy link

codecov-io commented Dec 23, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@0fd5c16). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #1590   +/-   ##
=========================================
  Coverage          ?   55.25%           
=========================================
  Files             ?      289           
  Lines             ?    19395           
  Branches          ?        0           
=========================================
  Hits              ?    10716           
  Misses            ?     7983           
  Partials          ?      696

@thaJeztah
Copy link
Member Author

@ahh-docker @vdemeester

Copy link
Collaborator

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

SGTM 👼

@thaJeztah thaJeztah force-pushed the docs_add_management_notes branch from 02fdf9b to 2cac3ea Compare December 24, 2018 09:49
@ahh-docker
Copy link
Contributor

LGTM

@SvenDowideit
Copy link
Contributor

mmm, LGTM - though i'm saddened that I didn't build something to let you transclude the note from one place :D

@thaJeztah
Copy link
Member Author

though i'm saddened that I didn't build something to let you transclude the note from one place :D

Yeah, the docs switched to Jekyll at some point, so perhaps it could be handled there, but I didn't want to jinx it 😂

Let me do a small update to address the "kubernetes as stack orchestrator" situation;
#1590 (comment)

@thaJeztah thaJeztah force-pushed the docs_add_management_notes branch from 2cac3ea to 8e1cdb5 Compare December 10, 2019 13:31
@thaJeztah
Copy link
Member Author

@silvin-lubecki slightly changed the wording for docker stack commands, to explicitly mention that a manager node must be targeted when using "swarm" as orchestrator.

Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the docs_add_management_notes branch from 8e1cdb5 to f540eae Compare December 12, 2019 13:57
@silvin-lubecki silvin-lubecki merged commit eb33f87 into docker:master Dec 12, 2019
@thaJeztah thaJeztah deleted the docs_add_management_notes branch December 12, 2019 14:38
@thaJeztah thaJeztah added this to the 20.03.0 milestone Feb 19, 2020
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.

7 participants