Merge pull request #62568 from liztio/kubeadm-upgrade-same-version

Automatic merge from submit-queue (batch tested with PRs 62568, 62220, 62743, 62751, 62753). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Kubeadm upgrade same version What this PR does / why we need it: When kubeadm 1.10 came out, it inadvertently introduced a backwards incompatible config change. Because the kubeadm MasterConfiguration is written by the old version of kubeadm and read by the new one, this incompatibility causes the upgrade to fail. To mitigate this, I've written a simple transform that operates on a map-based version of the config. This map is mutated to make it compatible with the new structure, then serialised to JSON and deserialised by the usual APIMachinery. Because of complications with the multiple versions, this PR enforces kubeadm only being used to upgrade to kubernetes of the same minor and major versions. Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged): Fixes [kubeadm#744](https://github.com/kubernetes/kubeadm/issues/744#issuecomment-379045823L) This PR is an alternate take on #62353. Instead of trying to gate migration on versions, this constrains kubeadm to only upgrade versions from the same major and minor versions. Special notes for your reviewer: ```release-note fixes configuration error when upgrading kubeadm from 1.9 to 1.10+ enforces kubeadm upgrading kubernetes from the same major and minor versions as the kubeadm binary. ```
2018-04-17 20:52:08 -07:00 · 2018-04-17 20:52:08 -07:00 · edee03ed66
parent 26e432cb3b 3c92e358f7
commit edee03ed66
8 changed files with 425 additions and 40 deletions
--- a/cmd/kubeadm/app/apis/kubeadm/v1alpha1/BUILD
+++ b/cmd/kubeadm/app/apis/kubeadm/v1alpha1/BUILD
@ -1,4 +1,4 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_library")
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

 go_library(
    name = "go_default_library",
@ -7,6 +7,7 @@ go_library(
        "doc.go",
        "register.go",
        "types.go",
+        "upgrade.go",
        "zz_generated.conversion.go",
        "zz_generated.deepcopy.go",
        "zz_generated.defaults.go",
@ -52,12 +53,17 @@ go_library(
        "//cmd/kubeadm/app/apis/kubeadm:go_default_library",
        "//cmd/kubeadm/app/constants:go_default_library",
        "//cmd/kubeadm/app/features:go_default_library",
+        "//pkg/api/legacyscheme:go_default_library",
        "//pkg/kubelet/apis/kubeletconfig/scheme:go_default_library",
        "//pkg/kubelet/apis/kubeletconfig/v1beta1:go_default_library",
        "//pkg/proxy/apis/kubeproxyconfig/scheme:go_default_library",
        "//pkg/proxy/apis/kubeproxyconfig/v1alpha1:go_default_library",
+        "//vendor/github.com/json-iterator/go:go_default_library",
+        "//vendor/github.com/ugorji/go/codec:go_default_library",
+        "//vendor/gopkg.in/yaml.v2:go_default_library",
        "//vendor/k8s.io/api/core/v1:go_default_library",
        "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library",
        "//vendor/k8s.io/apimachinery/pkg/conversion:go_default_library",
        "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
        "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
@ -77,3 +83,11 @@ filegroup(
    tags = ["automanaged"],
    visibility = ["//visibility:public"],
 )
+
+go_test(
+    name = "go_default_test",
+    srcs = ["upgrade_test.go"],
+    data = glob(["testdata/**"]),
+    embed = [":go_default_library"],
+    deps = ["//vendor/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library"],
+)
--- a/cmd/kubeadm/app/apis/kubeadm/v1alpha1/testdata/kubeadm196.yaml
+++ b/cmd/kubeadm/app/apis/kubeadm/v1alpha1/testdata/kubeadm196.yaml
@ -0,0 +1,62 @@
+api:
+  advertiseAddress: 172.31.93.180
+  bindPort: 6443
+authorizationModes:
+- Node
+- RBAC
+certificatesDir: /etc/kubernetes/pki
+cloudProvider: aws
+etcd:
+  caFile: ""
+  certFile: ""
+  dataDir: /var/lib/etcd
+  endpoints: null
+  image: ""
+  keyFile: ""
+imageRepository: gcr.io/google_containers
+kubeProxy:
+  config:
+    bindAddress: 0.0.0.0
+    clientConnection:
+      acceptContentTypes: ""
+      burst: 10
+      contentType: application/vnd.kubernetes.protobuf
+      kubeconfig: /var/lib/kube-proxy/kubeconfig.conf
+      qps: 5
+    clusterCIDR: 192.168.0.0/16
+    configSyncPeriod: 15m0s
+    conntrack:
+      max: null
+      maxPerCore: 32768
+      min: 131072
+      tcpCloseWaitTimeout: 1h0m0s
+      tcpEstablishedTimeout: 24h0m0s
+    enableProfiling: false
+    featureGates: ""
+    healthzBindAddress: 0.0.0.0:10256
+    hostnameOverride: ""
+    iptables:
+      masqueradeAll: false
+      masqueradeBit: 14
+      minSyncPeriod: 0s
+      syncPeriod: 30s
+    ipvs:
+      minSyncPeriod: 0s
+      scheduler: ""
+      syncPeriod: 30s
+    metricsBindAddress: 127.0.0.1:10249
+    mode: ""
+    oomScoreAdj: -999
+    portRange: ""
+    resourceContainer: /kube-proxy
+    udpTimeoutMilliseconds: 250ms
+kubeletConfiguration: {}
+kubernetesVersion: v1.9.6
+networking:
+  dnsDomain: cluster.local
+  podSubnet: 192.168.0.0/16
+  serviceSubnet: 10.96.0.0/12
+nodeName: ip-172-31-93-180.ec2.internal
+token: 8d69af.cd3e1c58f6228dfc
+tokenTTL: 24h0m0s
+unifiedControlPlaneImage: ""
--- a/cmd/kubeadm/app/apis/kubeadm/v1alpha1/upgrade.go
+++ b/cmd/kubeadm/app/apis/kubeadm/v1alpha1/upgrade.go
@ -0,0 +1,135 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1alpha1
+
+import (
+	"bytes"
+	"errors"
+	"fmt"
+	"reflect"
+	"strconv"
+	"strings"
+
+	"github.com/json-iterator/go"
+	"github.com/ugorji/go/codec"
+	yaml "gopkg.in/yaml.v2"
+
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	runtime "k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/kubernetes/pkg/api/legacyscheme"
+)
+
+var json = jsoniter.ConfigCompatibleWithStandardLibrary
+
+type configMutationFunc func(map[string]interface{}) error
+
+// These migrations are a stop-gap until we get a properly-versioned configuration file for MasterConfiguration.
+// https://github.com/kubernetes/kubeadm/issues/750
+var migrations = map[string][]configMutationFunc{
+	"MasterConfiguration": {
+		proxyFeatureListToMap,
+	},
+}
+
+// Migrate takes a map representing a config file and an object to decode into.
+// The map is transformed into a format suitable for encoding into the supplied object, then serialised and decoded.
+func Migrate(in map[string]interface{}, obj runtime.Object) error {
+	kind := reflect.TypeOf(obj).Elem().Name()
+	migrationsForKind := migrations[kind]
+
+	for _, m := range migrationsForKind {
+		err := m(in)
+		if err != nil {
+			return err
+		}
+	}
+
+	// Use codec instead of encoding/json to handle map[interface{}]interface{}
+	handle := &codec.JsonHandle{}
+	buf := new(bytes.Buffer)
+	if err := codec.NewEncoder(buf, handle).Encode(in); err != nil {
+		return fmt.Errorf("couldn't json encode object: %v", err)
+	}
+
+	return runtime.DecodeInto(legacyscheme.Codecs.UniversalDecoder(), buf.Bytes(), obj)
+}
+
+func proxyFeatureListToMap(m map[string]interface{}) error {
+	featureGatePath := []string{"kubeProxy", "config", "featureGates"}
+
+	// If featureGatePath is already a map, we don't need to do anything.
+	_, _, err := unstructured.NestedMap(m, featureGatePath...)
+	if err == nil {
+		return nil
+	}
+
+	gates, _, err := unstructured.NestedString(m, featureGatePath...)
+	if err != nil {
+		return fmt.Errorf("couldn't get featureGates: %v", err)
+	}
+
+	gateMap := make(map[string]interface{})
+	for _, gate := range strings.Split(gates, ",") {
+		if gate == "" {
+			continue
+		}
+		parts := strings.SplitN(gate, "=", 2)
+		if len(parts) != 2 {
+			return fmt.Errorf("unparsable kubeproxy feature gate %q", gate)
+		}
+		val, err := strconv.ParseBool(parts[1])
+		if err != nil {
+			return fmt.Errorf("unparsable kubeproxy feature gate %q: %v", gate, err)
+		}
+		gateMap[parts[0]] = val
+	}
+
+	unstructured.SetNestedMap(m, gateMap, featureGatePath...)
+	return nil
+}
+
+// LoadYAML is a small wrapper around go-yaml that ensures all nested structs are map[string]interface{} instead of map[interface{}]interface{}.
+func LoadYAML(bytes []byte) (map[string]interface{}, error) {
+	var decoded map[interface{}]interface{}
+	if err := yaml.Unmarshal(bytes, &decoded); err != nil {
+		return map[string]interface{}{}, fmt.Errorf("couldn't unmarshal YAML: %v", err)
+	}
+
+	converted, ok := convert(decoded).(map[string]interface{})
+	if !ok {
+		return map[string]interface{}{}, errors.New("yaml is not a map")
+	}
+
+	return converted, nil
+}
+
+// https://stackoverflow.com/questions/40737122/convert-yaml-to-json-without-struct-golang
+func convert(i interface{}) interface{} {
+	switch x := i.(type) {
+	case map[interface{}]interface{}:
+		m2 := map[string]interface{}{}
+		for k, v := range x {
+			m2[k.(string)] = convert(v)
+		}
+		return m2
+	case []interface{}:
+		for i, v := range x {
+			x[i] = convert(v)
+		}
+	}
+	return i
+}
--- a/cmd/kubeadm/app/apis/kubeadm/v1alpha1/upgrade_test.go
+++ b/cmd/kubeadm/app/apis/kubeadm/v1alpha1/upgrade_test.go
@ -0,0 +1,137 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1alpha1
+
+import (
+	"io/ioutil"
+	"testing"
+
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+)
+
+const test196 = "testdata/kubeadm196.yaml"
+
+func TestUpgrade(t *testing.T) {
+	testYAML, err := ioutil.ReadFile(test196)
+	if err != nil {
+		t.Fatalf("couldn't read test data: %v", err)
+	}
+
+	decoded, err := LoadYAML(testYAML)
+	if err != nil {
+		t.Fatalf("couldn't unmarshal test yaml: %v", err)
+	}
+
+	var obj MasterConfiguration
+	if err := Migrate(decoded, &obj); err != nil {
+		t.Fatalf("couldn't decode migrated object: %v", err)
+	}
+}
+
+func TestProxyFeatureListToMap(t *testing.T) {
+
+	cases := []struct {
+		name         string
+		featureGates interface{}
+		expected     map[string]interface{}
+		shouldError  bool
+	}{
+		{
+			name:         "multiple features",
+			featureGates: "feature1=true,feature2=false",
+			expected: map[string]interface{}{
+				"feature1": true,
+				"feature2": false,
+			},
+		},
+		{
+			name:         "single feature",
+			featureGates: "feature1=true",
+			expected: map[string]interface{}{
+				"feature1": true,
+			},
+		},
+		{
+			name: "already a map",
+			featureGates: map[string]interface{}{
+				"feature1": true,
+			},
+			expected: map[string]interface{}{
+				"feature1": true,
+			},
+		},
+		{
+			name:         "single feature",
+			featureGates: "",
+			expected:     map[string]interface{}{},
+		},
+		{
+			name:         "malformed string",
+			featureGates: "test,",
+			shouldError:  true,
+		},
+	}
+
+	for _, testCase := range cases {
+		t.Run(testCase.name, func(t *testing.T) {
+
+			cfg := map[string]interface{}{
+				"kubeProxy": map[string]interface{}{
+					"config": map[string]interface{}{
+						"featureGates": testCase.featureGates,
+					},
+				},
+			}
+
+			err := proxyFeatureListToMap(cfg)
+			if testCase.shouldError {
+				if err == nil {
+					t.Error("expected error, got nil")
+				}
+				return
+			}
+
+			if err != nil {
+				t.Errorf("unexpected error: %v", err)
+			}
+
+			gates, ok, err := unstructured.NestedMap(cfg, "kubeProxy", "config", "featureGates")
+			if !ok {
+				t.Errorf("missing map keys in nested map")
+			}
+			if err != nil {
+				t.Errorf("unexpected error in map: %v", err)
+			}
+
+			if len(testCase.expected) != len(gates) {
+				t.Errorf("expected feature gate size %d, got %d", len(testCase.expected), len(gates))
+			}
+
+			for k, v := range testCase.expected {
+				gateVal, ok := gates[k]
+				if !ok {
+					t.Errorf("featureGates missing key %q", k)
+					continue
+				}
+
+				if v != gateVal {
+					t.Errorf("expected value %v, got %v", v, gateVal)
+				}
+			}
+		})
+	}
+}
--- a/cmd/kubeadm/app/phases/upgrade/BUILD
+++ b/cmd/kubeadm/app/phases/upgrade/BUILD
@ -44,7 +44,6 @@ go_library(
        "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
        "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
        "//vendor/k8s.io/apimachinery/pkg/labels:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
        "//vendor/k8s.io/apimachinery/pkg/util/errors:go_default_library",
        "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
        "//vendor/k8s.io/client-go/kubernetes:go_default_library",
--- a/cmd/kubeadm/app/phases/upgrade/configuration.go
+++ b/cmd/kubeadm/app/phases/upgrade/configuration.go
@ -23,7 +23,6 @@ import (

 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 	clientset "k8s.io/client-go/kubernetes"
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmapiext "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1"
@ -103,9 +102,14 @@ func bytesToValidatedMasterConfig(b []byte) (*kubeadmapiext.MasterConfiguration,
 	finalCfg := &kubeadmapiext.MasterConfiguration{}
 	internalcfg := &kubeadmapi.MasterConfiguration{}

-	if err := runtime.DecodeInto(legacyscheme.Codecs.UniversalDecoder(), b, cfg); err != nil {
+	decoded, err := kubeadmapiext.LoadYAML(b)
+	if err != nil {
 		return nil, fmt.Errorf("unable to decode config from bytes: %v", err)
 	}
+
+	if err := kubeadmapiext.Migrate(decoded, cfg); err != nil {
+		return nil, fmt.Errorf("unable to migrate config from previous version: %v", err)
+	}
 	// Default and convert to the internal version
 	legacyscheme.Scheme.Default(cfg)
 	legacyscheme.Scheme.Convert(cfg, internalcfg, nil)
--- a/cmd/kubeadm/app/phases/upgrade/policy.go
+++ b/cmd/kubeadm/app/phases/upgrade/policy.go
@ -116,6 +116,12 @@ func EnforceVersionPolicies(versionGetter VersionGetter, newK8sVersionStr string
 		}
 	}

+	if kubeadmVersion.Major() > newK8sVersion.Major() ||
+		kubeadmVersion.Minor() > newK8sVersion.Minor() {
+		skewErrors.Mandatory = append(skewErrors.Mandatory, fmt.Errorf("Kubeadm version %s can only be used to upgrade to Kubernetes versions %d.%d", kubeadmVersionStr, kubeadmVersion.Major(), kubeadmVersion.Minor()))
+
+	}
+
 	// Detect if the version is unstable and the user didn't allow that
 	if err = detectUnstableVersionError(newK8sVersion, newK8sVersionStr, allowExperimentalUpgrades, allowRCUpgrades); err != nil {
 		skewErrors.Skippable = append(skewErrors.Skippable, err)
--- a/cmd/kubeadm/app/phases/upgrade/policy_test.go
+++ b/cmd/kubeadm/app/phases/upgrade/policy_test.go
@ -24,13 +24,15 @@ import (

 func TestEnforceVersionPolicies(t *testing.T) {
 	tests := []struct {
+		name                        string
 		vg                          *fakeVersionGetter
 		expectedMandatoryErrs       int
 		expectedSkippableErrs       int
 		allowExperimental, allowRCs bool
 		newK8sVersion               string
 	}{
-		{ // everything ok
+		{
+			name: "minor upgrade",
 			vg: &fakeVersionGetter{
 				clusterVersion: "v1.9.3",
 				kubeletVersion: "v1.9.3",
@ -38,7 +40,8 @@ func TestEnforceVersionPolicies(t *testing.T) {
 			},
 			newK8sVersion: "v1.9.5",
 		},
-		{ // everything ok
+		{
+			name: "major upgrade",
 			vg: &fakeVersionGetter{
 				clusterVersion: "v1.9.3",
 				kubeletVersion: "v1.9.2",
@ -46,7 +49,8 @@ func TestEnforceVersionPolicies(t *testing.T) {
 			},
 			newK8sVersion: "v1.10.0",
 		},
-		{ // downgrades ok
+		{
+			name: "downgrade",
 			vg: &fakeVersionGetter{
 				clusterVersion: "v1.9.3",
 				kubeletVersion: "v1.9.3",
@ -54,7 +58,8 @@ func TestEnforceVersionPolicies(t *testing.T) {
 			},
 			newK8sVersion: "v1.9.2",
 		},
-		{ // upgrades without bumping the version number ok
+		{
+			name: "same version upgrade",
 			vg: &fakeVersionGetter{
 				clusterVersion: "v1.9.3",
 				kubeletVersion: "v1.9.3",
@ -62,16 +67,18 @@ func TestEnforceVersionPolicies(t *testing.T) {
 			},
 			newK8sVersion: "v1.9.3",
 		},
-		{ // new version must be higher than v1.9.0
+		{
+			name: "new version must be higher than v1.9.0",
 			vg: &fakeVersionGetter{
 				clusterVersion: "v1.9.3",
 				kubeletVersion: "v1.9.3",
 				kubeadmVersion: "v1.9.3",
 			},
 			newK8sVersion:         "v1.8.10",
-			expectedMandatoryErrs: 1, // version must be higher than v1.9.0
+			expectedMandatoryErrs: 2, // version must be higher than v1.9.0, can't upgrade old k8s with newer kubeadm
 		},
-		{ // upgrading two minor versions in one go is not supported
+		{
+			name: "upgrading two minor versions in one go is not supported",
 			vg: &fakeVersionGetter{
 				clusterVersion: "v1.9.3",
 				kubeletVersion: "v1.9.3",
@ -81,16 +88,18 @@ func TestEnforceVersionPolicies(t *testing.T) {
 			expectedMandatoryErrs: 1, // can't upgrade two minor versions
 			expectedSkippableErrs: 1, // kubelet <-> apiserver skew too large
 		},
-		{ // downgrading two minor versions in one go is not supported
+		{
+			name: "downgrading two minor versions in one go is not supported",
 			vg: &fakeVersionGetter{
 				clusterVersion: "v1.11.3",
 				kubeletVersion: "v1.11.3",
 				kubeadmVersion: "v1.11.0",
 			},
 			newK8sVersion:         "v1.9.3",
-			expectedMandatoryErrs: 1, // can't downgrade two minor versions
+			expectedMandatoryErrs: 2, // can't downgrade two minor versions, can't upgrade old k8s with newer kubeadm
 		},
-		{ // kubeadm version must be higher than the new kube version. However, patch version skews may be forced
+		{
+			name: "kubeadm version must be higher than the new kube version. However, patch version skews may be forced",
 			vg: &fakeVersionGetter{
 				clusterVersion: "v1.9.3",
 				kubeletVersion: "v1.9.3",
@ -99,7 +108,8 @@ func TestEnforceVersionPolicies(t *testing.T) {
 			newK8sVersion:         "v1.9.5",
 			expectedSkippableErrs: 1,
 		},
-		{ // kubeadm version must be higher than the new kube version. Trying to upgrade k8s to a higher minor version than kubeadm itself should never be supported
+		{
+			name: "kubeadm version must be higher than the new kube version. Trying to upgrade k8s to a higher minor version than kubeadm itself should never be supported",
 			vg: &fakeVersionGetter{
 				clusterVersion: "v1.9.3",
 				kubeletVersion: "v1.9.3",
@ -108,7 +118,8 @@ func TestEnforceVersionPolicies(t *testing.T) {
 			newK8sVersion:         "v1.10.0",
 			expectedMandatoryErrs: 1,
 		},
-		{ // the maximum skew between the cluster version and the kubelet versions should be one minor version. This may be forced through though.
+		{
+			name: "the maximum skew between the cluster version and the kubelet versions should be one minor version. This may be forced through though.",
 			vg: &fakeVersionGetter{
 				clusterVersion: "v1.9.3",
 				kubeletVersion: "v1.8.8",
@ -117,7 +128,8 @@ func TestEnforceVersionPolicies(t *testing.T) {
 			newK8sVersion:         "v1.10.0",
 			expectedSkippableErrs: 1,
 		},
-		{ // experimental upgrades supported if the flag is set
+		{
+			name: "experimental upgrades supported if the flag is set",
 			vg: &fakeVersionGetter{
 				clusterVersion: "v1.9.3",
 				kubeletVersion: "v1.9.3",
@ -126,7 +138,8 @@ func TestEnforceVersionPolicies(t *testing.T) {
 			newK8sVersion:     "v1.10.0-beta.1",
 			allowExperimental: true,
 		},
-		{ // release candidate upgrades supported if the flag is set
+		{
+			name: "release candidate upgrades supported if the flag is set",
 			vg: &fakeVersionGetter{
 				clusterVersion: "v1.9.3",
 				kubeletVersion: "v1.9.3",
@ -135,7 +148,8 @@ func TestEnforceVersionPolicies(t *testing.T) {
 			newK8sVersion: "v1.10.0-rc.1",
 			allowRCs:      true,
 		},
-		{ // release candidate upgrades supported if the flag is set
+		{
+			name: "release candidate upgrades supported if the flag is set",
 			vg: &fakeVersionGetter{
 				clusterVersion: "v1.9.3",
 				kubeletVersion: "v1.9.3",
@ -144,7 +158,8 @@ func TestEnforceVersionPolicies(t *testing.T) {
 			newK8sVersion:     "v1.10.0-rc.1",
 			allowExperimental: true,
 		},
-		{ // the user should not be able to upgrade to an experimental version if they haven't opted into that
+		{
+			name: "the user should not be able to upgrade to an experimental version if they haven't opted into that",
 			vg: &fakeVersionGetter{
 				clusterVersion: "v1.9.3",
 				kubeletVersion: "v1.9.3",
@ -154,7 +169,8 @@ func TestEnforceVersionPolicies(t *testing.T) {
 			allowRCs:              true,
 			expectedSkippableErrs: 1,
 		},
-		{ // the user should not be able to upgrade to an release candidate version if they haven't opted into that
+		{
+			name: "the user should not be able to upgrade to an release candidate version if they haven't opted into that",
 			vg: &fakeVersionGetter{
 				clusterVersion: "v1.9.3",
 				kubeletVersion: "v1.9.3",
@ -163,30 +179,42 @@ func TestEnforceVersionPolicies(t *testing.T) {
 			newK8sVersion:         "v1.10.0-rc.1",
 			expectedSkippableErrs: 1,
 		},
+		{
+			name: "the user can't use a newer minor version of kubeadm to upgrade an older version of kubeadm",
+			vg: &fakeVersionGetter{
+				clusterVersion: "v1.9.3",
+				kubeletVersion: "v1.9.3",
+				kubeadmVersion: "v1.10.0",
+			},
+			newK8sVersion:         "v1.9.6",
+			expectedMandatoryErrs: 1, // can't upgrade old k8s with newer kubeadm
+		},
 	}

 	for _, rt := range tests {
+		t.Run(rt.name, func(t *testing.T) {

-		newK8sVer, err := version.ParseSemantic(rt.newK8sVersion)
-		if err != nil {
-			t.Fatalf("couldn't parse version %s: %v", rt.newK8sVersion, err)
-		}
-
-		actualSkewErrs := EnforceVersionPolicies(rt.vg, rt.newK8sVersion, newK8sVer, rt.allowExperimental, rt.allowRCs)
-		if actualSkewErrs == nil {
-			// No errors were seen. Report unit test failure if we expected to see errors
-			if rt.expectedMandatoryErrs+rt.expectedSkippableErrs > 0 {
-				t.Errorf("failed TestEnforceVersionPolicies\n\texpected errors but got none")
+			newK8sVer, err := version.ParseSemantic(rt.newK8sVersion)
+			if err != nil {
+				t.Fatalf("couldn't parse version %s: %v", rt.newK8sVersion, err)
 			}
-			// Otherwise, just move on with the next test
-			continue
-		}

-		if len(actualSkewErrs.Skippable) != rt.expectedSkippableErrs {
-			t.Errorf("failed TestEnforceVersionPolicies\n\texpected skippable errors: %d\n\tgot skippable errors: %d %v", rt.expectedSkippableErrs, len(actualSkewErrs.Skippable), *rt.vg)
-		}
-		if len(actualSkewErrs.Mandatory) != rt.expectedMandatoryErrs {
-			t.Errorf("failed TestEnforceVersionPolicies\n\texpected mandatory errors: %d\n\tgot mandatory errors: %d %v", rt.expectedMandatoryErrs, len(actualSkewErrs.Mandatory), *rt.vg)
-		}
+			actualSkewErrs := EnforceVersionPolicies(rt.vg, rt.newK8sVersion, newK8sVer, rt.allowExperimental, rt.allowRCs)
+			if actualSkewErrs == nil {
+				// No errors were seen. Report unit test failure if we expected to see errors
+				if rt.expectedMandatoryErrs+rt.expectedSkippableErrs > 0 {
+					t.Errorf("failed TestEnforceVersionPolicies\n\texpected errors but got none")
+				}
+				// Otherwise, just move on with the next test
+				return
+			}
+
+			if len(actualSkewErrs.Skippable) != rt.expectedSkippableErrs {
+				t.Errorf("failed TestEnforceVersionPolicies\n\texpected skippable errors: %d\n\tgot skippable errors: %d %v", rt.expectedSkippableErrs, len(actualSkewErrs.Skippable), *rt.vg)
+			}
+			if len(actualSkewErrs.Mandatory) != rt.expectedMandatoryErrs {
+				t.Errorf("failed TestEnforceVersionPolicies\n\texpected mandatory errors: %d\n\tgot mandatory errors: %d %v", rt.expectedMandatoryErrs, len(actualSkewErrs.Mandatory), *rt.vg)
+			}
+		})
 	}
 }