feat(ecs-patterns): adding support for custom HealthCheck in NetworkLoadBalancedFargateService

[daschaa]

Fixes #20233

Adds healthCheck property to NetworkLoadBalancedFargateService.

---

All Submissions:

• Have you followed the guidelines in our Contributing guide?

Adding new Unconventional Dependencies:

• This PR adds new unconventional dependencies following the process described here

New Features

• Have you added the new feature to an integration test?
  • Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[gitpod-io]

Pull request has been modified.

[daschaa]

@corymhall Thanks for your (as always) valuable feedback! I added more explanation to the docstring, I hope it is better now.
For the Readme I removed the property, but I don't know where to put a section about the health check. Maybe it is better to leave it out of the README and instead make the docstrings as good as possible? What do you think?

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 76b9634
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[corymhall]

After thinking about this more, I'm not sure we should expose this on the top level props. For an ECS service there behind a load balancer there are two different types of health checks that you can use.

1. ELB health checks (configured on the NLB in this case). The NLB will make a request to the service on a specific path/port and expect a certain response.
2. Container health checks (what this PR exposes). The service itself can perform health checks against the running containers (in addition to the ELB health checks).

I'm not sure this is a common use case for services running behind an ELB. The other linked PR that implemented this for the QueueProcessing service makes more sense because it is a worker service that is not running behind an ELB so container health checks are the only type of health check available.

I think in the majority of cases the user wants to configure the ELB health check and exposing this at the top level (an not the elb health check) would lead to more confusion and users configuring container health checks when they don't need to.

cc @madeline-k for a second opinion.

[daschaa]

@corymhall Ok I think this makes sense. Maybe we should close this (and the linked issue) and come up with a solution if there is a real need for this. And even then explicitly document the consequences.

And I want to apologize for implementing this feature without really thinking about it. I will try to go more in-depth in a topic I will implement in the future.