internal/coordinator/pool: check if EC2 instance is a buildlet
Before destroying an untracked EC2 instance, verify that it is a
buildlet. Non buildlets will be destroyed manually as they are most
likely instances created while an operator is debugging a problem.
For golang/go#36841
Change-Id: Id0f65192ce4e72f9ecc495dd3e94750724091b51
Reviewed-on: https://go-review.googlesource.com/c/build/+/249121
Run-TryBot: Carlos Amedee <carlos@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
diff --git a/internal/coordinator/pool/ec2.go b/internal/coordinator/pool/ec2.go
index b97052c..26b44b7 100644
--- a/internal/coordinator/pool/ec2.go
+++ b/internal/coordinator/pool/ec2.go
@@ -341,6 +341,12 @@
}
deleteInsts := make([]string, 0, len(insts))
for _, inst := range insts {
+ if !isBuildlet(inst.Name) {
+ // Non-buildlets have not been created by the EC2 buildlet pool. Their lifecycle
+ // should not be managed by the pool.
+ log.Printf("destroyUntrackedInstances: skipping non-buildlet %q", inst.Name)
+ continue
+ }
if eb.isRemoteBuildlet(inst.Name) {
// Remote buildlets have their own expiration mechanism that respects active SSH sessions.
log.Printf("destroyUntrackedInstances: skipping remote buildlet %q", inst.Name)
diff --git a/internal/coordinator/pool/ec2_test.go b/internal/coordinator/pool/ec2_test.go
index 502b412..1436c81 100644
--- a/internal/coordinator/pool/ec2_test.go
+++ b/internal/coordinator/pool/ec2_test.go
@@ -512,11 +512,11 @@
func TestEC2BuildeletDestroyUntrackedInstances(t *testing.T) {
awsC := cloud.NewFakeAWSClient()
- create := func() *cloud.Instance {
+ create := func(name string) *cloud.Instance {
inst, err := awsC.CreateInstance(context.Background(), &cloud.EC2VMConfiguration{
Description: "test instance",
ImageID: "image-x",
- Name: instanceName("host-test-type", 10),
+ Name: name,
SSHKeyID: "key-14",
Tags: map[string]string{},
Type: "type-x",
@@ -529,10 +529,11 @@
}
// create untracked instances
for it := 0; it < 10; it++ {
- _ = create()
+ _ = create(instanceName("host-test-type", 10))
}
- wantTrackedInst := create()
- wantRemoteInst := create()
+ wantTrackedInst := create(instanceName("host-test-type", 10))
+ wantRemoteInst := create(instanceName("host-test-type", 10))
+ _ = create("debug-tiger-host-14") // non buildlet instance
pool := &EC2Buildlet{
awsClient: awsC,
@@ -556,9 +557,10 @@
},
}
pool.destroyUntrackedInstances()
+ wantInstCount := 3
gotInsts, err := awsC.RunningInstances(context.Background())
- if err != nil || len(gotInsts) != 2 {
- t.Errorf("awsClient.RunningInstances(ctx) = %+v, %s; want single inst and no error", gotInsts, err)
+ if err != nil || len(gotInsts) != wantInstCount {
+ t.Errorf("awsClient.RunningInstances(ctx) = %+v, %s; want %d instances and no error", gotInsts, err, wantInstCount)
}
}
diff --git a/internal/coordinator/pool/gce.go b/internal/coordinator/pool/gce.go
index 2a1a21a..e2fd063 100644
--- a/internal/coordinator/pool/gce.go
+++ b/internal/coordinator/pool/gce.go
@@ -683,7 +683,7 @@
}
}
}
- isBuildlet := strings.HasPrefix(inst.Name, "buildlet-")
+ isBuildlet := isBuildlet(inst.Name)
if isBuildlet && !sawDeleteAt && !p.instanceUsed(inst.Name) {
createdAt, _ := time.Parse(time.RFC3339Nano, inst.CreationTimestamp)
