Repository: kubernetes/kops (go)

Issue:
Rolling update panic with warm pools
/kind bug

**1. What `kops` version are you running? The command `kops version`, will display
 this information.**

> Version 1.23.2 (git-1.23.2)

**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.**

```yaml
kubernetesVersion: 1.22.10
```

**3. What cloud provider are you using?**

```yaml
cloudProvider: aws
```

**4. What commands did you run?  What is the simplest way to reproduce this issue?**

`kops rolling-update cluster --yes`.  We were also upgrading from kOps 1.22.4 to 1.23.2, in case that's relevant.

**5. What happened after the commands executed?**

Panic.

**6. What did you expect to happen?**

Smooth rolling update.

**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.**


```yaml
---
apiVersion: kops.k8s.io/v1alpha2
kind: Cluster
# ...
spec:
  # ...
  cloudProvider: aws
  kubernetesVersion: 1.22.10 # was 1.22.7, in case that's relevant
---
apiVersion: kops.k8s.io/v1alpha2
kind: InstanceGroup
# ...
spec:
  # ...
  maxSize: 30
  minSize: 1
  role: Node
  rollingUpdate:
    drainAndTerminate: true
    maxSurge: 60%
  warmPool:
    enableLifecycleHook: true
    maxSize: 4
    minSize: 3
```
**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.**

```
I0609 14:19:12.802210     485 instancegroups.go:309] Tainting 2 nodes in "nodes-eu-central-1a" instancegroup.
I0609 14:19:12.837133     485 instancegroups.go:131] deleting warm pool instance "AAAA"
I0609 14:19:13.004328     485 instancegroups.go:131] deleting warm pool instance "BBBB"
I0609 14:19:13.161806     485 instancegroups.go:131] deleting warm pool instance "CCCC"
I0609 14:19:13.309859     485 instancegroups.go:565] Detaching instance "DDDD", ...
I0609 14:19:13.564841     485 instancegroups.go:170] waiting for 15s after detaching instance
I0609 14:19:28.565202     485 instancegroups.go:467] Validating the cluster.
I0609 14:19:30.949595     485 instancegroups.go:503] Cluster validated; revalidating in 10s to make sure it does not flap.
I0609 14:19:44.024648     485 instancegroups.go:500] Cluster validated.
I0609 14:19:44.024687     485 instancegroups.go:565] Detaching instance "EEEE", ...
panic: runtime error: index out of range [-1]
goroutine 1 [running]:
k8s.io/kops/pkg/instancegroups.(*RollingUpdateCluster).rollingUpdateInstanceGroup(0xc0005ba900, 0xc0007de000, 0x37e11d600)
	k8s.io/kops/pkg/instancegroups/instancegroups.go:154 +0x1031
k8s.io/kops/pkg/instancegroups.(*RollingUpdateCluster).RollingUpdate(0xc0005ba900, 0xc0007d5590, 0xc0008aeb40)
	k8s.io/kops/pkg/instancegroups/rollingupdate.go:196 +0xa45
main.RunRollingUpdateCluster({0x7439c80, 0xc00005c1c8}, 0xc0008bfd98, {0x7343860, 0xc000010018}, 0xc00071a510)
	k8s.io/kops/cmd/kops/rolling-update_cluster.go:434 +0xf45
main.NewCmdRollingUpdateCluster.func1(0xc000765900, {0xc000088800, 0x2, 0x2})
	k8s.io/kops/cmd/kops/rolling-update_cluster.go:176 +0x37
k8s.io/kops/vendor/github.com/spf13/cobra.(*Command).execute(0xc000765900, {0xc0000887e0, 0x2, 0x2})
	k8s.io/kops/vendor/github.com/spf13/cobra/command.go:856 +0x60e
k8s.io/kops/vendor/github.com/spf13/cobra.(*Command).ExecuteC(0x9ece900)
	k8s.io/kops/vendor/github.com/spf13/cobra/command.go:974 +0x3bc
k8s.io/kops/vendor/github.com/spf13/cobra.(*Command).Execute(...)
	k8s.io/kops/vendor/github.com/spf13/cobra/command.go:902
main.Execute()
	k8s.io/kops/cmd/kops/root.go:95 +0x5c
main.main()
	k8s.io/kops/cmd/kops/main.go:20 +0x17
```

**9. Anything else do we need to know?**

Similar to #11774 


Pull request:
Warm pool-enabled ASGs scaled to zero will no longer panic
Fixes #13758 

