Repository: kubernetes/kops (go)

Issue:
All instancegroups must fail before kops exits on rolling update
/kind bug

**1. What `kops` version are you running? The command `kops version`, will display
 this information.**

1.20.3

**2. What Kubernetes version are you running? `kubectl version` will print the
 version if a cluster is running or provide the Kubernetes version specified as
 a `kops` flag.**

1.20.15

**3. What cloud provider are you using?**

AWS

**4. What commands did you run?  What is the simplest way to reproduce this issue?**

```
$ kops rolling-update cluster my-cluster.io --state s3://plaid-kops --master-interval 5m0s --node-interval 5m0s --post-drain-delay 5s --validation-timeout 15m0s --yes
```

**5. What happened after the commands executed?**

The rolling update went appropriately! But, we use an AWS IAM role in order to control permissions to run kops commands. Some of the clusters are rather large, so due to https://github.com/kubernetes/kops/issues/5130 we end up waiting for kops to fail with a specific error, refresh the role and then re-run the command.

When this happens we see the following logs:

<details>

```
[2022-08-24T16:07:58.971Z] E0824 16:07:53.703940     321 instancegroups.go:457] Cluster did not validate within 15m0s

[2022-08-24T16:07:58.971Z] I0824 16:07:53.703951     321 instancegroups.go:452] Validating the cluster.

[2022-08-24T16:13:36.722Z] I0824 16:13:22.259215     321 instancegroups.go:498] Cluster did not validate, will retry in "30s": unable to find autoscale groups: error listing autoscaling cluster tags: ExpiredToken: The security token included in the request is expired

[2022-08-24T16:13:36.722Z] 	status code: 403, request id: 72e2c489-ec7c-4e80-98dc-cda58e3804ba.

[2022-08-24T16:19:00.195Z] I0824 16:18:57.924309     321 instancegroups.go:498] Cluster did not validate, will retry in "30s": unable to find autoscale groups: error listing autoscaling cluster tags: ExpiredToken: The security token included in the request is expired

[2022-08-24T16:19:00.195Z] 	status code: 403, request id: 62df166c-b6d2-47d0-9967-db488cbe5b76.

[2022-08-24T16:24:54.070Z] I0824 16:24:53.177895     321 instancegroups.go:495] Cluster did not validate within deadline: unable to find autoscale groups: error listing autoscaling cluster tags: ExpiredToken: The security token included in the request is expired

[2022-08-24T16:24:54.070Z] 	status code: 403, request id: abef1974-1752-42e8-82c0-c5cfe6a8b1d2.

[2022-08-24T16:24:54.070Z] E0824 16:24:53.177928     321 instancegroups.go:457] Cluster did not validate within 15m0s

[2022-08-24T16:24:54.070Z] I0824 16:24:53.177945     321 instancegroups.go:452] Validating the cluster.

[2022-08-24T16:32:17.885Z] I0824 16:32:03.178143     321 instancegroups.go:498] Cluster did not validate, will retry in "30s": unable to find autoscale groups: error listing autoscaling cluster tags: ExpiredToken: The security token included in the request is expired

[2022-08-24T16:32:17.885Z] 	status code: 403, request id: ab436289-abec-4c19-8a56-1f68daf6ffe3.

[2022-08-24T16:37:41.710Z] I0824 16:37:40.916761     321 instancegroups.go:498] Cluster did not validate, will retry in "30s": unable to find autoscale groups: error listing autoscaling cluster tags: ExpiredToken: The security token included in the request is expired

[2022-08-24T16:37:41.710Z] 	status code: 403, request id: 4068577b-0255-4bf8-a3fc-d6289d2db07e.

[2022-08-24T16:44:04.955Z] I0824 16:43:57.587224     321 instancegroups.go:495] Cluster did not validate within deadline: unable to find autoscale groups: error listing autoscaling cluster tags: ExpiredToken: The security token included in the request is expired

[2022-08-24T16:44:04.955Z] 	status code: 403, request id: 6d00c28e-f560-4923-985e-c89e0ee45cbc.

[2022-08-24T16:44:04.955Z] E0824 16:43:57.587257     321 instancegroups.go:457] Cluster did not validate within 15m0s

[2022-08-24T16:44:04.955Z] I0824 16:43:57.587273     321 instancegroups.go:452] Validating the cluster.

[2022-08-24T16:49:30.646Z] I0824 16:49:17.122051     321 instancegroups.go:498] Cluster did not validate, will retry in "30s": unable to find autoscale groups: error listing autoscaling cluster tags: ExpiredToken: The security token included in the request is expired

[2022-08-24T16:49:30.646Z] 	status code: 403, request id: 71f5d700-f207-4a77-a16d-5d66d4639967.

[2022-08-24T16:55:39.198Z] I0824 16:55:28.310586     321 instancegroups.go:498] Cluster did not validate, will retry in "30s": unable to find autoscale groups: error listing autoscaling cluster tags: ExpiredToken: The security token included in the request is expired

[2022-08-24T16:55:39.198Z] 	status code: 403, request id: d47edcf4-60a9-4058-b003-93e75d567763.

[2022-08-24T17:02:48.991Z] I0824 17:02:40.994726     321 instancegroups.go:495] Cluster did not validate within deadline: unable to find autoscale groups: error listing autoscaling cluster tags: ExpiredToken: The security token included in the request is expired

[2022-08-24T17:02:48.991Z] 	status code: 403, request id: 66c94c79-b5df-4fa6-9487-b36c722b3e64.

[2022-08-24T17:02:48.991Z] E0824 17:02:40.994764     321 instancegroups.go:457] Cluster did not validate within 15m0s

[2022-08-24T17:02:48.991Z] I0824 17:02:40.994780     321 instancegroups.go:452] Validating the cluster.

[2022-08-24T17:08:42.381Z] I0824 17:08:29.814311     321 instancegroups.go:498] Cluster did not validate, will retry in "30s": unable to find autoscale groups: error listing autoscaling cluster tags: ExpiredToken: The security token included in the request is expired

[2022-08-24T17:08:42.381Z] 	status code: 403, request id: 53a4ac96-10fc-4a45-969d-df6e77149d39.
```
</details>

