Merge pull request #949 from aws/dev

bvtujo · web-flow · commit 3970a6c1f38b · 2019-11-18T15:34:30.000-08:00
Merge dev into master
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,13 @@
 # Changelog
 
+## 1.18.0
+* Enhancement - Print verbose messages for credential chain errors (#937)
+* Enhancement - Add G4 instance support (#940)
+* Enhancement - Add more descriptive logging to integ tests (#931)
+* Bug - Pull correct AMI type for G3 instance types. (#940)
+* Bug - Send optional fields as nil instead of "" to ECS (#938)
+* Bug - ecs-cli local up can now read from parameter store when forward slashes exist in parameter name (#935)
+
 ## 1.17.0
 * Feature - Add support for Firelens (#924)
 * Bug - Remove spurious log warning (#926)
diff --git a/VERSION b/VERSION
@@ -1 +1 @@
-1.17.0
+1.18.0
diff --git a/buildspec.yml b/buildspec.yml
@@ -1,7 +1,73 @@
 version: 0.2
 
 phases:
+  install:
+    runtime-versions:
+      golang: 1.13
+  pre_build:
+    commands:
+      # GOPATH is setup like the following in Codebuild standard 2.0 image
+      # /go:/codebuild/output/src<some numbers>
+      # so we copy all the source code to the appropriate location before
+      # invoking any go command
+      - ls -lah
+      - mkdir -p /go/src/github.com/aws/amazon-ecs-cli
+      - cp -R . /go/src/github.com/aws/amazon-ecs-cli/
+      - |
+        env -i PATH=$PATH GOPATH=`go env GOPATH` GOROOT=`go env GOROOT` GOCACHE=`go env GOCACHE` \
+        go test -race -v -cover github.com/aws/amazon-ecs-cli/ecs-cli/modules/...
+      # make a copy of the version.go because `go run gen/version-gen.go` will
+      # modify it.
+      - cp /go/src/github.com/aws/amazon-ecs-cli/ecs-cli/modules/version/version.go /go/src/github.com/aws/amazon-ecs-cli/ecs-cli/modules/version/_version.go
+      # need to cd into the version package because version-gen.go assumes the relative
+      # location of the VERSION file.
+      - cd /go/src/github.com/aws/amazon-ecs-cli/ecs-cli/modules/version/
+      # since we are running the go program inside a Linux container, has to hardcode
+      # the GOOS and GOARCH correspondinly regardless of what the host OS is.
+      - GOOS=linux GOARCH=amd64 ECS_RELEASE=cleanbuild go run gen/version-gen.go
   build:
     commands:
-      - make integ-test
-      - ./bin/local/ecs-cli.test
+      - echo "cd into $CODEBUILD_SRC_DIR"
+      - cd $CODEBUILD_SRC_DIR
+      - echo "Compilation context:"
+      - echo "CODEBUILD_SOURCE_VERSION=$CODEBUILD_SOURCE_VERSION"
+      - VERSION=`git tag --points-at HEAD`
+      - echo "VERSION=$VERSION"
+      - GIT_COMMIT_ID=`git rev-parse HEAD`
+      - echo "GIT_COMMIT_ID=$GIT_COMMIT_ID"
+      # TODO: Get rid of the VERSION file after we fully switch to the new CI/CD
+      - CHECKED_IN_VERSION=`cat /go/src/github.com/aws/amazon-ecs-cli/VERSION`
+      - echo "VERSION_FILE=$CHECKED_IN_VERSION"
+      - echo "GOPATH=$GOPATH"
+      - |
+        # $CHECKED_IN_VERSION is not prefixed with "v", only the semantic version number,
+        # such as 1.17.0 instead of v1.17.0, which is what normal version tags look like.
+        if [ "$VERSION" != "v$CHECKED_IN_VERSION" ]; then
+          echo "the VERSION file contains a version number that is different from the git tag. file: $CHECKED_IN_VERSION, tag: $VERSION"  
+          exit 1
+        fi
+      - GOOS=windows GOARCH=amd64 CGO_ENABLED=0 go build -installsuffix cgo -a -ldflags "-s" -o aws/amazon-ecs-cli/ecs-cli-windows-amd64-$VERSION.exe github.com/aws/amazon-ecs-cli/ecs-cli/
+      - GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go build -installsuffix cgo -a -ldflags "-s" -o aws/amazon-ecs-cli/ecs-cli-linux-amd64-$VERSION github.com/aws/amazon-ecs-cli/ecs-cli/
+      - GOOS=darwin GOARCH=amd64 CGO_ENABLED=0 go build -installsuffix cgo -a -ldflags "-s" -o aws/amazon-ecs-cli/ecs-cli-darwin-amd64-$VERSION github.com/aws/amazon-ecs-cli/ecs-cli/
+    finally:
+      - echo "built artifacts:"
+      - ls -lah aws/amazon-ecs-cli/
+      - ./aws/amazon-ecs-cli/ecs-cli-linux-amd64-$VERSION --version
+  post_build:
+    commands:
+      # restore the version file
+      - mv /go/src/github.com/aws/amazon-ecs-cli/ecs-cli/modules/version/_version.go /go/src/github.com/aws/amazon-ecs-cli/ecs-cli/modules/version/version.go
+      - echo "Creating latest artifacts..."
+      - cp aws/amazon-ecs-cli/ecs-cli-windows-amd64-$VERSION.exe aws/amazon-ecs-cli/ecs-cli-windows-amd64-latest.exe
+      - cp aws/amazon-ecs-cli/ecs-cli-linux-amd64-$VERSION aws/amazon-ecs-cli/ecs-cli-linux-amd64-latest
+      - cp aws/amazon-ecs-cli/ecs-cli-darwin-amd64-$VERSION aws/amazon-ecs-cli/ecs-cli-darwin-amd64-latest
+      - MANIFESTFILE="$GIT_COMMIT_ID.manifest"
+      - echo "aws/amazon-ecs-cli/ecs-cli-windows-amd64-$VERSION.exe" >> $MANIFESTFILE
+      - echo "aws/amazon-ecs-cli/ecs-cli-linux-amd64-$VERSION" >> $MANIFESTFILE
+      - echo "aws/amazon-ecs-cli/ecs-cli-darwin-amd64-$VERSION" >> $MANIFESTFILE
+      - echo "aws/amazon-ecs-cli/ecs-cli-windows-amd64-latest.exe" >> $MANIFESTFILE
+      - echo "aws/amazon-ecs-cli/ecs-cli-linux-amd64-latest" >> $MANIFESTFILE
+      - echo "aws/amazon-ecs-cli/ecs-cli-darwin-amd64-latest" >> $MANIFESTFILE
+artifacts:
+  files:
+    - '**/*'
diff --git a/ecs-cli/integ/cmd/compose.go b/ecs-cli/integ/cmd/compose.go
@@ -268,6 +268,7 @@ func testServiceHasAllRunningContainers(t *testing.T, p *Project, wantedNumOfCon
 	containers := lines[1:] // Drop the headers
 	if wantedNumOfContainers != len(containers) {
 		t.Logf("Wanted = %d, got = %d running containers", wantedNumOfContainers, len(containers))
+		t.Logf("Current running containers = %v", containers)
 		return false
 	}
 	for _, container := range containers {
@@ -276,6 +277,7 @@ func testServiceHasAllRunningContainers(t *testing.T, p *Project, wantedNumOfCon
 			t.Logf("Container is not RUNNING: %s", container)
 			return false
 		}
+		t.Logf("Container is RUNNING: %s", container)
 	}
 	return true
 }
diff --git a/ecs-cli/modules/cli/local/secrets/clients/clients.go b/ecs-cli/modules/cli/local/secrets/clients/clients.go
@@ -15,8 +15,6 @@
 package clients
 
 import (
-	"strings"
-
 	"github.com/aws/amazon-ecs-cli/ecs-cli/modules/config"
 	"github.com/aws/aws-sdk-go/aws"
 	arnParser "github.com/aws/aws-sdk-go/aws/arn"
@@ -62,8 +60,7 @@ func (d *SSMDecrypter) DecryptSecret(arnOrName string) (string, error) {
 	// If the value is an ARN we need to retrieve the parameter name and update the region of the client.
 	paramName := arnOrName
 	if parsedARN, err := arnParser.Parse(arnOrName); err == nil {
-		resource := strings.Split(parsedARN.Resource, "/") // Resource is formatted as parameter/{paramName}.
-		paramName = strings.Join(resource[1:], "")
+		paramName = parsedARN.Resource[len("parameter/"):] // Resource is formatted as parameter/{paramName}.
 		d.SSMAPI = d.getClient(region(parsedARN.Region))
 	}
 
@@ -84,7 +81,10 @@ func (d *SSMDecrypter) getClient(r region) ssmiface.SSMAPI {
 		return c
 	}
 	c := ssm.New(session.Must(session.NewSessionWithOptions(session.Options{
-		Config: aws.Config{Region: aws.String(string(r))},
+		Config: aws.Config{
+			Region:                        aws.String(string(r)),
+			CredentialsChainVerboseErrors: aws.Bool(true),
+		},
 	})))
 	d.clients[r] = c
 	return c
@@ -111,7 +111,10 @@ func (d *SecretsManagerDecrypter) getClient(r region) secretsmanageriface.Secret
 		return c
 	}
 	c := secretsmanager.New(session.Must(session.NewSessionWithOptions(session.Options{
-		Config: aws.Config{Region: aws.String(string(r))},
+		Config: aws.Config{
+			Region:                        aws.String(string(r)),
+			CredentialsChainVerboseErrors: aws.Bool(true),
+		},
 	})))
 	d.clients[r] = c
 	return c
diff --git a/ecs-cli/modules/cli/local/secrets/clients/clients_test.go b/ecs-cli/modules/cli/local/secrets/clients/clients_test.go
@@ -116,6 +116,37 @@ func TestSSMDecrypter_DecryptSecret(t *testing.T) {
 					pdxClient.EXPECT().GetParameter(gomock.Any()).Times(0), // Should not have called PDX
 				)
 
+				return &SSMDecrypter{
+					SSMAPI:  iadClient,
+					clients: m,
+				}
+			},
+		},
+		"with forward slash": {
+			input:        "/TEST/DB/PASSWORD",
+			wantedSecret: "hello",
+			setupDecrypter: func(ctrl *gomock.Controller) *SSMDecrypter {
+				iadClient := mock_ssmiface.NewMockSSMAPI(ctrl)
+				pdxClient := mock_ssmiface.NewMockSSMAPI(ctrl)
+
+				m := make(map[region]ssmiface.SSMAPI)
+				m["default"] = iadClient
+				m["us-east-1"] = iadClient
+				m["us-west-2"] = pdxClient
+
+				gomock.InOrder(
+					iadClient.EXPECT().GetParameter(&ssm.GetParameterInput{
+						Name:           aws.String("/TEST/DB/PASSWORD"),
+						WithDecryption: aws.Bool(true),
+					}).Return(&ssm.GetParameterOutput{
+						Parameter: &ssm.Parameter{
+							Value: aws.String("hello"),
+						},
+					}, nil),
+
+					pdxClient.EXPECT().GetParameter(gomock.Any()).Times(0), // Should not have called PDX
+				)
+
 				return &SSMDecrypter{
 					SSMAPI:  iadClient,
 					clients: m,
diff --git a/ecs-cli/modules/clients/aws/amimetadata/client.go b/ecs-cli/modules/clients/aws/amimetadata/client.go
@@ -111,15 +111,21 @@ func isARM64Instance(instanceType string) bool {
 	return false
 }
 
+// See: https://docs.aws.amazon.com/AmazonECS/latest/developerguide/ecs-gpu.html
 func isGPUInstance(instanceType string) bool {
-	if strings.HasPrefix(instanceType, "p2.") {
-		return true
-	}
-	if strings.HasPrefix(instanceType, "p3.") {
-		return true
+	var gpuInstanceClasses = []string{
+		"p2.",
+		"p3.",
+		"p3dn.",
+		"g3.",
+		"g3s.",
+		"g4dn.",
 	}
-	if strings.HasPrefix(instanceType, "p3dn.") {
-		return true
+	for _, instanceClass := range gpuInstanceClasses {
+
+		if strings.HasPrefix(instanceType, instanceClass) {
+			return true
+		}
 	}
 	return false
 }
diff --git a/ecs-cli/modules/clients/aws/amimetadata/client_test.go b/ecs-cli/modules/clients/aws/amimetadata/client_test.go
@@ -41,6 +41,17 @@ func TestMetadataClient_GetRecommendedECSLinuxAMI(t *testing.T) {
 			},
 			nil,
 		},
+		{
+			// validate that we use GPU optimized AMI for GPU instances
+			"g4dn.xlarge",
+			func(ssmClient *mock_ssmiface.MockSSMAPI) *mock_ssmiface.MockSSMAPI {
+				ssmClient.EXPECT().GetParameter(gomock.Any()).Do(func(input *ssm.GetParameterInput) {
+					assert.Equal(t, amazonLinux2X86GPURecommendedParameterName, *input.Name)
+				}).Return(emptySSMParameterOutput(), nil)
+				return ssmClient
+			},
+			nil,
+		},
 		{
 			// validate that we use the generic AMI for other instances
 			"t2.micro",
diff --git a/ecs-cli/modules/clients/aws/cloudformation/cluster_template.go b/ecs-cli/modules/clients/aws/cloudformation/cluster_template.go
@@ -279,6 +279,12 @@ var clusterTemplate = `
         "g3.4xlarge",
         "g3.8xlarge",
         "g3.16xlarge",
+        "g4dn.xlarge",
+        "g4dn.2xlarge",
+        "g4dn.4xlarge",
+        "g4dn.8xlarge",
+        "g4dn.12xlarge",
+        "g4dn.16xlarge",
         "p2.xlarge",
         "p2.8xlarge",
         "p2.16xlarge",
diff --git a/ecs-cli/modules/clients/aws/cloudwatchlogs/client.go b/ecs-cli/modules/clients/aws/cloudwatchlogs/client.go
@@ -34,7 +34,10 @@ type cwLogsClient struct {
 
 // NewCloudWatchLogsClient creates an instance of ec2Client object.
 func NewCloudWatchLogsClient(params *config.CommandConfig, logRegion string) Client {
-	newSession := params.Session.Copy(&aws.Config{Region: aws.String(logRegion)})
+	newSession := params.Session.Copy(&aws.Config{
+		Region: aws.String(logRegion),
+		CredentialsChainVerboseErrors: aws.Bool(true),
+	})
 	client := cloudwatchlogs.New(newSession)
 	client.Handlers.Build.PushBackNamed(clients.CustomUserAgentHandler())
 	return &cwLogsClient{
diff --git a/ecs-cli/modules/config/config_v1.go b/ecs-cli/modules/config/config_v1.go
@@ -118,7 +118,9 @@ func NewLocalConfig(cluster string) *LocalConfig {
 //    a) AWS_DEFAULT_PROFILE environment variable (defaults to 'default')
 //  5) EC2 Instance role
 func (cfg *LocalConfig) ToAWSSession(context *cli.Context) (*session.Session, error) {
-	svcConfig := aws.Config{}
+	svcConfig := aws.Config{
+		CredentialsChainVerboseErrors: aws.Bool(true),
+	}
 	if ecsEndpoint := RecursiveFlagSearch(context, flags.EndpointFlag); ecsEndpoint != "" {
 		defaultResolver := endpoints.DefaultResolver()
 		ecsCustomResolverFn := func(service, region string, optFns ...func(*endpoints.Options)) (endpoints.ResolvedEndpoint, error) {
diff --git a/ecs-cli/modules/utils/compose/convert_task_definition.go b/ecs-cli/modules/utils/compose/convert_task_definition.go
@@ -286,8 +286,12 @@ func mergeVolumesWithoutHost(composeVolumes []string, ecsParams *ECSParams) ([]*
 		if dVol.Name != "" {
 			ecsVolume.DockerVolumeConfiguration = &ecs.DockerVolumeConfiguration{
 				Autoprovision: dVol.Autoprovision,
-				Driver:        aws.String(dVol.Driver),
-				Scope:         aws.String(dVol.Scope),
+			}
+			if dVol.Driver != nil {
+				ecsVolume.DockerVolumeConfiguration.Driver = dVol.Driver
+			}
+			if dVol.Scope != nil {
+				ecsVolume.DockerVolumeConfiguration.Scope = dVol.Scope
 			}
 			if dVol.DriverOptions != nil {
 				ecsVolume.DockerVolumeConfiguration.DriverOpts = aws.StringMap(dVol.DriverOptions)
diff --git a/ecs-cli/modules/utils/compose/convert_task_definition_test.go b/ecs-cli/modules/utils/compose/convert_task_definition_test.go
@@ -1383,8 +1383,8 @@ func TestConvertToTaskDefinitionWithECSParamsVolumeWithoutNameError(t *testing.T
 			DockerVolumes: []DockerVolume{
 				DockerVolume{
 					Autoprovision: aws.Bool(true),
-					Scope:         "shared",
-					Driver:        "local",
+					Scope:         aws.String("shared"),
+					Driver:        nil,
 					DriverOptions: options,
 					Labels:        labels,
 				},
diff --git a/ecs-cli/modules/utils/compose/ecs_params_reader.go b/ecs-cli/modules/utils/compose/ecs_params_reader.go
@@ -75,9 +75,9 @@ type ContainerDef struct {
 
 type DockerVolume struct {
 	Name          string            `yaml:"name"`
-	Scope         string            `yaml:"scope"`
+	Scope         *string           `yaml:"scope"`
 	Autoprovision *bool             `yaml:"autoprovision"`
-	Driver        string            `yaml:"driver"`
+	Driver        *string           `yaml:"driver"`
 	DriverOptions map[string]string `yaml:"driver_opts"`
 	Labels        map[string]string `yaml:"labels"`
 }
diff --git a/ecs-cli/modules/utils/compose/ecs_params_reader_test.go b/ecs-cli/modules/utils/compose/ecs_params_reader_test.go
@@ -647,9 +647,9 @@ task_definition:
 	expectedVolumes := []DockerVolume{
 		DockerVolume{
 			Name:          "my_volume",
-			Scope:         "shared",
+			Scope:         aws.String("shared"),
 			Autoprovision: aws.Bool(true),
-			Driver:        "doggyromcom",
+			Driver:        aws.String("doggyromcom"),
 			DriverOptions: map[string]string{
 				"pudding": "is-engaged-to-marry-Tum-Tum",
 				"clyde":   "professes-his-love-at-the-ceremony",
diff --git a/ecs-cli/modules/version/version.go b/ecs-cli/modules/version/version.go
@@ -22,7 +22,7 @@ package version
 // repository. Only the 'Version' const should change in checked-in source code
 
 // Version is the version of the ECS CLI
-const Version = "1.17.0"
+const Version = "1.18.0"
 
 // GitDirty indicates the cleanliness of the git repo when this ecs-cli was built
 const GitDirty = true
diff --git a/scripts/publish-staged b/scripts/publish-staged
@@ -43,7 +43,7 @@ usage() {
 }
 
 publish_s3() {
-	for tag in ${ARTIFACT_TAG_VERSION} ${ARTIFACT_TAG_SHA} ${ARTIFACT_TAG_LATEST}; do
+	for tag in ${ARTIFACT_TAG_VERSION} ${ARTIFACT_TAG_LATEST}; do
 		echo "Publishing as ecs-cli-linux-amd64-${tag}"
 		dexec s3_pull_push "s3://${STAGE_S3_BUCKET}/ecs-cli-linux-amd64-${tag}" "s3://${PUBLISH_S3_BUCKET}/ecs-cli-linux-amd64-${tag}"
 		dexec s3_pull_push "s3://${STAGE_S3_BUCKET}/ecs-cli-linux-amd64-${tag}.md5" "s3://${PUBLISH_S3_BUCKET}/ecs-cli-linux-amd64-${tag}.md5"

Original file line number	Diff line number	Diff line change
`@@ -268,6 +268,7 @@ func testServiceHasAllRunningContainers(t testing.T, p Project, wantedNumOfCon`
`268`	`268`	`containers := lines[1:] // Drop the headers`
`269`	`269`	`if wantedNumOfContainers != len(containers) {`
`270`	`270`	`t.Logf("Wanted = %d, got = %d running containers", wantedNumOfContainers, len(containers))`
	`271`	`+ t.Logf("Current running containers = %v", containers)`
`271`	`272`	`return false`
`272`	`273`	`}`
`273`	`274`	`for _, container := range containers {`
`@@ -276,6 +277,7 @@ func testServiceHasAllRunningContainers(t testing.T, p Project, wantedNumOfCon`
`276`	`277`	`t.Logf("Container is not RUNNING: %s", container)`
`277`	`278`	`return false`
`278`	`279`	`}`
	`280`	`+ t.Logf("Container is RUNNING: %s", container)`
`279`	`281`	`}`
`280`	`282`	`return true`
`281`	`283`	`}`