Implement version gate in fleetctl #428
Conversation
|
Added dumb strawman to start a conversation on this. |
| @@ -99,6 +98,7 @@ func newMachineFromConfig(cfg config.Config) (*machine.Machine, error) { | |||
| func newAgentFromConfig(mach *machine.Machine, cfg config.Config, mgr *systemd.SystemdManager) (*agent.Agent, error) { | |||
| regClient := newEtcdClientFromConfig(cfg) | |||
| reg := registry.New(regClient, cfg.EtcdKeyPrefix) | |||
| reg.SetLatestVersion(version.SemVersion) | |||
There was a problem hiding this comment.
I think it's overkill to do this right now. The version information is already published in each individual MachineState object. Could we not infer the latest version from that?
There was a problem hiding this comment.
Neat, somehow I missed that.
|
Updated to address feedback. Any other high-level comments? thoughts? concerns? |
| func (r *EtcdRegistry) GetLatestVersion() (*semver.Version, error) { | ||
| machs, err := r.GetActiveMachines() | ||
| if err != nil { | ||
| if err.(*etcd.EtcdError).ErrorCode == etcdErr.EcodeKeyNotFound { |
There was a problem hiding this comment.
We'll need to update this to use the new isKeyNotFound helper here or in the other PR depending on which lands first.
|
Looking good. Needs a rebase. |
|
Rebased. I'm still not necessarily totally sold on this. Is it too scary? Should we link to a github issue (e.g. having a proper API), or talk about future compatibility? |
|
@jonboulle We absolutely need something like this. What specifically are you concerned about? |
|
@bcwaldon I suppose the implication that we're incapable of providing any compatibility (for example, this doesn't discriminate between patch/minor versions). But since that seems to be the reality for now perhaps we should roll forward with this + revisit asap? |
| WARNING: fleetctl (%s) is older than the latest registered | ||
| version of fleet in the Registry (%s). You are strongly | ||
| recommended to upgrade fleetctl to prevent incompatibility issues. | ||
| #################################################################### |
There was a problem hiding this comment.
I wouldn't mention anything about how fleet determines what the current version is. Maybe link to the releases page?
####################################################################
WARNING: fleetctl (%s) does not match the version running on the
cluster (%s). You are strongly recommended to upgrade fleetctl to
prevent incompatibility issues.
Download from https://github.com/coreos/fleet/releases
####################################################################
There was a problem hiding this comment.
@bcwaldon thoughts?
There was a problem hiding this comment.
@robszumski What's your thinking behind that suggestion?
There was a problem hiding this comment.
it references the registry which most users have no idea a) that it exists or b) what it does.
There was a problem hiding this comment.
@robszumski What if we just replace "Registry" with "cluster"?
There was a problem hiding this comment.
I don't love latest registered version because it exposes a bit of how the system works to the user, when they don't care how fleet determines the holistic version of the cluster. But, it's not the end of the world.
There was a problem hiding this comment.
@jonboulle s/Registry/cluster/
|
@robszumski Until we have a single versioned entrypoint, they have to care how it works to a degree. |
|
@bcwaldon do you want this merged, then? |
This adds a basic version check/gate to fleetctl wherein it will print a scary warning to the user if the version of fleetctl appears to be older than the version of fleet running in the cluster. The version of fleet in the cluster is inferred from the latest version found in all machinestates registered in the registry.
|
updated |
|
lgtm when tests pass |
|
ok here we gooooo |
Implement version gate in fleetctl
No description provided.