Skip to content

Deprecate '--cluster-xx' options and add warning#40510

Merged
cpuguy83 merged 2 commits intomoby:masterfrom
aiordache:moby_cluster_flags_deprecate
Feb 27, 2020
Merged

Deprecate '--cluster-xx' options and add warning#40510
cpuguy83 merged 2 commits intomoby:masterfrom
aiordache:moby_cluster_flags_deprecate

Conversation

@aiordache
Copy link
Copy Markdown

@aiordache aiordache commented Feb 12, 2020

- What I did
Deprecated --cluster-advertise, --cluster-store , --cluster-store-opt flags from dockerd CLI.

  • logged warning when used

- How to verify it

$ dockerd --cluster-advertise 192.168.1.5:2376 --cluster-store etcd://192.168.1.6:2379
Flag --cluster-advertise has been deprecated, Deprecated option.
Flag --cluster-store has been deprecated, Deprecated option.
WARN[2020-02-12T17:43:14.108302242Z] The "cluster-advertise" option is deprecated. To be removed soon. 
WARN[2020-02-12T17:43:14.108351655Z] The "cluster-store" option is deprecated. To be removed soon. 
...
$ cat /etc/docker/daemon.json 
{
	"cluster-store": "etcd://192.168.1.5:1257",
	"cluster-store-opts": {},
	"cluster-advertise": "192.168.1.6:3123"
}
$ dockerd
WARN[2020-02-12T17:47:20.767700485Z] The "cluster-advertise" option is deprecated. To be removed soon. 
WARN[2020-02-12T17:47:20.767745964Z] The "cluster-store" option is deprecated. To be removed soon. 
INFO[2020-02-12T17:47:20.767757515Z] Starting up                                  
INFO[2020-02-12T17:47:20.768774398Z] libcontainerd: started new containerd process  pid=582
INFO[2020-02-12T17:47:20.768816420Z] parsed scheme: "unix"                         module=grpc
...

- A picture of a cute animal (not mandatory but encouraged)

Co-authored-by: Yves Brissaud yves.brissaud@gmail.com

Signed-off-by: Anca Iordache anca.iordache@docker.com

relates to #40383

Co-authored-by: Yves Brissaud <yves.brissaud@gmail.com>

Signed-off-by: Anca Iordache <anca.iordache@docker.com>
@thaJeztah
Copy link
Copy Markdown
Member

Windows failure is unrelated (see the discussion on #40512 (comment))

@thaJeztah thaJeztah added this to the 20.03.0 milestone Feb 13, 2020
flags.Var(opts.NewNamedMapOpts("log-opts", conf.LogConfig.Config, nil), "log-opt", "Default log driver options for containers")

flags.StringVar(&conf.ClusterAdvertise, "cluster-advertise", "", "Address or interface name to advertise")
_ = flags.MarkDeprecated("cluster-advertise", "Deprecated option.")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we add a more descriptive text as in flags.MarkDeprecated("restart", "Please use a restart policy on docker run")?

Maybe like flags.MarkDeprecated("cluster-advertise", "Swarm classic is deprecated. Please use Swarm-mode (docker swarm init)")?

return conf, nil
}

func warnOnDeprecatedConfigOptions(config *config.Config) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This function should also handle --restart?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It is mostly intended for the options set in the daemon configuration file. Can't find if --restart has an equivalent in the daemon config file.

@AkihiroSuda
Copy link
Copy Markdown
Member

LGTM but a couple of nits

Signed-off-by: Anca Iordache <anca.iordache@docker.com>
Copy link
Copy Markdown
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

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.

4 participants