Patch:
diff --git a/pkg/instancegroups/instancegroups.go b/pkg/instancegroups/instancegroups.go
index a8df30f2ea..5ec3c95666 100644
--- a/pkg/instancegroups/instancegroups.go
+++ b/pkg/instancegroups/instancegroups.go
@@ -105,13 +105,30 @@ func (c *RollingUpdateCluster) rollingUpdateInstanceGroup(group *cloudinstances.
 		}
 	}
 
+	nonWarmPool := []*cloudinstances.CloudInstance{}
+	// Run through the warm pool and delete all instances directly
+	for _, instance := range update {
+		if instance.State == cloudinstances.WarmPool {
+			klog.Infof("deleting warm pool instance %q", instance.ID)
+			err := c.Cloud.DeleteInstance(instance)
+			if err != nil {
+				return fmt.Errorf("failed to delete warm pool instance %q: %w", instance.ID, err)
+			}
+		} else {
+			nonWarmPool = append(nonWarmPool, instance)
+		}
+	}
+	update = nonWarmPool
+
 	settings := resolveSettings(c.Cluster, group.InstanceGroup, numInstances)
 
 	runningDrains := 0
 	maxSurge := settings.MaxSurge.IntValue()
+
 	if maxSurge > len(update) {
 		maxSurge = len(update)
 	}
+
 	maxConcurrency := maxSurge + settings.MaxUnavailable.IntValue()
 
 	if group.InstanceGroup.Spec.Role == api.InstanceGroupRoleMaster && maxSurge != 0 {
@@ -125,21 +142,6 @@ func (c *RollingUpdateCluster) rollingUpdateInstanceGroup(group *cloudinstances.
 		}
 	}
 
-	nonWarmPool := []*cloudinstances.CloudInstance{}
-	// Run through the warm pool and delete all instances directly
-	for _, instance := range update {
-		if instance.State == cloudinstances.WarmPool {
-			klog.Infof("deleting warm pool instance %q", instance.ID)
-			err := c.Cloud.DeleteInstance(instance)
-			if err != nil {
-				return fmt.Errorf("failed to delete warm pool instance %q: %w", instance.ID, err)
-			}
-		} else {
-			nonWarmPool = append(nonWarmPool, instance)
-		}
-	}
-	update = nonWarmPool
-
 	if c.Interactive {
 		if maxSurge > 1 {
 			maxSurge = 1


Test patch:
diff --git a/pkg/instancegroups/instancegroups_test.go b/pkg/instancegroups/instancegroups_test.go
new file mode 100644
index 0000000000..3656fd73ac
--- /dev/null
+++ b/pkg/instancegroups/instancegroups_test.go
@@ -0,0 +1,66 @@
+/*
+Copyright 2022 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 instancegroups
+
+import (
+	"testing"
+	"time"
+
+	"github.com/aws/aws-sdk-go/service/autoscaling"
+	"k8s.io/apimachinery/pkg/util/intstr"
+	kopsapi "k8s.io/kops/pkg/apis/kops"
+	"k8s.io/kops/pkg/cloudinstances"
+)
+
+func TestWarmPoolOnlyRoll(t *testing.T) {
+	c, cloud := getTestSetup()
+
+	groupName := "warmPoolOnly"
+	instanceID := "node-1"
+
+	groups := make(map[string]*cloudinstances.CloudInstanceGroup)
+	makeGroup(groups, c.K8sClient, cloud, groupName, kopsapi.InstanceGroupRoleNode, 0, 0)
+
+	group := groups[groupName]
+	group.MinSize = 0
+	group.MaxSize = 10
+
+	maxSurge := intstr.FromString("25%")
+
+	group.InstanceGroup.Spec.RollingUpdate = &kopsapi.RollingUpdate{
+		MaxSurge: &maxSurge,
+	}
+
+	cloud.Autoscaling().AttachInstances(&autoscaling.AttachInstancesInput{
+		AutoScalingGroupName: &groupName,
+		InstanceIds:          []*string{&instanceID},
+	})
+
+	instance, err := group.NewCloudInstance("node-1", cloudinstances.CloudInstanceStatusNeedsUpdate, nil)
+	if err != nil {
+		t.Fatalf("could not create cloud instance: %v", err)
+	}
+
+	instance.State = cloudinstances.WarmPool
+
+	{
+		err := c.rollingUpdateInstanceGroup(group, 0*time.Second)
+		if err != nil {
+			t.Fatalf("could not roll instance group: %v", err)
+		}
+	}
+}