From 2d37ab1ca50098fe9d0142a32eae995f8fb29e97 Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Mon, 6 Feb 2017 00:49:19 -0500 Subject: [PATCH 1/2] Recommend a k8s version based on each kops version So the flow is that we recommend (or strongly recommend) a new kops version when one is required for a new version, and then the new kops version will recommend (or strongly recommend) a new k8s version. We don't have a notion of multiple recommended k8s versions per kops version - that is what channels are for. Users are always free to disregard updates, even "required" ones by setting a flag. --- channels/alpha | 6 +- channels/pkg/channels/channel_version.go | 4 +- cmd/kops/upgrade_cluster.go | 44 +++++--- nodeup/pkg/model/docker.go | 2 +- pkg/apis/kops/channel.go | 129 ++++++++++++++++++++--- upup/pkg/fi/cloudup/apply_cluster.go | 31 +++--- upup/pkg/fi/cloudup/defaults.go | 5 +- upup/pkg/kutil/convert_kubeup_cluster.go | 7 +- 8 files changed, 173 insertions(+), 55 deletions(-) diff --git a/channels/alpha b/channels/alpha index c01d403af6..2b0143b5bf 100644 --- a/channels/alpha +++ b/channels/alpha @@ -20,8 +20,10 @@ spec: requiredVersion: 1.4.2 kopsVersions: - range: ">=1.5.0-alpha1" - recommendedVersion: 1.5.0-beta1 - #requiredVersion: 1.5.0-beta1 + recommendedVersion: 1.5.0-beta2 + #requiredVersion: 1.5.0-beta2 + kubernetesVersion: 1.5.2 - range: "<1.5.0" recommendedVersion: 1.4.4 #requiredVersion: 1.4.4 + kubernetesVersion: 1.4.8 diff --git a/channels/pkg/channels/channel_version.go b/channels/pkg/channels/channel_version.go index 0101b84c36..0e1849bc98 100644 --- a/channels/pkg/channels/channel_version.go +++ b/channels/pkg/channels/channel_version.go @@ -96,12 +96,12 @@ func (c *ChannelVersion) Replaces(existing *ChannelVersion) bool { if c.Version == nil { return false } - cVersion, err := semver.Parse(*c.Version) + cVersion, err := semver.ParseTolerant(*c.Version) if err != nil { glog.Warningf("error parsing version %q; will ignore this version", *c.Version) return false } - existingVersion, err := semver.Parse(*existing.Version) + existingVersion, err := semver.ParseTolerant(*existing.Version) if err != nil { glog.Warningf("error parsing existing version %q", *existing.Version) return true diff --git a/cmd/kops/upgrade_cluster.go b/cmd/kops/upgrade_cluster.go index f172c49fe4..7e0727aefe 100644 --- a/cmd/kops/upgrade_cluster.go +++ b/cmd/kops/upgrade_cluster.go @@ -20,6 +20,7 @@ import ( "fmt" "os" + "github.com/blang/semver" "github.com/golang/glog" "github.com/spf13/cobra" api "k8s.io/kops/pkg/apis/kops" @@ -129,26 +130,40 @@ func (c *UpgradeClusterCmd) Run(args []string) error { channelClusterSpec = &api.ClusterSpec{} } - //latestKubernetesVersion, err := api.FindLatestKubernetesVersion() - //if err != nil { - // return err - //} + var currentKubernetesVersion *semver.Version + { + sv, err := util.ParseKubernetesVersion(cluster.Spec.KubernetesVersion) + if err != nil { + glog.Warningf("error parsing KubernetesVersion %q", cluster.Spec.KubernetesVersion) + } else { + currentKubernetesVersion = sv + } + } - // So we can propose an image for the upgraded k8s version - upgradedKubernetesVersion := cluster.Spec.KubernetesVersion + proposedKubernetesVersion := api.RecommendedKubernetesVersion(channel) - if channelClusterSpec.KubernetesVersion != "" && cluster.Spec.KubernetesVersion != channelClusterSpec.KubernetesVersion { + // We won't propose a downgrade + // TODO: What if a kubernetes version is bad? + if currentKubernetesVersion != nil && proposedKubernetesVersion != nil && currentKubernetesVersion.GT(*proposedKubernetesVersion) { + glog.Warningf("cluster version %q is greater than recommended version %q", *currentKubernetesVersion, *proposedKubernetesVersion) + proposedKubernetesVersion = currentKubernetesVersion + } + + if proposedKubernetesVersion != nil && currentKubernetesVersion != nil && currentKubernetesVersion.NE(*proposedKubernetesVersion) { actions = append(actions, &upgradeAction{ Item: "Cluster", Property: "KubernetesVersion", Old: cluster.Spec.KubernetesVersion, - New: channelClusterSpec.KubernetesVersion, + New: proposedKubernetesVersion.String(), apply: func() { - cluster.Spec.KubernetesVersion = channelClusterSpec.KubernetesVersion + cluster.Spec.KubernetesVersion = proposedKubernetesVersion.String() }, }) + } - upgradedKubernetesVersion = channelClusterSpec.KubernetesVersion + // For further calculations, default to the current kubernetes version + if proposedKubernetesVersion == nil { + proposedKubernetesVersion = currentKubernetesVersion } // Prompt to upgrade addins? @@ -179,13 +194,8 @@ func (c *UpgradeClusterCmd) Run(args []string) error { } // Prompt to upgrade image - { - sv, err := util.ParseKubernetesVersion(upgradedKubernetesVersion) - if err != nil { - // We could continue, but something has gone wrong here... - return fmt.Errorf("unable to parse kubernetes version %q", upgradedKubernetesVersion) - } - image := channel.FindImage(cloud.ProviderID(), *sv) + if proposedKubernetesVersion != nil { + image := channel.FindImage(cloud.ProviderID(), *proposedKubernetesVersion) if image == nil { glog.Warningf("No matching images specified in channel; cannot prompt for upgrade") diff --git a/nodeup/pkg/model/docker.go b/nodeup/pkg/model/docker.go index 7afdc2a1e0..0e47616895 100644 --- a/nodeup/pkg/model/docker.go +++ b/nodeup/pkg/model/docker.go @@ -272,7 +272,7 @@ func (b *DockerBuilder) Build(c *fi.ModelBuilderContext) error { } } - dockerSemver, err := semver.Parse(dockerVersion) + dockerSemver, err := semver.ParseTolerant(dockerVersion) if err != nil { return fmt.Errorf("error parsing docker version %q as semver: %v", dockerVersion, err) } diff --git a/pkg/apis/kops/channel.go b/pkg/apis/kops/channel.go index df648c5e06..c6b1ab9716 100644 --- a/pkg/apis/kops/channel.go +++ b/pkg/apis/kops/channel.go @@ -20,6 +20,8 @@ import ( "fmt" "github.com/blang/semver" "github.com/golang/glog" + "k8s.io/kops" + "k8s.io/kops/pkg/apis/kops/util" "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/util/pkg/vfs" "k8s.io/kubernetes/pkg/api" @@ -44,13 +46,22 @@ type ChannelSpec struct { Cluster *ClusterSpec `json:"cluster,omitempty"` // KopsVersions allows us to recommend/require kops versions - KopsVersions []VersionSpec `json:"kopsVersions,omitempty"` + KopsVersions []KopsVersionSpec `json:"kopsVersions,omitempty"` // KubernetesVersions allows us to recommend/requires kubernetes versions - KubernetesVersions []VersionSpec `json:"kubernetesVersions,omitempty"` + KubernetesVersions []KubernetesVersionSpec `json:"kubernetesVersions,omitempty"` } -type VersionSpec struct { +type KopsVersionSpec struct { + Range string `json:"range,omitempty"` + + RecommendedVersion string `json:"recommendedVersion,omitempty"` + RequiredVersion string `json:"requiredVersion,omitempty"` + + KubernetesVersion string `json:"kubernetesVersion,omitempty"` +} + +type KubernetesVersionSpec struct { Range string `json:"range,omitempty"` RecommendedVersion string `json:"recommendedVersion,omitempty"` @@ -110,33 +121,53 @@ func ParseChannel(channelBytes []byte) (*Channel, error) { } // FindRecommendedUpgrade returns a string with a new version, if the current version is out of date -func FindRecommendedUpgrade(v *VersionSpec, version semver.Version) (string, error) { +func (v *KubernetesVersionSpec) FindRecommendedUpgrade(version semver.Version) (*semver.Version, error) { if v.RecommendedVersion == "" { glog.V(2).Infof("VersionRecommendationSpec does not specify RecommendedVersion") - return "", nil + return nil, nil } - recommendedVersion, err := semver.Parse(v.RecommendedVersion) + recommendedVersion, err := util.ParseKubernetesVersion(v.RecommendedVersion) if err != nil { - return "", fmt.Errorf("error parsing RecommendedVersion %q from channel", v.RecommendedVersion) + return nil, fmt.Errorf("error parsing RecommendedVersion %q from channel", v.RecommendedVersion) } if recommendedVersion.GT(version) { glog.V(2).Infof("RecommendedVersion=%q, Have=%q. Recommending upgrade", recommendedVersion, version) - return v.RecommendedVersion, nil + return recommendedVersion, nil } else { glog.V(4).Infof("RecommendedVersion=%q, Have=%q. No upgrade needed.", recommendedVersion, version) } - return "", nil + return nil, nil +} + +// FindRecommendedUpgrade returns a string with a new version, if the current version is out of date +func (v *KopsVersionSpec) FindRecommendedUpgrade(version semver.Version) (*semver.Version, error) { + if v.RecommendedVersion == "" { + glog.V(2).Infof("VersionRecommendationSpec does not specify RecommendedVersion") + return nil, nil + } + + recommendedVersion, err := semver.ParseTolerant(v.RecommendedVersion) + if err != nil { + return nil, fmt.Errorf("error parsing RecommendedVersion %q from channel", v.RecommendedVersion) + } + if recommendedVersion.GT(version) { + glog.V(2).Infof("RecommendedVersion=%q, Have=%q. Recommending upgrade", recommendedVersion, version) + return &recommendedVersion, nil + } else { + glog.V(4).Infof("RecommendedVersion=%q, Have=%q. No upgrade needed.", recommendedVersion, version) + } + return nil, nil } // IsUpgradeRequired returns true if the current version is not acceptable -func IsUpgradeRequired(v *VersionSpec, version semver.Version) (bool, error) { +func (v *KubernetesVersionSpec) IsUpgradeRequired(version semver.Version) (bool, error) { if v.RequiredVersion == "" { glog.V(2).Infof("VersionRecommendationSpec does not specify RequiredVersion") return false, nil } - requiredVersion, err := semver.Parse(v.RequiredVersion) + requiredVersion, err := util.ParseKubernetesVersion(v.RequiredVersion) if err != nil { return false, fmt.Errorf("error parsing RequiredVersion %q from channel", v.RequiredVersion) } @@ -149,8 +180,49 @@ func IsUpgradeRequired(v *VersionSpec, version semver.Version) (bool, error) { return false, nil } -// FindVersionInfo returns a VersionSpec for the current version -func FindVersionInfo(versions []VersionSpec, version semver.Version) *VersionSpec { +// IsUpgradeRequired returns true if the current version is not acceptable +func (v *KopsVersionSpec) IsUpgradeRequired(version semver.Version) (bool, error) { + if v.RequiredVersion == "" { + glog.V(2).Infof("VersionRecommendationSpec does not specify RequiredVersion") + return false, nil + } + + requiredVersion, err := semver.ParseTolerant(v.RequiredVersion) + if err != nil { + return false, fmt.Errorf("error parsing RequiredVersion %q from channel", v.RequiredVersion) + } + if requiredVersion.GT(version) { + glog.V(2).Infof("RequiredVersion=%q, Have=%q. Requiring upgrade", requiredVersion, version) + return true, nil + } else { + glog.V(4).Infof("RequiredVersion=%q, Have=%q. No upgrade needed.", requiredVersion, version) + } + return false, nil +} + +// FindKubernetesVersionSpec returns a KubernetesVersionSpec for the current version +func FindKubernetesVersionSpec(versions []KubernetesVersionSpec, version semver.Version) *KubernetesVersionSpec { + for i := range versions { + v := &versions[i] + if v.Range != "" { + versionRange, err := semver.ParseRange(v.Range) + if err != nil { + glog.Warningf("unable to parse range in channel version spec: %q", v.Range) + continue + } + if !versionRange(version) { + glog.V(8).Infof("version range %q does not apply to version %q; skipping", v.Range, version) + continue + } + } + return v + } + + return nil +} + +// FindKopsVersionSpec returns a KopsVersionSpec for the current version +func FindKopsVersionSpec(versions []KopsVersionSpec, version semver.Version) *KopsVersionSpec { for i := range versions { v := &versions[i] if v.Range != "" { @@ -203,3 +275,34 @@ func (c *Channel) FindImage(provider fi.CloudProviderID, kubernetesVersion semve } return matches[0] } + +func RecommendedKubernetesVersion(c *Channel) *semver.Version { + kopsVersionString := kops.Version + kopsVersion, err := semver.ParseTolerant(kopsVersionString) + if err != nil { + glog.Warningf("unable to parse kops version %q", kopsVersionString) + } else { + kopsVersionSpec := FindKopsVersionSpec(c.Spec.KopsVersions, kopsVersion) + if kopsVersionSpec != nil { + if kopsVersionSpec.KubernetesVersion != "" { + sv, err := util.ParseKubernetesVersion(kopsVersionSpec.KubernetesVersion) + if err != nil { + glog.Warningf("unable to parse kubernetes version %q", kopsVersionSpec.KubernetesVersion) + } else { + return sv + } + } + } + } + + if c.Spec.Cluster != nil { + sv, err := util.ParseKubernetesVersion(c.Spec.Cluster.KubernetesVersion) + if err != nil { + glog.Warningf("unable to parse kubernetes version %q", c.Spec.Cluster.KubernetesVersion) + } else { + return sv + } + } + + return nil +} diff --git a/upup/pkg/fi/cloudup/apply_cluster.go b/upup/pkg/fi/cloudup/apply_cluster.go index 391c473ede..b4ab910ca6 100644 --- a/upup/pkg/fi/cloudup/apply_cluster.go +++ b/upup/pkg/fi/cloudup/apply_cluster.go @@ -623,38 +623,38 @@ func (c *ApplyClusterCmd) upgradeSpecs() error { // validateKopsVersion ensures that kops meet the version requirements / recommendations in the channel func (c *ApplyClusterCmd) validateKopsVersion() error { - kopsVersion, err := semver.Parse(kops.Version) + kopsVersion, err := semver.ParseTolerant(kops.Version) if err != nil { glog.Warningf("unable to parse kops version %q", kops.Version) // Not a hard-error return nil } - versionInfo := api.FindVersionInfo(c.channel.Spec.KopsVersions, kopsVersion) + versionInfo := api.FindKopsVersionSpec(c.channel.Spec.KopsVersions, kopsVersion) if versionInfo == nil { glog.Warningf("unable to find version information for kops version %q in channel", kopsVersion) // Not a hard-error return nil } - recommended, err := api.FindRecommendedUpgrade(versionInfo, kopsVersion) + recommended, err := versionInfo.FindRecommendedUpgrade(kopsVersion) if err != nil { glog.Warningf("unable to parse version recommendation for kops version %q in channel", kopsVersion) } - required, err := api.IsUpgradeRequired(versionInfo, kopsVersion) + required, err := versionInfo.IsUpgradeRequired(kopsVersion) if err != nil { glog.Warningf("unable to parse version requirement for kops version %q in channel", kopsVersion) } - if recommended != "" && !required { + if recommended != nil && !required { fmt.Printf("\n") fmt.Printf(starline) fmt.Printf("\n") fmt.Printf("A new kops version is available: %s\n", recommended) fmt.Printf("\n") fmt.Printf("Upgrading is recommended\n") - fmt.Printf("More information: %s\n", buildPermalink("upgrade_kops", recommended)) + fmt.Printf("More information: %s\n", buildPermalink("upgrade_kops", recommended.String())) fmt.Printf("\n") fmt.Printf(starline) fmt.Printf("\n") @@ -662,18 +662,17 @@ func (c *ApplyClusterCmd) validateKopsVersion() error { fmt.Printf("\n") fmt.Printf(starline) fmt.Printf("\n") - if recommended != "" { + if recommended != nil { fmt.Printf("A new kops version is available: %s\n", recommended) } fmt.Printf("\n") fmt.Printf("This version of kops is no longer supported; upgrading is required\n") fmt.Printf("(you can bypass this check by exporting KOPS_RUN_OBSOLETE_VERSION)\n") fmt.Printf("\n") - fmt.Printf("More information: %s\n", buildPermalink("upgrade_kops", recommended)) + fmt.Printf("More information: %s\n", buildPermalink("upgrade_kops", recommended.String())) fmt.Printf("\n") fmt.Printf(starline) fmt.Printf("\n") - } if required { @@ -697,31 +696,31 @@ func (c *ApplyClusterCmd) validateKubernetesVersion() error { // TODO: make util.ParseKubernetesVersion not return a pointer kubernetesVersion := *parsed - versionInfo := api.FindVersionInfo(c.channel.Spec.KubernetesVersions, kubernetesVersion) + versionInfo := api.FindKubernetesVersionSpec(c.channel.Spec.KubernetesVersions, kubernetesVersion) if versionInfo == nil { glog.Warningf("unable to find version information for kubernetes version %q in channel", kubernetesVersion) // Not a hard-error return nil } - recommended, err := api.FindRecommendedUpgrade(versionInfo, kubernetesVersion) + recommended, err := versionInfo.FindRecommendedUpgrade(kubernetesVersion) if err != nil { glog.Warningf("unable to parse version recommendation for kubernetes version %q in channel", kubernetesVersion) } - required, err := api.IsUpgradeRequired(versionInfo, kubernetesVersion) + required, err := versionInfo.IsUpgradeRequired(kubernetesVersion) if err != nil { glog.Warningf("unable to parse version requirement for kubernetes version %q in channel", kubernetesVersion) } - if recommended != "" && !required { + if recommended != nil && !required { fmt.Printf("\n") fmt.Printf(starline) fmt.Printf("\n") fmt.Printf("A new kubernetes version is available: %s\n", recommended) fmt.Printf("Upgrading is recommended (try kops upgrade cluster)\n") fmt.Printf("\n") - fmt.Printf("More information: %s\n", buildPermalink("upgrade_k8s", recommended)) + fmt.Printf("More information: %s\n", buildPermalink("upgrade_k8s", recommended.String())) fmt.Printf("\n") fmt.Printf(starline) fmt.Printf("\n") @@ -729,14 +728,14 @@ func (c *ApplyClusterCmd) validateKubernetesVersion() error { fmt.Printf("\n") fmt.Printf(starline) fmt.Printf("\n") - if recommended != "" { + if recommended != nil { fmt.Printf("A new kubernetes version is available: %s\n", recommended) } fmt.Printf("\n") fmt.Printf("This version of kubernetes is no longer supported; upgrading is required\n") fmt.Printf("(you can bypass this check by exporting KOPS_RUN_OBSOLETE_VERSION)\n") fmt.Printf("\n") - fmt.Printf("More information: %s\n", buildPermalink("upgrade_k8s", recommended)) + fmt.Printf("More information: %s\n", buildPermalink("upgrade_k8s", recommended.String())) fmt.Printf("\n") fmt.Printf(starline) fmt.Printf("\n") diff --git a/upup/pkg/fi/cloudup/defaults.go b/upup/pkg/fi/cloudup/defaults.go index 9a498c466a..fb7870a409 100644 --- a/upup/pkg/fi/cloudup/defaults.go +++ b/upup/pkg/fi/cloudup/defaults.go @@ -91,8 +91,9 @@ func ensureKubernetesVersion(c *kops.Cluster) error { if err != nil { return err } - if channel.Spec.Cluster.KubernetesVersion != "" { - c.Spec.KubernetesVersion = channel.Spec.Cluster.KubernetesVersion + kubernetesVersion := kops.RecommendedKubernetesVersion(channel) + if kubernetesVersion != nil { + c.Spec.KubernetesVersion = kubernetesVersion.String() } } } diff --git a/upup/pkg/kutil/convert_kubeup_cluster.go b/upup/pkg/kutil/convert_kubeup_cluster.go index ff89eccb08..9e39711b6c 100644 --- a/upup/pkg/kutil/convert_kubeup_cluster.go +++ b/upup/pkg/kutil/convert_kubeup_cluster.go @@ -86,8 +86,11 @@ func (x *ConvertKubeupCluster) Upgrade() error { } // Set KubernetesVersion from channel - if x.Channel != nil && x.Channel.Spec.Cluster != nil && x.Channel.Spec.Cluster.KubernetesVersion != "" { - cluster.Spec.KubernetesVersion = x.Channel.Spec.Cluster.KubernetesVersion + if x.Channel != nil { + kubernetesVersion := api.RecommendedKubernetesVersion(x.Channel) + if kubernetesVersion != nil { + cluster.Spec.KubernetesVersion = kubernetesVersion.String() + } } err = cloudup.PerformAssignments(cluster) From 32bd620f3659d1730f7b8a0a68e39d5bd5b029e8 Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Mon, 6 Feb 2017 16:23:00 -0500 Subject: [PATCH 2/2] Fix tests --- tests/integration/channel/integration_test.go | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/tests/integration/channel/integration_test.go b/tests/integration/channel/integration_test.go index bcdf397b8d..150b56c260 100644 --- a/tests/integration/channel/integration_test.go +++ b/tests/integration/channel/integration_test.go @@ -80,13 +80,13 @@ func TestKopsUpgrades(t *testing.T) { for _, g := range grid { kopsVersion := semver.MustParse(g.KopsVersion) - versionInfo := kops.FindVersionInfo(channel.Spec.KopsVersions, kopsVersion) + versionInfo := kops.FindKopsVersionSpec(channel.Spec.KopsVersions, kopsVersion) if versionInfo == nil { t.Errorf("unable to find version information for kops version %q in channel", kopsVersion) continue } - actual, err := kops.FindRecommendedUpgrade(versionInfo, kopsVersion) + actual, err := versionInfo.FindRecommendedUpgrade(kopsVersion) if g.ExpectedError { if err == nil { t.Errorf("expected error from FindRecommendedUpgrade(%q)", g.KopsVersion) @@ -98,12 +98,12 @@ func TestKopsUpgrades(t *testing.T) { continue } } - if actual != g.ExpectedUpgrade { + if semverString(actual) != g.ExpectedUpgrade { t.Errorf("unexpected result from IsUpgradeRequired(%q): expected=%q, actual=%q", g.KopsVersion, g.ExpectedUpgrade, actual) continue } - required, err := kops.IsUpgradeRequired(versionInfo, kopsVersion) + required, err := versionInfo.IsUpgradeRequired(kopsVersion) if err != nil { t.Errorf("unexpected error from IsUpgradeRequired(%q)", g.KopsVersion, err) continue @@ -174,13 +174,13 @@ func TestKubernetesUpgrades(t *testing.T) { for _, g := range grid { kubernetesVersion := semver.MustParse(g.KubernetesVersion) - versionInfo := kops.FindVersionInfo(channel.Spec.KubernetesVersions, kubernetesVersion) + versionInfo := kops.FindKubernetesVersionSpec(channel.Spec.KubernetesVersions, kubernetesVersion) if versionInfo == nil { t.Errorf("unable to find version information for kubernetes version %q in channel", kubernetesVersion) continue } - actual, err := kops.FindRecommendedUpgrade(versionInfo, kubernetesVersion) + actual, err := versionInfo.FindRecommendedUpgrade(kubernetesVersion) if g.ExpectedError { if err == nil { t.Errorf("expected error from FindRecommendedUpgrade(%q)", g.KubernetesVersion) @@ -192,12 +192,12 @@ func TestKubernetesUpgrades(t *testing.T) { continue } } - if actual != g.ExpectedUpgrade { + if semverString(actual) != g.ExpectedUpgrade { t.Errorf("unexpected result from IsUpgradeRequired(%q): expected=%q, actual=%q", g.KubernetesVersion, g.ExpectedUpgrade, actual) continue } - required, err := kops.IsUpgradeRequired(versionInfo, kubernetesVersion) + required, err := versionInfo.IsUpgradeRequired(kubernetesVersion) if err != nil { t.Errorf("unexpected error from IsUpgradeRequired(%q)", g.KubernetesVersion, err) continue @@ -249,3 +249,10 @@ func TestFindImage(t *testing.T) { } } } + +func semverString(sv *semver.Version) string { + if sv == nil { + return "" + } + return sv.String() +}