From 339a9c1c1b6425f1d778621e9c854d36289e7746 Mon Sep 17 00:00:00 2001 From: Dmitry Rozhkov Date: Wed, 9 Jan 2019 11:50:58 +0200 Subject: [PATCH] kubeadm: unhide the logic for non-interactiveness Currently we maintain the state of the mode of interactiveness by updating flags.nonInteractiveMode even if the flag hasn't been set by the user. Since the computation of the mode is done only once it's easier and less error prone to calculate the mode in the function sessionIsInteractive() without mutating any flags. --- cmd/kubeadm/app/cmd/upgrade/apply.go | 15 +-- cmd/kubeadm/app/cmd/upgrade/apply_test.go | 132 +++++++--------------- 2 files changed, 47 insertions(+), 100 deletions(-) diff --git a/cmd/kubeadm/app/cmd/upgrade/apply.go b/cmd/kubeadm/app/cmd/upgrade/apply.go index 4a204d0495..5c791d3698 100644 --- a/cmd/kubeadm/app/cmd/upgrade/apply.go +++ b/cmd/kubeadm/app/cmd/upgrade/apply.go @@ -59,9 +59,9 @@ type applyFlags struct { imagePullTimeout time.Duration } -// SessionIsInteractive returns true if the session is of an interactive type (the default, can be opted out of with -y, -f or --dry-run) -func (f *applyFlags) SessionIsInteractive() bool { - return !f.nonInteractiveMode +// sessionIsInteractive returns true if the session is of an interactive type (the default, can be opted out of with -y, -f or --dry-run) +func (f *applyFlags) sessionIsInteractive() bool { + return !(f.nonInteractiveMode || f.dryRun || f.force) } // NewCmdApply returns the cobra command for `kubeadm upgrade apply` @@ -185,8 +185,8 @@ func runApply(flags *applyFlags) error { return errors.Wrap(err, "[upgrade/version] FATAL") } - // If the current session is interactive, ask the user whether they really want to upgrade - if flags.SessionIsInteractive() { + // If the current session is interactive, ask the user whether they really want to upgrade. + if flags.sessionIsInteractive() { if err := InteractivelyConfirmUpgrade("Are you sure you want to proceed with the upgrade?"); err != nil { return err } @@ -233,11 +233,6 @@ func runApply(flags *applyFlags) error { // SetImplicitFlags handles dynamically defaulting flags based on each other's value func SetImplicitFlags(flags *applyFlags) error { - // If we are in dry-run or force mode; we should automatically execute this command non-interactively - if flags.dryRun || flags.force { - flags.nonInteractiveMode = true - } - if len(flags.newK8sVersionStr) == 0 { return errors.New("version string can't be empty") } diff --git a/cmd/kubeadm/app/cmd/upgrade/apply_test.go b/cmd/kubeadm/app/cmd/upgrade/apply_test.go index 2a820efdcf..ec02306fd4 100644 --- a/cmd/kubeadm/app/cmd/upgrade/apply_test.go +++ b/cmd/kubeadm/app/cmd/upgrade/apply_test.go @@ -33,96 +33,6 @@ func TestSetImplicitFlags(t *testing.T) { expectedFlags applyFlags errExpected bool }{ - { - name: "if not dryRun or force is set; the nonInteractiveMode field should not be touched", - flags: &applyFlags{ - newK8sVersionStr: "v1.8.0", - dryRun: false, - force: false, - nonInteractiveMode: false, - }, - expectedFlags: applyFlags{ - newK8sVersionStr: "v1.8.0", - dryRun: false, - force: false, - nonInteractiveMode: false, - }, - }, - { - name: "if not dryRun or force is set; the nonInteractiveMode field should not be touched", - flags: &applyFlags{ - newK8sVersionStr: "v1.8.0", - dryRun: false, - force: false, - nonInteractiveMode: true, - }, - expectedFlags: applyFlags{ - newK8sVersionStr: "v1.8.0", - dryRun: false, - force: false, - nonInteractiveMode: true, - }, - }, - { - name: "if dryRun or force is set; the nonInteractiveMode field should be set to true", - flags: &applyFlags{ - newK8sVersionStr: "v1.8.0", - dryRun: true, - force: false, - nonInteractiveMode: false, - }, - expectedFlags: applyFlags{ - newK8sVersionStr: "v1.8.0", - dryRun: true, - force: false, - nonInteractiveMode: true, - }, - }, - { - name: "if dryRun or force is set; the nonInteractiveMode field should be set to true", - flags: &applyFlags{ - newK8sVersionStr: "v1.8.0", - dryRun: false, - force: true, - nonInteractiveMode: false, - }, - expectedFlags: applyFlags{ - newK8sVersionStr: "v1.8.0", - dryRun: false, - force: true, - nonInteractiveMode: true, - }, - }, - { - name: "if dryRun or force is set; the nonInteractiveMode field should be set to true", - flags: &applyFlags{ - newK8sVersionStr: "v1.8.0", - dryRun: true, - force: true, - nonInteractiveMode: false, - }, - expectedFlags: applyFlags{ - newK8sVersionStr: "v1.8.0", - dryRun: true, - force: true, - nonInteractiveMode: true, - }, - }, - { - name: "if dryRun or force is set; the nonInteractiveMode field should be set to true", - flags: &applyFlags{ - newK8sVersionStr: "v1.8.0", - dryRun: true, - force: true, - nonInteractiveMode: true, - }, - expectedFlags: applyFlags{ - newK8sVersionStr: "v1.8.0", - dryRun: true, - force: true, - nonInteractiveMode: true, - }, - }, { name: "if the new version is empty; it should error out", flags: &applyFlags{ @@ -161,6 +71,48 @@ func TestSetImplicitFlags(t *testing.T) { } } +func TestSessionIsInteractive(t *testing.T) { + var tcases = []struct { + name string + flags *applyFlags + expected bool + }{ + { + name: "Explicitly non-interactive", + flags: &applyFlags{ + nonInteractiveMode: true, + }, + expected: false, + }, + { + name: "Implicitly non-interactive since --dryRun is used", + flags: &applyFlags{ + dryRun: true, + }, + expected: false, + }, + { + name: "Implicitly non-interactive since --force is used", + flags: &applyFlags{ + force: true, + }, + expected: false, + }, + { + name: "Interactive session", + flags: &applyFlags{}, + expected: true, + }, + } + for _, tt := range tcases { + t.Run(tt.name, func(t *testing.T) { + if tt.flags.sessionIsInteractive() != tt.expected { + t.Error("unexpected result") + } + }) + } +} + func TestGetPathManagerForUpgrade(t *testing.T) { haEtcd := &kubeadmapi.InitConfiguration{