What is happening is that each instance group (on some clusters we have quite a few of those) needs to fail before the validation command can completely exit: https://github.com/kubernetes/kops/blob/v1.20.3/pkg/instancegroups/rollingupdate.go#L175-L181

So (assuming we roll an entire cluster at once), if our validation timeout is 15 minutes, and we have X instancegroups, we can expect to take 15 minutes * X amount of time to fail the command. At a minimum, with 3 masters and 1 instancegroup for nodes this ends up being an hour. Some of our clusters have 5 masters and 6 instancegroups, so we end up waiting close to 3 hours for a failure.

**6. What did you expect to happen?**

I expected that kops would fail out early when an instancegroup could not be rolled. That way, the rolling update can only fail for as long as the validation timeout (a maximum of 15 minutes instead of hours).

**7. Please provide your cluster manifest. Execute
  `kops get --name my.example.com -o yaml` to display your cluster manifest.
  You may want to remove your cluster name and other sensitive information.**

Happy to provide, but I don't think this is actually relevant.

**8. Please run the commands with most verbose logging by adding the `-v 10` flag.
  Paste the logs into this report, or in a gist and provide the gist link here.**

Please see the logs above!

**9. Anything else do we need to know?**

Happy to PR this and have the change hidden behind a flag so that existing behavior is not changed.

Pull request:
Exit rolling updates when encountering specific errors
Fixes #14176 

Add a new error to `instancegroups` which is returned when cluster validation times out. Then, check this error during cluster node and apiserver instancegroup rolls. If the cluster fails to be validated within the timeout then it is unlikely to succeed on subsequent node rolls, so the preference is to exit the cluster roll (to allow operators to fix the cluster validation issue).

Patch:
diff --git a/docs/releases/1.26-NOTES.md b/docs/releases/1.26-NOTES.md
index 31e1b86e8a..5bc1c6b2af 100644
--- a/docs/releases/1.26-NOTES.md
+++ b/docs/releases/1.26-NOTES.md
@@ -13,6 +13,12 @@ with "control-plane-". The names of groups for existing clusters are unchanged.
 
 * The channels CLI that kOps use to manage addons is now bundled with the kOps binary. These commands are useful for addon diagnostics and troubleshooting. For example, to list installed addons, run `kops toolbox addons get addons`.
 
+* Since kOps 1.24, by default during rolling updates, kOps will time out after
+spending 15 minutes on an InstanceGroup (instead of hanging indefinitely on
+eviction errors), proceeding to the next InstanceGroup after timing out.
+As of kOps 1.26, rolling updates will not proceed if a cluster validation
+error is encountered while updating an InstanceGroup.
+
 ## AWS
 
 * Bastions are now fronted by a Network Load Balancer.
@@ -69,4 +75,4 @@ CNIs, use the "cni" networking option instead.
 # Help Wanted
 
 * kOps needs maintainers for Canal, Flannel, and Kube-Router, to keep versions up to date and move the integration from experimental to stable.
-If no volunteers step up by the time kOps 1.27 is released, support will be phased out.
\ No newline at end of file
+If no volunteers step up by the time kOps 1.27 is released, support will be phased out.
diff --git a/pkg/instancegroups/instancegroups.go b/pkg/instancegroups/instancegroups.go
index e9226db258..231b633ee1 100644
--- a/pkg/instancegroups/instancegroups.go
+++ b/pkg/instancegroups/instancegroups.go
@@ -43,6 +43,30 @@ import (
 
 const rollingUpdateTaintKey = "kops.k8s.io/scheduled-for-update"
 
+// ValidationTimeoutError represents an error that occurs when
+// the cluster fails to validate within the designated timeout.
+type ValidationTimeoutError struct {
+	operation string
+	err       error
+}
+
+func (v *ValidationTimeoutError) Error() string {
+	return fmt.Sprintf("error validating cluster%s: %s", v.operation, v.err.Error())
+}
+
+func (v *ValidationTimeoutError) Unwrap() error {
+	return v.err
+}
+
+// Is checks that a given error is a ValidationTimeoutError.
+func (v *ValidationTimeoutError) Is(err error) bool {
+	// Currently all validation timeout errors are equivalent
+	// If you wish to differentiate, please update the instances of `errors.Is` that check
+	// this error to take that into account
+	_, ok := err.(*ValidationTimeoutError)
+	return ok
+}
+
 // promptInteractive asks the user to continue, mostly copied from vendor/google.golang.org/api/examples/gmail.go.
 func promptInteractive(upgradedHostID, upgradedHostName string) (stopPrompting bool, err error) {
 	stopPrompting = false
@@ -480,7 +504,10 @@ func (c *RollingUpdateCluster) maybeValidate(operation string, validateCount int
 
 			if c.FailOnValidate {
 				klog.Errorf("Cluster did not validate within %s", c.ValidationTimeout)
-				return fmt.Errorf("error validating cluster%s: %v", operation, err)
+				return &ValidationTimeoutError{
+					operation: operation,
+					err:       err,
+				}
 			}
 
 			klog.Warningf("Cluster validation failed%s, proceeding since fail-on-validate is set to false: %v", operation, err)
diff --git a/pkg/instancegroups/rollingupdate.go b/pkg/instancegroups/rollingupdate.go
index d03484f27b..f361d3db2d 100644
--- a/pkg/instancegroups/rollingupdate.go
+++ b/pkg/instancegroups/rollingupdate.go
@@ -18,6 +18,7 @@ package instancegroups
 
 import (
 	"context"
+	stderrors "errors"
 	"fmt"
 	"sort"
 	"sync"
@@ -191,7 +192,10 @@ func (c *RollingUpdateCluster) RollingUpdate(groups map[string]*cloudinstances.C
 			if err != nil {
 				klog.Errorf("failed to roll InstanceGroup %q: %v", k, err)
 			}
-			// TODO: Bail on error?
+
+			if isExitableError(err) {
+				return err
+			}
 		}
 	}
 
@@ -213,7 +217,10 @@ func (c *RollingUpdateCluster) RollingUpdate(groups map[string]*cloudinstances.C
 			if err != nil {
 				klog.Errorf("failed to roll InstanceGroup %q: %v", k, err)
 			}
-			// TODO: Bail on error?
+
+			if isExitableError(err) {
+				return err
+			}
 		}
 	}
 
@@ -236,3 +243,13 @@ func sortGroups(groupMap map[string]*cloudinstances.CloudInstanceGroup) []string
 	sort.Strings(groups)
 	return groups
 }
+
+// isExitableError inspects an error to determine if the error is
+// fatal enough that the rolling update cannot continue.
+//
+// For example, if a cluster is unable to be validated by the deadline, then it
+// is unlikely that it will validate on the next instance roll, so an early exit as a
+// warning to the user is more appropriate.
+func isExitableError(err error) bool {
+	return stderrors.Is(err, &ValidationTimeoutError{})
+}


Test patch:
diff --git a/pkg/instancegroups/rollingupdate_test.go b/pkg/instancegroups/rollingupdate_test.go
index 8c3383b12f..767aca626b 100644
--- a/pkg/instancegroups/rollingupdate_test.go
+++ b/pkg/instancegroups/rollingupdate_test.go
@@ -562,6 +562,31 @@ func TestRollingUpdateValidationErrorInstanceGroupNil(t *testing.T) {
 	assertGroupInstanceCount(t, cloud, "bastion-1", 1)
 }
 
+func TestRollingUpdateValidationErrorInstanceGroupExitableError(t *testing.T) {
+	c, cloud := getTestSetup()
+
+	groups := make(map[string]*cloudinstances.CloudInstanceGroup)
+	makeGroup(groups, c.K8sClient, cloud, "node-1", kopsapi.InstanceGroupRoleNode, 3, 3)
+	makeGroup(groups, c.K8sClient, cloud, "node-2", kopsapi.InstanceGroupRoleNode, 3, 3)
+	makeGroup(groups, c.K8sClient, cloud, "node-3", kopsapi.InstanceGroupRoleNode, 3, 3)
+	makeGroup(groups, c.K8sClient, cloud, "master-1", kopsapi.InstanceGroupRoleControlPlane, 2, 0)
+	makeGroup(groups, c.K8sClient, cloud, "bastion-1", kopsapi.InstanceGroupRoleBastion, 1, 0)
+
+	c.ClusterValidator = &instanceGroupNodeSpecificErrorClusterValidator{
+		InstanceGroup: groups["node-2"].InstanceGroup,
+	}
+
+	err := c.RollingUpdate(groups, &kopsapi.InstanceGroupList{})
+	assert.Error(t, err, "rolling update")
+
+	assertGroupInstanceCount(t, cloud, "node-1", 0)
+	assertGroupInstanceCount(t, cloud, "node-2", 3)
+	assertGroupInstanceCount(t, cloud, "node-3", 3)
+	assertGroupInstanceCount(t, cloud, "master-1", 2)
+	assertGroupInstanceCount(t, cloud, "bastion-1", 1)
+
+}
+
 func TestRollingUpdateClusterFailsValidationAfterOneNode(t *testing.T) {
 	c, cloud := getTestSetup()