mirror of https://github.com/k3s-io/k3s
kubeadm: drop applyFlags.newK8sVersionStr field
The structure `applyFlags` is meant to keep a user's input from command line and as such should be immutable. Use either a variable or the validated `InitConfig.KubernetesVersion` field instead.pull/564/head
Dmitry Rozhkov
parent
8d69dc630b
commit
45ed7ae051
|
|
@ -54,7 +54,6 @@ type applyFlags struct {
|
|||
dryRun bool
|
||||
etcdUpgrade bool
|
||||
criSocket string
|
||||
newK8sVersionStr string
|
||||
imagePullTimeout time.Duration
|
||||
}
|
||||
|
||||
|
|
@ -77,6 +76,7 @@ func NewCmdApply(apf *applyPlanFlags) *cobra.Command {
|
|||
DisableFlagsInUseLine: true,
|
||||
Short: "Upgrade your Kubernetes cluster to the specified version.",
|
||||
Run: func(cmd *cobra.Command, args []string) {
|
||||
var userVersion string
|
||||
var err error
|
||||
flags.ignorePreflightErrorsSet, err = validation.ValidateIgnorePreflightErrors(flags.ignorePreflightErrors)
|
||||
kubeadmutil.CheckErr(err)
|
||||
|
|
@ -88,31 +88,30 @@ func NewCmdApply(apf *applyPlanFlags) *cobra.Command {
|
|||
|
||||
// If the version is specified in config file, pick up that value.
|
||||
if flags.cfgPath != "" {
|
||||
// Note that cfg isn't preserved here, it's just an one-off to populate flags.newK8sVersionStr based on --config
|
||||
// Note that cfg isn't preserved here, it's just an one-off to populate userVersion based on --config
|
||||
cfg, err := configutil.LoadInitConfigurationFromFile(flags.cfgPath)
|
||||
kubeadmutil.CheckErr(err)
|
||||
|
||||
if cfg.KubernetesVersion != "" {
|
||||
flags.newK8sVersionStr = cfg.KubernetesVersion
|
||||
userVersion = cfg.KubernetesVersion
|
||||
}
|
||||
}
|
||||
|
||||
// If the new version is already specified in config file, version arg is optional.
|
||||
if flags.newK8sVersionStr == "" {
|
||||
if userVersion == "" {
|
||||
err = cmdutil.ValidateExactArgNumber(args, []string{"version"})
|
||||
kubeadmutil.CheckErr(err)
|
||||
}
|
||||
|
||||
// If option was specified in both args and config file, args will overwrite the config file.
|
||||
if len(args) == 1 {
|
||||
flags.newK8sVersionStr = args[0]
|
||||
userVersion = args[0]
|
||||
}
|
||||
|
||||
// Default the flags dynamically, based on each others' value
|
||||
err = SetImplicitFlags(flags)
|
||||
err = assertVersionStringIsEmpty(userVersion)
|
||||
kubeadmutil.CheckErr(err)
|
||||
|
||||
err = runApply(flags)
|
||||
err = runApply(flags, userVersion)
|
||||
kubeadmutil.CheckErr(err)
|
||||
},
|
||||
}
|
||||
|
|
@ -145,12 +144,12 @@ func NewCmdApply(apf *applyPlanFlags) *cobra.Command {
|
|||
// - Creating the RBAC rules for the bootstrap tokens and the cluster-info ConfigMap
|
||||
// - Applying new kube-dns and kube-proxy manifests
|
||||
// - Uploads the newly used configuration to the cluster ConfigMap
|
||||
func runApply(flags *applyFlags) error {
|
||||
func runApply(flags *applyFlags, userVersion string) error {
|
||||
|
||||
// Start with the basics, verify that the cluster is healthy and get the configuration from the cluster (using the ConfigMap)
|
||||
klog.V(1).Infof("[upgrade/apply] verifying health of cluster")
|
||||
klog.V(1).Infof("[upgrade/apply] retrieving configuration from cluster")
|
||||
client, versionGetter, cfg, err := enforceRequirements(flags.applyPlanFlags, flags.dryRun, flags.newK8sVersionStr)
|
||||
client, versionGetter, cfg, err := enforceRequirements(flags.applyPlanFlags, flags.dryRun, userVersion)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
|
@ -167,7 +166,6 @@ func runApply(flags *applyFlags) error {
|
|||
}
|
||||
|
||||
// Use normalized version string in all following code.
|
||||
flags.newK8sVersionStr = cfg.KubernetesVersion
|
||||
newK8sVersion, err := version.ParseSemantic(cfg.KubernetesVersion)
|
||||
if err != nil {
|
||||
return errors.Errorf("unable to parse normalized version %q as a semantic version", cfg.KubernetesVersion)
|
||||
|
|
@ -179,7 +177,7 @@ func runApply(flags *applyFlags) error {
|
|||
|
||||
// Enforce the version skew policies
|
||||
klog.V(1).Infof("[upgrade/version] enforcing version skew policies")
|
||||
if err := EnforceVersionPolicies(newK8sVersion, flags, versionGetter); err != nil {
|
||||
if err := EnforceVersionPolicies(cfg.KubernetesVersion, newK8sVersion, flags, versionGetter); err != nil {
|
||||
return errors.Wrap(err, "[upgrade/version] FATAL")
|
||||
}
|
||||
|
||||
|
|
@ -222,16 +220,15 @@ func runApply(flags *applyFlags) error {
|
|||
}
|
||||
|
||||
fmt.Println("")
|
||||
fmt.Printf("[upgrade/successful] SUCCESS! Your cluster was upgraded to %q. Enjoy!\n", flags.newK8sVersionStr)
|
||||
fmt.Printf("[upgrade/successful] SUCCESS! Your cluster was upgraded to %q. Enjoy!\n", cfg.KubernetesVersion)
|
||||
fmt.Println("")
|
||||
fmt.Println("[upgrade/kubelet] Now that your control plane is upgraded, please proceed with upgrading your kubelets if you haven't already done so.")
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// SetImplicitFlags handles dynamically defaulting flags based on each other's value
|
||||
func SetImplicitFlags(flags *applyFlags) error {
|
||||
if len(flags.newK8sVersionStr) == 0 {
|
||||
func assertVersionStringIsEmpty(version string) error {
|
||||
if len(version) == 0 {
|
||||
return errors.New("version string can't be empty")
|
||||
}
|
||||
|
||||
|
|
@ -240,10 +237,10 @@ func SetImplicitFlags(flags *applyFlags) error {
|
|||
|
||||
// EnforceVersionPolicies makes sure that the version the user specified is valid to upgrade to
|
||||
// There are both fatal and skippable (with --force) errors
|
||||
func EnforceVersionPolicies(newK8sVersion *version.Version, flags *applyFlags, versionGetter upgrade.VersionGetter) error {
|
||||
fmt.Printf("[upgrade/version] You have chosen to change the cluster version to %q\n", flags.newK8sVersionStr)
|
||||
func EnforceVersionPolicies(newK8sVersionStr string, newK8sVersion *version.Version, flags *applyFlags, versionGetter upgrade.VersionGetter) error {
|
||||
fmt.Printf("[upgrade/version] You have chosen to change the cluster version to %q\n", newK8sVersionStr)
|
||||
|
||||
versionSkewErrs := upgrade.EnforceVersionPolicies(versionGetter, flags.newK8sVersionStr, newK8sVersion, flags.allowExperimentalUpgrades, flags.allowRCUpgrades)
|
||||
versionSkewErrs := upgrade.EnforceVersionPolicies(versionGetter, newK8sVersionStr, newK8sVersion, flags.allowExperimentalUpgrades, flags.allowRCUpgrades)
|
||||
if versionSkewErrs != nil {
|
||||
|
||||
if len(versionSkewErrs.Mandatory) > 0 {
|
||||
|
|
@ -268,7 +265,7 @@ func EnforceVersionPolicies(newK8sVersion *version.Version, flags *applyFlags, v
|
|||
func PerformControlPlaneUpgrade(flags *applyFlags, client clientset.Interface, waiter apiclient.Waiter, internalcfg *kubeadmapi.InitConfiguration) error {
|
||||
|
||||
// OK, the cluster is hosted using static pods. Upgrade a static-pod hosted cluster
|
||||
fmt.Printf("[upgrade/apply] Upgrading your Static Pod-hosted control plane to version %q...\n", flags.newK8sVersionStr)
|
||||
fmt.Printf("[upgrade/apply] Upgrading your Static Pod-hosted control plane to version %q...\n", internalcfg.KubernetesVersion)
|
||||
|
||||
if flags.dryRun {
|
||||
return DryRunStaticPodUpgrade(internalcfg)
|
||||
|
|
|
|||
|
|
@ -19,48 +19,31 @@ package upgrade
|
|||
import (
|
||||
"io/ioutil"
|
||||
"os"
|
||||
"reflect"
|
||||
"testing"
|
||||
|
||||
kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
|
||||
"k8s.io/kubernetes/cmd/kubeadm/app/constants"
|
||||
)
|
||||
|
||||
func TestSetImplicitFlags(t *testing.T) {
|
||||
var tests = []struct {
|
||||
name string
|
||||
flags *applyFlags
|
||||
expectedFlags applyFlags
|
||||
errExpected bool
|
||||
func TestAssertVersionStringIsEmpty(t *testing.T) {
|
||||
var tcases = []struct {
|
||||
name string
|
||||
version string
|
||||
expectedErr bool
|
||||
}{
|
||||
{
|
||||
name: "if the new version is empty; it should error out",
|
||||
flags: &applyFlags{
|
||||
newK8sVersionStr: "",
|
||||
},
|
||||
expectedFlags: applyFlags{
|
||||
newK8sVersionStr: "",
|
||||
},
|
||||
errExpected: true,
|
||||
name: "Version string is not empty",
|
||||
version: "some string",
|
||||
},
|
||||
{
|
||||
name: "Version string is empty",
|
||||
expectedErr: true,
|
||||
},
|
||||
}
|
||||
for _, rt := range tests {
|
||||
t.Run(rt.name, func(t *testing.T) {
|
||||
actualErr := SetImplicitFlags(rt.flags)
|
||||
|
||||
if !reflect.DeepEqual(*rt.flags, rt.expectedFlags) {
|
||||
t.Errorf(
|
||||
"failed SetImplicitFlags:\n\texpected flags: %v\n\t actual: %v",
|
||||
rt.expectedFlags,
|
||||
*rt.flags,
|
||||
)
|
||||
}
|
||||
if (actualErr != nil) != rt.errExpected {
|
||||
t.Errorf(
|
||||
"failed SetImplicitFlags:\n\texpected error: %t\n\t actual: %t",
|
||||
rt.errExpected,
|
||||
(actualErr != nil),
|
||||
)
|
||||
for _, tt := range tcases {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if assertVersionStringIsEmpty(tt.version) == nil && tt.expectedErr {
|
||||
t.Errorf("No error triggered for string '%s'", tt.version)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -38,8 +38,6 @@ import (
|
|||
|
||||
type planFlags struct {
|
||||
*applyPlanFlags
|
||||
|
||||
newK8sVersionStr string
|
||||
}
|
||||
|
||||
// NewCmdPlan returns the cobra command for `kubeadm upgrade plan`
|
||||
|
|
@ -52,6 +50,7 @@ func NewCmdPlan(apf *applyPlanFlags) *cobra.Command {
|
|||
Use: "plan [version] [flags]",
|
||||
Short: "Check which versions are available to upgrade to and validate whether your current cluster is upgradeable. To skip the internet check, pass in the optional [version] parameter.",
|
||||
Run: func(_ *cobra.Command, args []string) {
|
||||
var userVersion string
|
||||
var err error
|
||||
flags.ignorePreflightErrorsSet, err = validation.ValidateIgnorePreflightErrors(flags.ignorePreflightErrors)
|
||||
kubeadmutil.CheckErr(err)
|
||||
|
|
@ -65,15 +64,15 @@ func NewCmdPlan(apf *applyPlanFlags) *cobra.Command {
|
|||
kubeadmutil.CheckErr(err)
|
||||
|
||||
if cfg.KubernetesVersion != "" {
|
||||
flags.newK8sVersionStr = cfg.KubernetesVersion
|
||||
userVersion = cfg.KubernetesVersion
|
||||
}
|
||||
}
|
||||
// If option was specified in both args and config file, args will overwrite the config file.
|
||||
if len(args) == 1 {
|
||||
flags.newK8sVersionStr = args[0]
|
||||
userVersion = args[0]
|
||||
}
|
||||
|
||||
err = runPlan(flags)
|
||||
err = runPlan(flags, userVersion)
|
||||
kubeadmutil.CheckErr(err)
|
||||
},
|
||||
}
|
||||
|
|
@ -84,11 +83,11 @@ func NewCmdPlan(apf *applyPlanFlags) *cobra.Command {
|
|||
}
|
||||
|
||||
// runPlan takes care of outputting available versions to upgrade to for the user
|
||||
func runPlan(flags *planFlags) error {
|
||||
func runPlan(flags *planFlags, userVersion string) error {
|
||||
// Start with the basics, verify that the cluster is healthy, build a client and a versionGetter. Never dry-run when planning.
|
||||
klog.V(1).Infof("[upgrade/plan] verifying health of cluster")
|
||||
klog.V(1).Infof("[upgrade/plan] retrieving configuration from cluster")
|
||||
client, versionGetter, cfg, err := enforceRequirements(flags.applyPlanFlags, false, flags.newK8sVersionStr)
|
||||
client, versionGetter, cfg, err := enforceRequirements(flags.applyPlanFlags, false, userVersion)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue