contiv / netplugin

Container networking for various use cases
Apache License 2.0
513 stars 177 forks source link

netmaster cli refactor #1088

Closed tiewei closed 6 years ago

tiewei commented 6 years ago

Refactor netmaster cli to use "github.com/urfave/cli", it brings benefit like setting forwarding mode, infra type and network mode, allows setting multi etcd/consul endpoints and using SSL, enabling syslog,json log and multi log levels.

Signed-off-by: Wei Tie wtie@cisco.com

tiewei commented 6 years ago

TODO: update container entrypoint scripts

dseevr commented 6 years ago

build pr

tiewei commented 6 years ago

build PR

dseevr commented 6 years ago

@tiewei How do you feel about raising an error if etcd and consul endpoints are set? Right now it says "consul endpoints are ignored if etcd endpoints are set"... if both of them are set, we can't really determine what the user intent was IMO. I think we should just explode in that case.

tiewei commented 6 years ago

@chrisplo mentioned that as well in last commit ... i'll error it out then

tiewei commented 6 years ago

build PR

unclejack commented 6 years ago

What release is this meant to be included in? It'd be nice to deprecate the old flags instead of removing them. This should make it easier if we make a 1.2 release which is still backwards compatible and some users upgrade an existing setup. What do you think?

tiewei commented 6 years ago

Most old flag was kept except --cluster-store due to the schema of url change, yes 1.2 would be ideal for this type of changes

tiewei commented 6 years ago

Build PR

chrisplo commented 6 years ago

overall substance looks good, just some style nits