feat: add container healthcheck for sidecars (#2822)

<!-- Provide summary of changes -->
fixes #2723.
<!-- Issue number, if available. E.g. "Fixes #31", "Addresses #42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License..

internal/pkg/deploy/cloudformation/stack/backend_svc.go

@@ -134,7 +134,7 @@ func (s *BackendService) Template() (string, error) {
 		DesiredCountOnSpot:       desiredCountOnSpot,
 		ExecuteCommand:           convertExecuteCommand(&s.manifest.ExecuteCommand),
 		WorkloadType:             manifest.BackendServiceType,
-		HealthCheck:              s.manifest.BackendServiceConfig.ImageConfig.HealthCheckOpts(),
+		HealthCheck:              convertContainerHealthCheck(s.manifest.BackendServiceConfig.ImageConfig.HealthCheck),
 		LogConfig:                convertLogging(s.manifest.Logging),
 		DockerLabels:             s.manifest.ImageConfig.DockerLabels,
 		DesiredCountLambda:       desiredCountLambda.String(),

internal/pkg/deploy/cloudformation/stack/backend_svc_test.go

@@ -12,7 +12,6 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/cloudformation"
-	"github.com/aws/aws-sdk-go/service/ecs"
 	"github.com/aws/copilot-cli/internal/pkg/addon"
 	"github.com/aws/copilot-cli/internal/pkg/deploy/cloudformation/stack/mocks"
 	"github.com/aws/copilot-cli/internal/pkg/manifest"
@@ -185,8 +184,8 @@ Outputs:
 				m.EXPECT().Read(envControllerPath).Return(&template.Content{Buffer: bytes.NewBufferString("something")}, nil)
 				m.EXPECT().ParseBackendService(template.WorkloadOpts{
 					WorkloadType: manifest.BackendServiceType,
-					HealthCheck: &ecs.HealthCheck{
-						Command:     aws.StringSlice([]string{"CMD-SHELL", "curl -f http://localhost/ || exit 1"}),
+					HealthCheck: &template.ContainerHealthCheck{
+						Command:     []string{"CMD-SHELL", "curl -f http://localhost/ || exit 1"},
 						Interval:    aws.Int64(5),
 						Retries:     aws.Int64(3),
 						StartPeriod: aws.Int64(0),

internal/pkg/deploy/cloudformation/stack/lb_web_svc.go

@@ -177,7 +177,7 @@ func (s *LoadBalancedWebService) Template() (string, error) {
 		DesiredCountOnSpot:       desiredCountOnSpot,
 		ExecuteCommand:           convertExecuteCommand(&s.manifest.ExecuteCommand),
 		WorkloadType:             manifest.LoadBalancedWebServiceType,
-		HealthCheck:              s.manifest.ImageConfig.HealthCheckOpts(),
+		HealthCheck:              convertContainerHealthCheck(s.manifest.ImageConfig.HealthCheck),
 		HTTPHealthCheck:          convertHTTPHealthCheck(&s.manifest.HealthCheck),
 		DeregistrationDelay:      deregistrationDelay,
 		AllowedSourceIps:         allowedSourceIPs,

internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go

@@ -11,7 +11,6 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/cloudformation"
-	"github.com/aws/aws-sdk-go/service/ecs"
 	"github.com/aws/copilot-cli/internal/pkg/addon"
 	"github.com/aws/copilot-cli/internal/pkg/deploy"
 	"github.com/aws/copilot-cli/internal/pkg/deploy/cloudformation/stack/mocks"
@@ -92,8 +91,8 @@ func TestLoadBalancedWebService_StackName(t *testing.T) {
 }
 
 func TestLoadBalancedWebService_Template(t *testing.T) {
-	var overridenContainerHealthCheck = ecs.HealthCheck{
-		Command:     []*string{aws.String("CMD-SHELL"), aws.String("curl -f http://localhost/ || exit 1")},
+	var overridenContainerHealthCheck = template.ContainerHealthCheck{
+		Command:     []string{"CMD-SHELL", "curl -f http://localhost/ || exit 1"},
 		Interval:    aws.Int64(10),
 		StartPeriod: aws.Int64(0),
 		Timeout:     aws.Int64(5),

internal/pkg/deploy/cloudformation/stack/scheduled_job.go

@@ -176,7 +176,7 @@ func (j *ScheduledJob) Template() (string, error) {
 		Sidecars:                 sidecars,
 		ScheduleExpression:       schedule,
 		StateMachine:             stateMachine,
-		HealthCheck:              j.manifest.ImageConfig.HealthCheckOpts(),
+		HealthCheck:              convertContainerHealthCheck(j.manifest.ImageConfig.HealthCheck),
 		LogConfig:                convertLogging(j.manifest.Logging),
 		DockerLabels:             j.manifest.ImageConfig.DockerLabels,
 		Storage:                  storage,

internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-manifest.yml

@@ -59,5 +59,13 @@ environments:
         spot_from: 6
     logging:
       retention: 1
-
+    sidecars:
+      nginx:
+        port: 80
+        image: 1234567890.dkr.ecr.us-west-2.amazonaws.com/reverse-proxy:revision_1
+        variables:
+          NGINX_PORT: 80
+        healthcheck:
+          command: ["CMD-SHELL", "curl -f http://localhost:8080 || exit 1"]
+          interval: 10s
 

internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-prod.stack.yml

@@ -107,9 +107,26 @@ Resources:
               ReadOnly: true
               SourceVolume: persistence
           PortMappings:
-            - ContainerPort: !Ref ContainerPort        
-
-
+            - ContainerPort: !Ref ContainerPort 
+        - Name: nginx
+          Image: 1234567890.dkr.ecr.us-west-2.amazonaws.com/reverse-proxy:revision_1
+          PortMappings:
+            - ContainerPort: 80
+          HealthCheck:
+            Command: ["CMD-SHELL", "curl -f http://localhost:8080 || exit 1"]
+            Interval: 10
+            Retries: 2
+            StartPeriod: 0
+            Timeout: 5
+          Environment:
+            - Name: NGINX_PORT
+              Value: "80"
+          LogConfiguration:
+            LogDriver: awslogs
+            Options:
+              awslogs-region: !Ref AWS::Region
+              awslogs-group: !Ref LogGroup
+              awslogs-stream-prefix: copilot     
       Volumes:
         - Name: persistence
   ExecutionRole:

internal/pkg/deploy/cloudformation/stack/transformers.go

@@ -118,12 +118,28 @@ func convertSidecar(s convertSidecarOpts) ([]*template.SidecarOpts, error) {
 			DockerLabels: config.DockerLabels,
 			DependsOn:    config.DependsOn,
 			EntryPoint:   entrypoint,
+			HealthCheck:  convertContainerHealthCheck(config.HealthCheck),
 			Command:      command,
 		})
 	}
 	return sidecars, nil
 }
 
+func convertContainerHealthCheck(hc manifest.ContainerHealthCheck) *template.ContainerHealthCheck {
+	if hc.IsEmpty() {
+		return nil
+	}
+	// Make sure that unset fields in the healthcheck gets a default value.
+	hc.ApplyIfNotSet(manifest.NewDefaultContainerHealthCheck())
+	return &template.ContainerHealthCheck{
+		Command:     hc.Command,
+		Interval:    aws.Int64(int64(hc.Interval.Seconds())),
+		Retries:     aws.Int64(int64(aws.IntValue(hc.Retries))),
+		StartPeriod: aws.Int64(int64(hc.StartPeriod.Seconds())),
+		Timeout:     aws.Int64(int64(hc.Timeout.Seconds())),
+	}
+}
+
 // convertDependsOnStatus converts image and sidecar depends on fields to have upper case statuses
 func convertDependsOnStatus(s *convertSidecarOpts) {
 	if s.sidecarConfig != nil {

internal/pkg/deploy/cloudformation/stack/transformers_test.go

@@ -29,6 +29,7 @@ func Test_convertSidecar(t *testing.T) {
 		inDependsOn       map[string]string
 		inImg             manifest.Image
 		inImageOverride   manifest.ImageOverride
+		inHealthCheck     manifest.ContainerHealthCheck
 		circDepContainers []string
 
 		wanted    *template.SidecarOpts
@@ -241,6 +242,27 @@ func Test_convertSidecar(t *testing.T) {
 				Command:    []string{"arg1", "arg2"},
 			},
 		},
+		"with health check": {
+			inHealthCheck: manifest.ContainerHealthCheck{
+				Command: []string{"foo", "bar"},
+			},
+
+			wanted: &template.SidecarOpts{
+				Name:       aws.String("foo"),
+				CredsParam: mockCredsParam,
+				Image:      mockImage,
+				Secrets:    mockMap,
+				Variables:  mockMap,
+				Essential:  aws.Bool(false),
+				HealthCheck: &template.ContainerHealthCheck{
+					Command:     []string{"foo", "bar"},
+					Interval:    aws.Int64(10),
+					Retries:     aws.Int64(2),
+					StartPeriod: aws.Int64(0),
+					Timeout:     aws.Int64(5),
+				},
+			},
+		},
 	}
 	for name, tc := range testCases {
 		t.Run(name, func(t *testing.T) {
@@ -255,6 +277,7 @@ func Test_convertSidecar(t *testing.T) {
 					DockerLabels:  tc.inLabels,
 					DependsOn:     tc.inDependsOn,
 					ImageOverride: tc.inImageOverride,
+					HealthCheck:   tc.inHealthCheck,
 				},
 			}
 			got, err := convertSidecar(convertSidecarOpts{

internal/pkg/deploy/cloudformation/stack/worker_svc.go

@@ -124,7 +124,7 @@ func (s *WorkerService) Template() (string, error) {
 		DesiredCountOnSpot:       desiredCountOnSpot,
 		ExecuteCommand:           convertExecuteCommand(&s.manifest.ExecuteCommand),
 		WorkloadType:             manifest.WorkerServiceType,
-		HealthCheck:              s.manifest.WorkerServiceConfig.ImageConfig.HealthCheckOpts(),
+		HealthCheck:              convertContainerHealthCheck(s.manifest.WorkerServiceConfig.ImageConfig.HealthCheck),
 		LogConfig:                convertLogging(s.manifest.Logging),
 		DockerLabels:             s.manifest.ImageConfig.DockerLabels,
 		DesiredCountLambda:       desiredCountLambda.String(),

internal/pkg/deploy/cloudformation/stack/worker_svc_test.go

@@ -11,7 +11,6 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/cloudformation"
-	"github.com/aws/aws-sdk-go/service/ecs"
 	"github.com/aws/copilot-cli/internal/pkg/addon"
 	"github.com/aws/copilot-cli/internal/pkg/deploy/cloudformation/stack/mocks"
 	"github.com/aws/copilot-cli/internal/pkg/manifest"
@@ -201,8 +200,8 @@ Outputs:
 				m.EXPECT().Read(envControllerPath).Return(&template.Content{Buffer: bytes.NewBufferString("something")}, nil)
 				m.EXPECT().ParseWorkerService(template.WorkloadOpts{
 					WorkloadType: manifest.WorkerServiceType,
-					HealthCheck: &ecs.HealthCheck{
-						Command:     aws.StringSlice([]string{"CMD-SHELL", "curl -f http://localhost/ || exit 1"}),
+					HealthCheck: &template.ContainerHealthCheck{
+						Command:     []string{"CMD-SHELL", "curl -f http://localhost/ || exit 1"},
 						Interval:    aws.Int64(5),
 						Retries:     aws.Int64(3),
 						StartPeriod: aws.Int64(0),

internal/pkg/manifest/lb_web_svc_test.go

@@ -1248,7 +1248,7 @@ func Test_Temp(t *testing.T) {
 			},
 			Port: aws.Uint16(5000),
 		},
-		HealthCheck: *newDefaultContainerHealthCheck(),
+		HealthCheck: *NewDefaultContainerHealthCheck(),
 	}
 	t.Run("temporary", func(t *testing.T) {
 		// WHEN
@@ -1292,7 +1292,7 @@ func Test_Temp(t *testing.T) {
 							},
 							Port: aws.Uint16(5000),
 						},
-						HealthCheck: *newDefaultContainerHealthCheck(),
+						HealthCheck: *NewDefaultContainerHealthCheck(),
 					},
 				},
 			},

internal/pkg/manifest/validate.go

@@ -523,6 +523,9 @@ func (s *SidecarConfig) Validate() error {
 			return fmt.Errorf(`validate "mount_points[%d]: %w`, ind, err)
 		}
 	}
+	if err := s.HealthCheck.Validate(); err != nil {
+		return fmt.Errorf(`validate "healthcheck": %w`, err)
+	}
 	return s.ImageOverride.Validate()
 }
 

internal/pkg/manifest/workload.go

@@ -18,7 +18,6 @@ import (
 	"github.com/google/shlex"
 
 	"github.com/aws/aws-sdk-go/aws"
-	"github.com/aws/aws-sdk-go/service/ecs"
 	"gopkg.in/yaml.v3"
 )
 
@@ -405,15 +404,16 @@ func (lc *Logging) GetEnableMetadata() *string {
 
 // SidecarConfig represents the configurable options for setting up a sidecar container.
 type SidecarConfig struct {
-	Port          *string             `yaml:"port"`
-	Image         *string             `yaml:"image"`
-	Essential     *bool               `yaml:"essential"`
-	CredsParam    *string             `yaml:"credentialsParameter"`
-	Variables     map[string]string   `yaml:"variables"`
-	Secrets       map[string]string   `yaml:"secrets"`
-	MountPoints   []SidecarMountPoint `yaml:"mount_points"`
-	DockerLabels  map[string]string   `yaml:"labels"`
-	DependsOn     map[string]string   `yaml:"depends_on"`
+	Port          *string              `yaml:"port"`
+	Image         *string              `yaml:"image"`
+	Essential     *bool                `yaml:"essential"`
+	CredsParam    *string              `yaml:"credentialsParameter"`
+	Variables     map[string]string    `yaml:"variables"`
+	Secrets       map[string]string    `yaml:"secrets"`
+	MountPoints   []SidecarMountPoint  `yaml:"mount_points"`
+	DockerLabels  map[string]string    `yaml:"labels"`
+	DependsOn     map[string]string    `yaml:"depends_on"`
+	HealthCheck   ContainerHealthCheck `yaml:"healthcheck"`
 	ImageOverride `yaml:",inline"`
 }
 
@@ -558,9 +558,9 @@ type ContainerHealthCheck struct {
 	StartPeriod *time.Duration `yaml:"start_period"`
 }
 
-// newDefaultContainerHealthCheck returns container health check configuration
+// NewDefaultContainerHealthCheck returns container health check configuration
 // that's identical to a load balanced web service's defaults.
-func newDefaultContainerHealthCheck() *ContainerHealthCheck {
+func NewDefaultContainerHealthCheck() *ContainerHealthCheck {
 	return &ContainerHealthCheck{
 		Command:     []string{"CMD-SHELL", "curl -f http://localhost/ || exit 1"},
 		Interval:    durationp(10 * time.Second),
@@ -575,8 +575,8 @@ func (hc ContainerHealthCheck) IsEmpty() bool {
 	return hc.Command == nil && hc.Interval == nil && hc.Retries == nil && hc.Timeout == nil && hc.StartPeriod == nil
 }
 
-// applyIfNotSet changes the healthcheck's fields only if they were not set and the other healthcheck has them set.
-func (hc *ContainerHealthCheck) applyIfNotSet(other *ContainerHealthCheck) {
+// ApplyIfNotSet changes the healthcheck's fields only if they were not set and the other healthcheck has them set.
+func (hc *ContainerHealthCheck) ApplyIfNotSet(other *ContainerHealthCheck) {
 	if hc.Command == nil && other.Command != nil {
 		hc.Command = other.Command
 	}
@@ -594,33 +594,6 @@ func (hc *ContainerHealthCheck) applyIfNotSet(other *ContainerHealthCheck) {
 	}
 }
 
-func (hc *ContainerHealthCheck) healthCheckOpts() *ecs.HealthCheck {
-	// Make sure that unset fields in the healthcheck gets a default value.
-	hc.applyIfNotSet(newDefaultContainerHealthCheck())
-	return &ecs.HealthCheck{
-		Command:     aws.StringSlice(hc.Command),
-		Interval:    aws.Int64(int64(hc.Interval.Seconds())),
-		Retries:     aws.Int64(int64(*hc.Retries)),
-		StartPeriod: aws.Int64(int64(hc.StartPeriod.Seconds())),
-		Timeout:     aws.Int64(int64(hc.Timeout.Seconds())),
-	}
-}
-
-// HealthCheckOpts converts the image's healthcheck configuration into a format parsable by the templates pkg.
-func (i ImageWithPortAndHealthcheck) HealthCheckOpts() *ecs.HealthCheck {
-	if i.HealthCheck.IsEmpty() {
-		return nil
-	}
-	return i.HealthCheck.healthCheckOpts()
-}
-
-func (i ImageWithHealthcheck) HealthCheckOpts() *ecs.HealthCheck {
-	if i.HealthCheck.IsEmpty() {
-		return nil
-	}
-	return i.HealthCheck.healthCheckOpts()
-}
-
 // PlatformArgsOrString is a custom type which supports unmarshaling yaml which
 // can either be of type string or type PlatformArgs.
 type PlatformArgsOrString struct {

internal/pkg/template/template_functions.go

@@ -90,20 +90,6 @@ func QuoteSliceFunc(elems []string) []string {
 	return quotedElems
 }
 
-// QuotePSliceFunc places quotation marks around all
-// dereferenced elements of elems and returns a []string slice.
-func QuotePSliceFunc(elems []*string) []string {
-	var quotedElems []string
-	if len(elems) == 0 {
-		return quotedElems
-	}
-	quotedElems = make([]string, len(elems))
-	for i, el := range elems {
-		quotedElems[i] = strconv.Quote(*el)
-	}
-	return quotedElems
-}
-
 // generateMountPointJSON turns a list of MountPoint objects into a JSON string:
 // `{"myEFSVolume": "/var/www", "myEBSVolume": "/usr/data"}`
 // This function must be called on an array of correctly constructed MountPoint objects.

internal/pkg/template/template_functions_test.go

@@ -225,13 +225,6 @@ func TestQuoteSliceFunc(t *testing.T) {
 	}
 }
 
-func TestQuotePSliceFunc(t *testing.T) {
-	require.Equal(t, []string(nil), QuotePSliceFunc(nil))
-	require.Equal(t, []string(nil), QuotePSliceFunc([]*string{}))
-	require.Equal(t, []string{`"a"`}, QuotePSliceFunc(aws.StringSlice([]string{"a"})))
-	require.Equal(t, []string{`"a"`, `"b"`, `"c"`}, QuotePSliceFunc(aws.StringSlice([]string{"a", "b", "c"})))
-}
-
 func TestGenerateMountPointJSON(t *testing.T) {
 	require.Equal(t, `{"myEFSVolume":"/var/www"}`, generateMountPointJSON([]*MountPoint{{ContainerPath: aws.String("/var/www"), SourceVolume: aws.String("myEFSVolume")}}), "JSON should render correctly")
 	require.Equal(t, "{}", generateMountPointJSON([]*MountPoint{}), "nil list of arguments should render ")

internal/pkg/template/templates/workloads/partials/cf/sidecars.yml

@@ -16,12 +16,26 @@
 {{- end}}
 {{- range $sidecar := .Sidecars}}
 - Name: {{$sidecar.Name}}
-  Image: {{$sidecar.Image}}{{if $sidecar.Essential}}
-  Essential: {{$sidecar.Essential}}{{end}}{{if $sidecar.Port}}
+  Image: {{$sidecar.Image}}
+{{- if $sidecar.Essential}}
+  Essential: {{$sidecar.Essential}}
+{{- end}}
+{{- if $sidecar.Port}}
 {{include "image-overrides" . | indent 2}}
   PortMappings:
-    - ContainerPort: {{$sidecar.Port}}{{if $sidecar.Protocol}}
-      Protocol: {{$sidecar.Protocol}}{{end}}{{end}}
+    - ContainerPort: {{$sidecar.Port}}
+    {{- if $sidecar.Protocol}}
+      Protocol: {{$sidecar.Protocol}}
+    {{- end}}
+{{- end}}
+{{- if $sidecar.HealthCheck}}
+  HealthCheck:
+    Command: {{quoteSlice $sidecar.HealthCheck.Command | fmtSlice}}
+    Interval: {{$sidecar.HealthCheck.Interval}}
+    Retries: {{$sidecar.HealthCheck.Retries}}
+    StartPeriod: {{$sidecar.HealthCheck.StartPeriod}}
+    Timeout: {{$sidecar.HealthCheck.Timeout}}
+{{- end}}
 {{- if $sidecar.Variables}}
   Environment:
   {{- range $name, $value := $sidecar.Variables}}