Merge pull request #63991 from neolit123/test-upgrade-diff

Automatic merge from submit-queue (batch tested with PRs 62025, 63851, 64077, 63967, 63991). 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>.

Add unit tests to `kubeadm upgrade diff` and small improvements

**What this PR does / why we need it**:

has a couple of commits:

I.
```
1) Store the io.Writer and pass it to sub-commands in upgrade.go
2) Check if the manifest path is an empty string in diff.go:runDiff()
3) Use the io.Writer that upgrade.go defines instead of writing to
os.Stdout directly.
```
II.
```
Add the file diff_test.go, which has a single test:
  TestRunDiff

The test covers most error cases for the runDiff() function,
and also performs a valid diff.
```


**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 kubernetes/kubeadm#826

**Special notes for your reviewer**:
@liztio @luxas 

**Release note**:

```release-note
NONE
```

cmd/kubeadm/app/cmd/upgrade/BUILD

@@ -44,8 +44,10 @@ go_test(
     srcs = [
         "apply_test.go",
         "common_test.go",
+        "diff_test.go",
         "plan_test.go",
     ],
+    data = glob(["testdata/**"]),
     embed = [":go_default_library"],
     deps = [
         "//cmd/kubeadm/app/apis/kubeadm:go_default_library",

cmd/kubeadm/app/cmd/upgrade/diff.go

@@ -17,8 +17,8 @@ limitations under the License.
 package upgrade
 
 import (
+	"fmt"
 	"io/ioutil"
-	"os"
 
 	"github.com/golang/glog"
 	"github.com/pmezard/go-difflib/difflib"
@@ -119,6 +119,9 @@ func runDiff(flags *diffFlags, args []string) error {
 		if err != nil {
 			return err
 		}
+		if path == "" {
+			return fmt.Errorf("empty manifest path")
+		}
 		existingManifest, err := ioutil.ReadFile(path)
 		if err != nil {
 			return err
@@ -133,7 +136,7 @@ func runDiff(flags *diffFlags, args []string) error {
 			Context:  flags.contextLines,
 		}
 
-		difflib.WriteUnifiedDiff(os.Stdout, diff)
+		difflib.WriteUnifiedDiff(flags.parent.out, diff)
 	}
 	return nil
 }

cmd/kubeadm/app/cmd/upgrade/diff_test.go

@@ -0,0 +1,93 @@
+/*
+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 upgrade
+
+import (
+	"io/ioutil"
+	"testing"
+)
+
+const (
+	testUpgradeDiffConfig   = `testdata/diff_master_config.yaml`
+	testUpgradeDiffManifest = `testdata/diff_dummy_manifest.yaml`
+)
+
+func TestRunDiff(t *testing.T) {
+	parentFlags := &cmdUpgradeFlags{
+		cfgPath: "",
+		out:     ioutil.Discard,
+	}
+	flags := &diffFlags{
+		parent: parentFlags,
+	}
+
+	testCases := []struct {
+		name            string
+		args            []string
+		setManifestPath bool
+		manifestPath    string
+		cfgPath         string
+		expectedError   bool
+	}{
+		{
+			name:            "valid: run diff on valid manifest path",
+			cfgPath:         testUpgradeDiffConfig,
+			setManifestPath: true,
+			manifestPath:    testUpgradeDiffManifest,
+			expectedError:   false,
+		},
+		{
+			name:          "invalid: missing config file",
+			cfgPath:       "missing-path-to-a-config",
+			expectedError: true,
+		},
+		{
+			name:            "invalid: valid config but empty manifest path",
+			cfgPath:         testUpgradeDiffConfig,
+			setManifestPath: true,
+			manifestPath:    "",
+			expectedError:   true,
+		},
+		{
+			name:            "invalid: valid config but bad manifest path",
+			cfgPath:         testUpgradeDiffConfig,
+			setManifestPath: true,
+			manifestPath:    "bad-path",
+			expectedError:   true,
+		},
+		{
+			name:          "invalid: badly formatted version as argument",
+			cfgPath:       testUpgradeDiffConfig,
+			args:          []string{"bad-version"},
+			expectedError: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			parentFlags.cfgPath = tc.cfgPath
+			if tc.setManifestPath {
+				flags.apiServerManifestPath = tc.manifestPath
+				flags.controllerManagerManifestPath = tc.manifestPath
+				flags.schedulerManifestPath = tc.manifestPath
+			}
+			if err := runDiff(flags, tc.args); (err != nil) != tc.expectedError {
+				t.Fatalf("expected error: %v, saw: %v, error: %v", tc.expectedError, (err != nil), err)
+			}
+		})
+	}
+}

cmd/kubeadm/app/cmd/upgrade/testdata/diff_dummy_manifest.yaml

@@ -0,0 +1 @@
+some-empty-file-to-diff

cmd/kubeadm/app/cmd/upgrade/testdata/diff_master_config.yaml

@@ -0,0 +1,3 @@
+apiVersion: kubeadm.k8s.io/v1alpha1
+kind: MasterConfiguration
+kubernetesVersion: 1.11.0

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

@@ -37,6 +37,7 @@ type cmdUpgradeFlags struct {
 	skipPreFlight             bool
 	ignorePreflightErrors     []string
 	ignorePreflightErrorsSet  sets.String
+	out                       io.Writer
 }
 
 // NewCmdUpgrade returns the cobra command for `kubeadm upgrade`
@@ -50,6 +51,7 @@ func NewCmdUpgrade(out io.Writer) *cobra.Command {
 		printConfig:               false,
 		skipPreFlight:             false,
 		ignorePreflightErrorsSet:  sets.NewString(),
+		out: out,
 	}
 
 	cmd := &cobra.Command{