From 3b94076fa1ef9573281b41dbf3c3b65b4a3ac679 Mon Sep 17 00:00:00 2001 From: Carlos Amedee Date: Tue, 5 May 2020 18:01:50 -0400 Subject: [PATCH] buildlet: modify the request logic and parameters for EC2 instances This change makes multiple changes to the creation of EC2 instances via the AWS buildlet: - Adds a tag resource type. - Adds an ssh key. - Instead of waiting for an instance to exist, it now waits for the instance to be running. - Renames the user data type. - It base64 encodes the user data. Updates golang/go#36841 Change-Id: I90e98dfa11f3a14478580ba4ca4a79724c085be9 Reviewed-on: https://go-review.googlesource.com/c/build/+/233798 Run-TryBot: Carlos Amedee TryBot-Result: Gobot Gobot Reviewed-by: Alexander Rakoczy --- buildlet/aws.go | 27 ++++++++++++++++++--------- buildlet/aws_test.go | 14 +++++++++++--- buildlet/buildlet_test.go | 4 ++-- 3 files changed, 31 insertions(+), 14 deletions(-) diff --git a/buildlet/aws.go b/buildlet/aws.go index a24aca50..8842fc36 100644 --- a/buildlet/aws.go +++ b/buildlet/aws.go @@ -6,6 +6,7 @@ package buildlet import ( "context" + "encoding/base64" "encoding/json" "errors" "fmt" @@ -13,20 +14,19 @@ import ( "net" "time" - "golang.org/x/build/buildenv" - "golang.org/x/build/dashboard" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/credentials" "github.com/aws/aws-sdk-go/aws/request" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ec2" + "golang.org/x/build/buildenv" + "golang.org/x/build/dashboard" ) -// AWSUserData is stored in the user data for each EC2 instance. This is +// EC2UserData is stored in the user data for each EC2 instance. This is // used to store metadata about the running instance. The buildlet will retrieve // this on EC2 instances before allowing connections from the coordinator. -type AWSUserData struct { +type EC2UserData struct { BuildletBinaryURL string `json:"buildlet_binary_url,omitempty"` BuildletHostType string `json:"buildlet_host_type,omitempty"` Metadata map[string]string `json:"metadata,omitempty"` @@ -41,7 +41,7 @@ type ec2Client interface { DescribeInstancesWithContext(context.Context, *ec2.DescribeInstancesInput, ...request.Option) (*ec2.DescribeInstancesOutput, error) RunInstancesWithContext(context.Context, *ec2.RunInstancesInput, ...request.Option) (*ec2.Reservation, error) TerminateInstancesWithContext(context.Context, *ec2.TerminateInstancesInput, ...request.Option) (*ec2.TerminateInstancesOutput, error) - WaitUntilInstanceExistsWithContext(context.Context, *ec2.DescribeInstancesInput, ...request.WaiterOption) error + WaitUntilInstanceRunningWithContext(context.Context, *ec2.DescribeInstancesInput, ...request.WaiterOption) error } // AWSClient is the client used to create and destroy buildlets on AWS. @@ -92,6 +92,7 @@ func (c *AWSClient) StartNewVM(ctx context.Context, buildEnv *buildenv.Environme } vmConfig := c.configureVM(buildEnv, hconf, vmName, hostType, opts) + vmID, err := c.createVM(ctx, vmConfig, opts) if err != nil { return nil, err @@ -122,7 +123,7 @@ func (c *AWSClient) createVM(ctx context.Context, vmConfig *ec2.RunInstancesInpu // WaitUntilVMExists submits a request which waits until an instance exists before returning. func (c *AWSClient) WaitUntilVMExists(ctx context.Context, instID string, opts *VMOpts) error { - err := c.client.WaitUntilInstanceExistsWithContext(ctx, &ec2.DescribeInstancesInput{ + err := c.client.WaitUntilInstanceRunningWithContext(ctx, &ec2.DescribeInstancesInput{ InstanceIds: []*string{aws.String(instID)}, }) if err != nil { @@ -158,9 +159,11 @@ func (c *AWSClient) configureVM(buildEnv *buildenv.Environment, hconf *dashboard Placement: &ec2.Placement{ AvailabilityZone: aws.String(opts.Zone), }, + KeyName: aws.String("ec2-go-builders"), InstanceInitiatedShutdownBehavior: aws.String("terminate"), TagSpecifications: []*ec2.TagSpecification{ &ec2.TagSpecification{ + ResourceType: aws.String("instance"), Tags: []*ec2.Tag{ &ec2.Tag{ Key: aws.String("Name"), @@ -174,9 +177,13 @@ func (c *AWSClient) configureVM(buildEnv *buildenv.Environment, hconf *dashboard }, }, } + c.vmUserDataSpec(vmConfig, buildEnv, hconf, vmName, hostType, opts) + return vmConfig +} +func (c *AWSClient) vmUserDataSpec(vmConfig *ec2.RunInstancesInput, buildEnv *buildenv.Environment, hconf *dashboard.HostConfig, vmName, hostType string, opts *VMOpts) { // add custom metadata to the user data. - ud := AWSUserData{ + ud := EC2UserData{ BuildletBinaryURL: hconf.BuildletBinaryURL(buildEnv), BuildletHostType: hostType, TLSCert: opts.TLS.CertPEM, @@ -191,7 +198,9 @@ func (c *AWSClient) configureVM(buildEnv *buildenv.Environment, hconf *dashboard if err != nil { log.Printf("unable to marshal user data: %v", err) } - return vmConfig.SetUserData(string(jsonUserData)) + // user data must be base64 encoded + jud := base64.StdEncoding.EncodeToString([]byte(jsonUserData)) + vmConfig.SetUserData(jud) } // DestroyVM submits a request to destroy a VM. diff --git a/buildlet/aws_test.go b/buildlet/aws_test.go index 26d35376..1815f029 100644 --- a/buildlet/aws_test.go +++ b/buildlet/aws_test.go @@ -6,6 +6,7 @@ package buildlet import ( "context" + "encoding/base64" "encoding/json" "errors" "testing" @@ -78,7 +79,7 @@ func (f *fakeEC2Client) TerminateInstancesWithContext(ctx context.Context, input }, nil } -func (f *fakeEC2Client) WaitUntilInstanceExistsWithContext(ctx context.Context, input *ec2.DescribeInstancesInput, opt ...request.WaiterOption) error { +func (f *fakeEC2Client) WaitUntilInstanceRunningWithContext(ctx context.Context, input *ec2.DescribeInstancesInput, opt ...request.WaiterOption) error { if ctx == nil || input == nil || len(input.InstanceIds) == 0 { return request.ErrInvalidParams{} } @@ -502,6 +503,9 @@ func TestConfigureVM(t *testing.T) { if *got.InstanceInitiatedShutdownBehavior != "terminate" { t.Errorf("InstanceType got %s; want %s", *got.InstanceInitiatedShutdownBehavior, "terminate") } + if *got.TagSpecifications[0].ResourceType != "instance" { + t.Errorf("Tag Resource Type got %s; want %s", *got.TagSpecifications[0].ResourceType, "instance") + } if *got.TagSpecifications[0].Tags[0].Key != "Name" { t.Errorf("First Tag Key got %s; want %s", *got.TagSpecifications[0].Tags[0].Key, "Name") } @@ -514,8 +518,12 @@ func TestConfigureVM(t *testing.T) { if *got.TagSpecifications[0].Tags[1].Value != tc.wantDesc { t.Errorf("Second Tag Value got %s; want %s", *got.TagSpecifications[0].Tags[1].Value, tc.wantDesc) } - gotUD := &AWSUserData{} - err := json.Unmarshal([]byte(*got.UserData), &gotUD) + gotUD := &EC2UserData{} + gotUDJson, err := base64.StdEncoding.DecodeString(*got.UserData) + if err != nil { + t.Fatalf("unable to base64 decode string %q: %s", *got.UserData, err) + } + err = json.Unmarshal([]byte(gotUDJson), gotUD) if err != nil { t.Errorf("unable to unmarshal user data: %v", err) } diff --git a/buildlet/buildlet_test.go b/buildlet/buildlet_test.go index eae93173..e270055c 100644 --- a/buildlet/buildlet_test.go +++ b/buildlet/buildlet_test.go @@ -92,10 +92,10 @@ func TestBuildletClientError(t *testing.T) { t.Error("http endpoint called") } if OnBeginBuildletProbeCalled { - t.Error("OnBeginBuildletProbe() was not called") + t.Error("OnBeginBuildletProbe() was called") } if OnEndBuildletProbeCalled { - t.Error("OnEndBuildletProbe() was not called") + t.Error("OnEndBuildletProbe() was called") } }