Add version annotation to various flags added in 1.13 #30186
Merged
Conversation
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
Pull request moby#27745 added support for the client to talk to older versions of the daemon. Various flags were added to docker 1.13 that are not compatible with older daemons. This PR adds annotations to those flags, so that they are automatically hidden if the daemon is older than docker 1.13 (API 1.25). Not all new flags affect the API (some are client-side only). The following PR's added new flags to docker 1.13 that affect the API; - moby#23430 added `--cpu-rt-period`and `--cpu-rt-runtime` - moby#27800 / moby#25317 added `--group` / `--group-add` / `--group-rm` - moby#27702 added `--network` to `docker build` - moby#25962 added `--attachable` to `docker network create` - moby#27998 added `--compose-file` to `docker stack deploy` - moby#22566 added `--stop-timeout` to `docker run` and `docker create` - moby#26061 added `--init` to `docker run` and `docker create` - moby#26941 added `--init-path` to `docker run` and `docker create` - moby#27958 added `--cpus` on `docker run` / `docker create` - moby#27567 added `--dns`, `--dns-opt`, and `--dns-search` to `docker service create` - moby#27596 added `--force` to `docker service update` - moby#27857 added `--hostname` to `docker service create` - moby#28031 added `--hosts`, `--host-add` / `--host-rm` to `docker service create` and `docker service update` - moby#28076 added `--tty` on `docker service create` / `docker service update` - moby#26421 added `--update-max-failure-ratio`, `--update-monitor` and `--rollback` on `docker service update` - moby#27369 added `--health-cmd`, `--health-interval`, `--health-retries`, `--health-timeout` and `--no-healthcheck` options to `docker service create` and `docker service update` Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
commented
Jan 16, 2017
| @@ -236,9 +236,12 @@ func addFlags(flags *pflag.FlagSet) *containerOptions { | |||
| flags.Int64Var(&copts.cpuPeriod, "cpu-period", 0, "Limit CPU CFS (Completely Fair Scheduler) period") | |||
| flags.Int64Var(&copts.cpuQuota, "cpu-quota", 0, "Limit CPU CFS (Completely Fair Scheduler) quota") | |||
| flags.Int64Var(&copts.cpuRealtimePeriod, "cpu-rt-period", 0, "Limit CPU real-time period in microseconds") | |||
| flags.SetAnnotation("cpu-rt-period", "version", []string{"1.25"}) | |||
There was a problem hiding this comment.
Wondering if we should add a utility for this, or to specify the version when adding flags (this does the job, but it's rather ugly to have these SetAnnotation() everywhere.
There was a problem hiding this comment.
We could have a util and group the flags by version:
SetFlagVersion(flags, "1.25", "cpu-rt-period", "cpu-rt-runtime", "cpus", "init", "init-path")There was a problem hiding this comment.
Yes I considered putting all the annotations at the end (looking a bit cleaner), but decided keeping them together with the flag also has it's advantages.
Ideally, something like this;
flags.Var(&copts.foo, "foo", "Set foo option", flagAnnotation{"version", "1.25"}, flagAnnotation{"platform", "windows"})or
flags.Var(&copts.foo, "foo", "Set foo option").Annotate("version", "1.25").Annotate("platform", "windows")There was a problem hiding this comment.
We'll have to fork pflag to make that kind of change, but that's fine with me. Having the flag Vars return the object would be a good idea.
However many of these lines are already too long, so I don't know that adding more to the line is an improvement.
dnephin
approved these changes
Jan 19, 2017
| @@ -236,9 +236,12 @@ func addFlags(flags *pflag.FlagSet) *containerOptions { | |||
| flags.Int64Var(&copts.cpuPeriod, "cpu-period", 0, "Limit CPU CFS (Completely Fair Scheduler) period") | |||
| flags.Int64Var(&copts.cpuQuota, "cpu-quota", 0, "Limit CPU CFS (Completely Fair Scheduler) quota") | |||
| flags.Int64Var(&copts.cpuRealtimePeriod, "cpu-rt-period", 0, "Limit CPU real-time period in microseconds") | |||
| flags.SetAnnotation("cpu-rt-period", "version", []string{"1.25"}) | |||
There was a problem hiding this comment.
We could have a util and group the flags by version:
SetFlagVersion(flags, "1.25", "cpu-rt-period", "cpu-rt-runtime", "cpus", "init", "init-path")|
LGTM |
dnephin
pushed a commit
to dnephin/docker
that referenced
this pull request
Apr 17, 2017
…o-flags Add version annotation to various flags added in 1.13
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Pull request #27745 added support for the
client to talk to older versions of the daemon. Various flags were added to
docker 1.13 that are not compatible with older daemons.
This PR adds annotations to those flags, so that they are automatically hidden
if the daemon is older than docker 1.13 (API 1.25).
Not all new flags affect the API (some are client-side only). The following
PR's added new flags to docker 1.13 that affect the API;
--cpu-rt-periodand--cpu-rt-runtime--groupindocker service create#25317 added--group/--group-add/--group-rm--networktodocker build--attachabletodocker network createdocker stack deployfrom a Compose file #27998 added--compose-filetodocker stack deploy--stop-timeouttodocker runanddocker create--inittodocker runanddocker create--init-pathtodocker runanddocker create--cpusflag to control cpu resources #27958 added--cpusondocker run/docker create--dns,--dns-opt, and--dns-searchtodocker service create--forcetodocker service update--hostnametodocker service create--hosttoservice createand--host-add/rmtoservice update#28031 added--hosts,--host-add/--host-rmtodocker service createanddocker service update--ttytodocker service create/update#28076 added--ttyondocker service create/docker service update--update-max-failure-ratio,--update-monitorand--rollbackondocker service update--health-cmd,--health-interval,--health-retries,--health-timeoutand--no-healthcheckoptions todocker service createanddocker service update- How to verify it
Verify that the correct flags are hidden when overriding the API version. For example;
Without
DOCKER_API_VERSION;And with
DOCKER_API_VERSION;DOCKER_API_VERSION=1.24 docker service create --help Usage: docker service create [OPTIONS] IMAGE [COMMAND] [ARG...] Create a new service Options: --constraint list Placement constraints (default []) --container-label list Container labels (default []) --endpoint-mode string Endpoint mode (vip or dnsrr) -e, --env list Set environment variables (default []) --env-file list Read in a file of environment variables (default []) --help Print usage -l, --label list Service labels (default []) --limit-cpu decimal Limit CPUs (default 0.000) --limit-memory bytes Limit Memory (default 0 B) --log-driver string Logging driver for service --log-opt list Logging driver options (default []) --mode string Service mode (replicated or global) (default "replicated") --mount mount Attach a filesystem mount to the service --name string Service name --network list Network attachments (default []) -p, --publish port Publish a port as a node port --replicas uint Number of tasks --reserve-cpu decimal Reserve CPUs (default 0.000) --reserve-memory bytes Reserve Memory (default 0 B) --restart-condition string Restart when condition is met (none, on-failure, or any) --restart-delay duration Delay between restart attempts (ns|us|ms|s|m|h) --restart-max-attempts uint Maximum number of restarts before giving up --restart-window duration Window used to evaluate the restart policy (ns|us|ms|s|m|h) --stop-grace-period duration Time to wait before force killing a container (ns|us|ms|s|m|h) --update-delay duration Delay between updates (ns|us|ms|s|m|h) (default 0s) --update-failure-action string Action on update failure (pause|continue) (default "pause") --update-parallelism uint Maximum number of tasks updated simultaneously (0 to update all at once) (default 1) -u, --user string Username or UID (format: <name|uid>[:<group|gid>]) --with-registry-auth Send registry authentication details to swarm agents -w, --workdir string Working directory inside the container- Description for the changelog
--helpoutput