From 5adee740008a65907bfd8d6e3c3275e24f23f27d Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Thu, 17 May 2018 20:39:21 +0300 Subject: [PATCH 1/2] kubeadm-upgrade: small improvements to diff 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. --- cmd/kubeadm/app/cmd/upgrade/diff.go | 7 +++++-- cmd/kubeadm/app/cmd/upgrade/upgrade.go | 2 ++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/cmd/kubeadm/app/cmd/upgrade/diff.go b/cmd/kubeadm/app/cmd/upgrade/diff.go index 957b05f72ee..cf87756c678 100644 --- a/cmd/kubeadm/app/cmd/upgrade/diff.go +++ b/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 } diff --git a/cmd/kubeadm/app/cmd/upgrade/upgrade.go b/cmd/kubeadm/app/cmd/upgrade/upgrade.go index 1993e67a22e..a7928aa01bc 100644 --- a/cmd/kubeadm/app/cmd/upgrade/upgrade.go +++ b/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{ From f93d064e931bd85b5a39a5c7d9b526adca32f56f Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Thu, 17 May 2018 20:45:17 +0300 Subject: [PATCH 2/2] kubeadm-upgrade: add unit tests for the diff command 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. A couple of test files are added in: cmd/kubeadm/app/cmd/upgrade/testdata/ --- cmd/kubeadm/app/cmd/upgrade/BUILD | 2 + cmd/kubeadm/app/cmd/upgrade/diff_test.go | 93 +++++++++++++++++++ .../upgrade/testdata/diff_dummy_manifest.yaml | 1 + .../upgrade/testdata/diff_master_config.yaml | 3 + 4 files changed, 99 insertions(+) create mode 100644 cmd/kubeadm/app/cmd/upgrade/diff_test.go create mode 100644 cmd/kubeadm/app/cmd/upgrade/testdata/diff_dummy_manifest.yaml create mode 100644 cmd/kubeadm/app/cmd/upgrade/testdata/diff_master_config.yaml diff --git a/cmd/kubeadm/app/cmd/upgrade/BUILD b/cmd/kubeadm/app/cmd/upgrade/BUILD index f22be4b4435..201c49acfaf 100644 --- a/cmd/kubeadm/app/cmd/upgrade/BUILD +++ b/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", diff --git a/cmd/kubeadm/app/cmd/upgrade/diff_test.go b/cmd/kubeadm/app/cmd/upgrade/diff_test.go new file mode 100644 index 00000000000..2c7e2834399 --- /dev/null +++ b/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) + } + }) + } +} diff --git a/cmd/kubeadm/app/cmd/upgrade/testdata/diff_dummy_manifest.yaml b/cmd/kubeadm/app/cmd/upgrade/testdata/diff_dummy_manifest.yaml new file mode 100644 index 00000000000..b65b6743bf6 --- /dev/null +++ b/cmd/kubeadm/app/cmd/upgrade/testdata/diff_dummy_manifest.yaml @@ -0,0 +1 @@ +some-empty-file-to-diff diff --git a/cmd/kubeadm/app/cmd/upgrade/testdata/diff_master_config.yaml b/cmd/kubeadm/app/cmd/upgrade/testdata/diff_master_config.yaml new file mode 100644 index 00000000000..7bfbed6015a --- /dev/null +++ b/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