From d77c6d04408a3b4987cedfdc46dbd0944c403ae4 Mon Sep 17 00:00:00 2001
From: Carlos Amedee <carlos@golang.org>
Date: Tue, 18 Aug 2020 13:32:05 -0400
Subject: [PATCH] dashboard: add host-linux-arm64-aws builder

Add the host-linux-arm64-aws builder. The HostConfig IsVM method has
been modified to support the concept that an EC2 VM can be created
which runs the buildlet binary in a container. This configuration
would have both a VM image and a container image set in the host
configuration.

For golang/go#36841

Change-Id: I092b48b1dda24d24bcd0b10dff0b2126f5f43017
Reviewed-on: https://go-review.googlesource.com/c/build/+/249118
Run-TryBot: Carlos Amedee <carlos@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
---
 dashboard/builders.go      | 20 ++++++++++++--
 dashboard/builders_test.go | 53 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/dashboard/builders.go b/dashboard/builders.go
index 358fe6569e..ed8a895a95 100644
--- a/dashboard/builders.go
+++ b/dashboard/builders.go
@@ -661,7 +661,9 @@ type HostConfig struct {
 	// it automatically.
 	buildletURLTmpl string
 
-	// Exactly 1 of these must be set:
+	// Exactly 1 of these must be set (with the exception of EC2 instances).
+	// An EC2 instance may run a container inside a VM. In that case, a VMImage
+	// and ContainerImage will both be set.
 	VMImage        string // e.g. "openbsd-amd64-60"
 	ContainerImage string // e.g. "linux-buildlet-std:latest" (suffix after "gcr.io/<PROJ>/")
 	IsReverse      bool   // if true, only use the reverse buildlet pool
@@ -912,7 +914,17 @@ func (c *BuildConfig) IsContainer() bool { return c.HostConfig().IsContainer() }
 func (c *HostConfig) IsContainer() bool  { return c.ContainerImage != "" }
 
 func (c *BuildConfig) IsVM() bool { return c.HostConfig().IsVM() }
-func (c *HostConfig) IsVM() bool  { return c.VMImage != "" }
+
+// IsVM reports whether the instance running the job is ultimately a VM. Hosts where
+// a VM is used only to initiate a container are considered a container, not a VM.
+// EC2 instances may be configured to run in containers that are running
+// on custom AMIs.
+func (c *HostConfig) IsVM() bool {
+	if c.isEC2 {
+		return c.VMImage != "" && c.ContainerImage == ""
+	}
+	return c.VMImage != ""
+}
 
 func (c *BuildConfig) GOOS() string { return c.Name[:strings.Index(c.Name, "-")] }
 
@@ -2209,6 +2221,10 @@ func init() {
 		HostType: "host-linux-arm64-packet",
 		FlakyNet: true, // maybe not flaky, but here conservatively
 	})
+	addBuilder(BuildConfig{
+		Name:     "linux-arm64-aws",
+		HostType: "host-linux-arm64-aws",
+	})
 	addBuilder(BuildConfig{
 		FlakyNet:       true,
 		HostType:       "host-linux-mipsle-mengzhuo",
diff --git a/dashboard/builders_test.go b/dashboard/builders_test.go
index 7f593f369d..8c87c693a3 100644
--- a/dashboard/builders_test.go
+++ b/dashboard/builders_test.go
@@ -598,7 +598,6 @@ func TestHostConfigsAllUsed(t *testing.T) {
 		"host-linux-armel-cross": true,
 
 		"host-linux-x86-alpine": true, // TODO(golang.org/issue/19938): Fix the Alpine builder, or remove it.
-		"host-linux-arm64-aws":  true, // TODO(golang.org/issue/36841): Add a builder that uses this host, or remove it.
 	}
 
 	used := make(map[string]bool)
@@ -874,3 +873,55 @@ func TestLongTestBuilder(t *testing.T) {
 		t.Error("the linux-amd64-longtest builder doesn't set GO_TEST_SHORT=0, is that intentional?")
 	}
 }
+
+func TestHostConfigIsVM(t *testing.T) {
+	testCases := []struct {
+		desc   string
+		config *HostConfig
+		want   bool
+	}{
+		{
+			desc: "non-ec2-vm",
+			config: &HostConfig{
+				VMImage:        "image-x",
+				ContainerImage: "",
+				isEC2:          false,
+			},
+			want: true,
+		},
+		{
+			desc: "non-ec2-container",
+			config: &HostConfig{
+				VMImage:        "",
+				ContainerImage: "container-image-x",
+				isEC2:          false,
+			},
+			want: false,
+		},
+		{
+			desc: "ec2-container",
+			config: &HostConfig{
+				VMImage:        "image-x",
+				ContainerImage: "container-image-x",
+				isEC2:          true,
+			},
+			want: false,
+		},
+		{
+			desc: "ec2-vm",
+			config: &HostConfig{
+				VMImage:        "image-x",
+				ContainerImage: "",
+				isEC2:          true,
+			},
+			want: true,
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(fmt.Sprintf(tc.desc), func(t *testing.T) {
+			if got := tc.config.IsVM(); got != tc.want {
+				t.Errorf("HostConfig.IsVM() = %t; want %t", got, tc.want)
+			}
+		})
+	}
+}