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

Failing-fast or warn when tried to update cluster created with older kube-aws #585

Closed
mumoshu opened this issue Apr 24, 2017 · 15 comments
Closed
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Milestone

Comments

@mumoshu
Copy link
Contributor

mumoshu commented Apr 24, 2017

According to #455, you need to recreate your cluster to upgrade kube-aws as of today.
However, I believe some people are still getting into trouble trying to run kube-aws update with newer kube-aws on their old clusters.
Making kube-aws failing fast when tried to do so would allow people to notice the fact.

@danielfm
Copy link
Contributor

danielfm commented Apr 24, 2017

Couldn't we add a metadata to cluster.yaml to specify the kube-aws version used to generate it, and then only allow further commands, like render, up and update to go through if the versions match?

EDIT: That's what I'm doing here; I manually added a comment at the top of my cluster.yaml to document the kube-aws version I should use to manage that cluster.

@mumoshu
Copy link
Contributor Author

mumoshu commented Apr 24, 2017

@danielfm Thanks for the suggestion. That sounds good to me 👍
Actually, I was initially thinking about adding a version tag to the root cfn stack so that we can use it to prevent kube-aws update to be run. Probably yours is better though.

@cknowles
Copy link
Contributor

I think users may just update it in the yaml if it's in there? Especially if they use the git flow style mechanism of updating. Having said that, ideally we only want to prevent updates where we know running update will break.

@mumoshu
Copy link
Contributor Author

mumoshu commented Apr 26, 2017

@c-knowles

I think users may just update it in the yaml if it's in there? Especially if they use the git flow style mechanism of updating.

Good point. Then, we'd better add a cfn tag via a template snippet like "Value": "{{.KubeAwsVersion}}" so that the git workflow like described in #462 #238 won't accidentally update the version number associated to a cluster.

@cknowles
Copy link
Contributor

@mumoshu if we do that though, how do we support upgrade path for cases that do work? We reject by default unless an update is allowed? Or allow by default unless an update is disallowed?

@mumoshu
Copy link
Contributor Author

mumoshu commented Apr 26, 2017

@c-knowles I guess the former is better because then we can ensure that only upgrade paths tested by contributors can be allowed explicitly in code.

@redbaron
Copy link
Contributor

{{ .KubeAwsVersion }} is not informative enough as it mightbe just a git commit of a private fork :)

@mumoshu
Copy link
Contributor Author

mumoshu commented Apr 26, 2017

@redbaron Thanks for the feedback 👍

  1. How about tagging some commits in your private fork with version numbers so that you can explicitly control one version is updatable or not in code?
  2. Or I guess you can just leave it just as a git commit id so that kube-aws can prevent kube-aws update among different commit ids by default?
  3. If none of the above sounds good, how about adding an another flag in cluster.yaml to disable update prevention at all...? 😃

@redbaron
Copy link
Contributor

how kube-aws itself defines which version is updatable and which is not?

@mumoshu
Copy link
Contributor Author

mumoshu commented Apr 26, 2017

@redbaron Probably in a golang type and func which is used like:

func (g UpdateGate) ValidateClusterUpgradableTo(v Version) error {
    // All the upgrade paths tested by contributors
    testedUpgradePaths := TestedUpgradePaths{
        // Tested by @mumoshu
        TestedUpgradePath{ClusterVersion: NewVersion("v0.9.5"), KubeAwsVersion: NewVersion("v0.9.6")},
        // Tested by @redbaron
        TestedUpgradePath{ClusterVersion: NewVersion("v0.9.5"), KubeAwsVersion: NewVersion("v0.9.7")},
    }

    for p, _ := range testedUpgdatePaths {
        if p.MatchVersions(g.ClusterKubeAwsVersion, v) {
            return nil
        }
    }
    return fmt.Errorf("Automatic upgrade path from kube-aws %s to %s is not tested: Updating a cluster created by kube-aws %s  with kube-aws %s is not not allowed.", g.ClusterKubeAwsVersion, v, g.ClusterKubeAwsVersion, v))
}

// And somewhere in the code path of `kube-aws update`

if err := NewUpdateGate(clusterKubeAwsVersion).ValidateUpgradableTo(kubeAwsVersion); err != nil {
    return err
}

@mumoshu
Copy link
Contributor Author

mumoshu commented Apr 30, 2017

@redbaron Does my idea make sense to you? I'd appreciate your feedback 🙇

@mumoshu mumoshu added this to the backlog milestone May 12, 2017
@mumoshu mumoshu changed the title Failing-fast when tried to update cluster created with older kube-aws Failing-fast or warn when tried to update cluster created with older kube-aws Jul 5, 2017
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 21, 2019
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 21, 2019
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

No branches or pull requests

6 participants