Merge pull request #74131 from rojkov/kubeadm-refactor-SessionIsInteractive

kubeadm: unhide the logic for non-interactiveness

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)
+// 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 {
+func (f *applyFlags) sessionIsInteractive() bool {
-	return !f.nonInteractiveMode
+	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 the current session is interactive, ask the user whether they really want to upgrade.
-	if flags.SessionIsInteractive() {
+	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")
 	}

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{