Skip to content
This repository was archived by the owner on Jan 30, 2020. It is now read-only.

fleetctl: set --driver=etcd if --endpoint is etcd-like#1205

Merged
bcwaldon merged 1 commit into
coreos:masterfrom
bcwaldon:default-driver
Apr 29, 2015
Merged

fleetctl: set --driver=etcd if --endpoint is etcd-like#1205
bcwaldon merged 1 commit into
coreos:masterfrom
bcwaldon:default-driver

Conversation

@bcwaldon
Copy link
Copy Markdown
Contributor

There is one remaining wart of the switch from --driver=etcd to --driver=API. If someone had been using fleetctl with a custom endpoint like so:

fleetctl --endpoint http://example.com:4001 list-machines

They now all of a sudden get directed to the fleet API rather than etcd. Given that we can tell (based on the port) that the user wants to use etcd, the default of --driver in this case should remain "etcd".

Comment thread fleetctl/fleetctl.go Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

7 levels of indentation?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

deal with it

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

UMM

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How does this look if it's in an anonymous function, returning after failed conditions?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Or if you wrap the check in a recover() and just assume that the Split, Parse, and SplitHostPort worked.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

visited := make(map[string]bool, 0)
...
if !visited["driver"] && visited["endpoint"] {
    if u, err := url.Parse(strings.Split(globalFlags.Endpoint, ",")[0]); err == nil  {
        if _, port, err := net.SplitHostPort(u.Host); err == nil && (port == "4001" || port == "2379") {
            log.Debugf("Defaulting to --driver=%s as --endpoint appears to be etcd", clientDriverEtcd)
            globalFlags.ClientDriver = clientDriverEtcd
        }
    }
}

@crawford
Copy link
Copy Markdown
Contributor

LGTM

bcwaldon added a commit that referenced this pull request Apr 29, 2015
fleetctl: set --driver=etcd if --endpoint is etcd-like
@bcwaldon bcwaldon merged commit 5f9b86d into coreos:master Apr 29, 2015
@bcwaldon bcwaldon added this to the v0.10.1 milestone Apr 29, 2015
@bcwaldon bcwaldon deleted the default-driver branch June 9, 2015 18:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants