Merge pull request #114080 from chendave/fix_testcases

kubeadm: fix invalid testcase for `enforceRequirements`

cmd/kubeadm/app/cmd/upgrade/apply.go

@@ -105,7 +105,7 @@ func runApply(flags *applyFlags, args []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).Infoln("[upgrade/apply] verifying health of cluster")
 	klog.V(1).Infoln("[upgrade/apply] retrieving configuration from cluster")
-	client, versionGetter, cfg, err := enforceRequirements(flags.applyPlanFlags, args, flags.dryRun, true, &output.TextPrinter{})
+	client, versionGetter, cfg, err := enforceRequirements(flags.applyPlanFlags, args, flags.dryRun, true, &output.TextPrinter{}, loadConfig)
 	if err != nil {
 		return err
 	}

cmd/kubeadm/app/cmd/upgrade/common.go

@@ -119,8 +119,10 @@ func loadConfig(cfgPath string, client clientset.Interface, skipComponentConfigs
 	return initCfg, false, nil
 }
 
+type LoadConfigFunc func(cfgPath string, client clientset.Interface, skipComponentConfigs bool, printer output.Printer) (*kubeadmapi.InitConfiguration, bool, error)
+
 // enforceRequirements verifies that it's okay to upgrade and then returns the variables needed for the rest of the procedure
-func enforceRequirements(flags *applyPlanFlags, args []string, dryRun bool, upgradeApply bool, printer output.Printer) (clientset.Interface, upgrade.VersionGetter, *kubeadmapi.InitConfiguration, error) {
+func enforceRequirements(flags *applyPlanFlags, args []string, dryRun bool, upgradeApply bool, printer output.Printer, loadConfig LoadConfigFunc) (clientset.Interface, upgrade.VersionGetter, *kubeadmapi.InitConfiguration, error) {
 	client, err := getClient(flags.kubeConfigPath, dryRun)
 	if err != nil {
 		return nil, nil, nil, errors.Wrapf(err, "couldn't create a Kubernetes client from file %q", flags.kubeConfigPath)

cmd/kubeadm/app/cmd/upgrade/common_test.go

@@ -19,50 +19,100 @@ package upgrade
 import (
 	"bytes"
 	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
 	"testing"
 
+	clientset "k8s.io/client-go/kubernetes"
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta3"
 	"k8s.io/kubernetes/cmd/kubeadm/app/util/output"
+	testutil "k8s.io/kubernetes/cmd/kubeadm/test"
 )
 
+const testConfigToken = `apiVersion: v1
+clusters:
+- cluster:
+    certificate-authority-data:
+    server: localhost:8000
+  name: prod
+contexts:
+- context:
+    cluster: prod
+    namespace: default
+    user: default-service-account
+  name: default
+current-context: default
+kind: Config
+preferences: {}
+users:
+- name: kubernetes-admin
+  user:
+    client-certificate-data:
+`
+
+func fakeLoadConfig(cfgPath string, client clientset.Interface, skipComponentConfigs bool, printer output.Printer) (*kubeadmapi.InitConfiguration, bool, error) {
+	return &kubeadmapi.InitConfiguration{}, false, nil
+}
+
 func TestEnforceRequirements(t *testing.T) {
+	tmpDir := testutil.SetupTempDir(t)
+	defer os.RemoveAll(tmpDir)
+
+	fullPath := filepath.Join(tmpDir, "test-config-file")
+	f, err := os.Create(fullPath)
+	if err != nil {
+		t.Errorf("Unable to create test file %q: %v", fullPath, err)
+	}
+	defer f.Close()
+
+	if _, err = f.WriteString(testConfigToken); err != nil {
+		t.Errorf("Unable to write test file %q: %v", fullPath, err)
+	}
+
 	tcases := []struct {
 		name          string
 		newK8sVersion string
 		dryRun        bool
 		flags         applyPlanFlags
-		expectedErr   bool
+		expectedErr   string
 	}{
 		{
-			name:        "Fail pre-flight check",
+			name: "Fail pre-flight check",
-			expectedErr: true,
+			flags: applyPlanFlags{
+				kubeConfigPath: fullPath,
+			},
+			expectedErr: "ERROR CoreDNSUnsupportedPlugins",
 		},
 		{
-			name: "Bogus preflight check disabled when also 'all' is specified",
+			name: "Bogus preflight check specify all with individual check",
 			flags: applyPlanFlags{
 				ignorePreflightErrors: []string{"bogusvalue", "all"},
+				kubeConfigPath:        fullPath,
 			},
-			expectedErr: true,
+			expectedErr: "don't specify individual checks if 'all' is used",
 		},
 		{
 			name: "Fail to create client",
 			flags: applyPlanFlags{
 				ignorePreflightErrors: []string{"all"},
 			},
-			expectedErr: true,
+			expectedErr: "couldn't create a Kubernetes client from file",
 		},
 	}
 	for _, tt := range tcases {
 		t.Run(tt.name, func(t *testing.T) {
-			_, _, _, err := enforceRequirements(&tt.flags, nil, tt.dryRun, false, &output.TextPrinter{})
+			_, _, _, err := enforceRequirements(&tt.flags, nil, tt.dryRun, false, &output.TextPrinter{}, fakeLoadConfig)
 
-			if err == nil && tt.expectedErr {
+			if err == nil && len(tt.expectedErr) != 0 {
 				t.Error("Expected error, but got success")
 			}
-			if err != nil && !tt.expectedErr {
+
-				t.Errorf("Unexpected error: %+v", err)
+			if err != nil && !strings.Contains(err.Error(), tt.expectedErr) {
+				t.Fatalf("enforceRequirements returned unexpected error, expected: %s, got %v", tt.expectedErr, err)
 			}
+
 		})
 	}
 }

cmd/kubeadm/app/cmd/upgrade/plan.go

@@ -250,7 +250,7 @@ func runPlan(flags *planFlags, args []string, printer output.Printer) 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).Infoln("[upgrade/plan] verifying health of cluster")
 	klog.V(1).Infoln("[upgrade/plan] retrieving configuration from cluster")
-	client, versionGetter, cfg, err := enforceRequirements(flags.applyPlanFlags, args, false, false, printer)
+	client, versionGetter, cfg, err := enforceRequirements(flags.applyPlanFlags, args, false, false, printer, loadConfig)
 	if err != nil {
 		return err
 	}