fix (ecs): Unstable deployments for task definitions with more than one container (#5544)

[atyutyunnik]

The original code will not finish deployment and will eventually time out, in spite of task being successfully deployed by ECS and running (spinnaker/spinnaker#5544)

        "containers": [
            {
                "networkBindings": [],
                "networkInterfaces": [],
            },
            {
                "lastStatus": "RUNNING",
                "networkBindings": [
                    {
                        "bindIP": "0.0.0.0",
                        "containerPort": 8080,
                        "hostPort": 32773,
                        "protocol": "tcp"
                    }
            }]

[ezimanyi]

@atyutyunnik : Thanks for the PR! In general our process is to open fixes against the master branch then cherry-pick them back to release that need them. Can you please open this agains the master branch? Thanks!

[atyutyunnik]

ezimanyi, yes, I already have (see PR 4409), but it's not source-compatible with release 1.18. Hence an additional PR

[ezimanyi]

Oh, thanks, I mis-understood! If the code has changed enough that it can't be cleanly cherry-picked to 1.18, then it's fine to open this against 1.18 to manually back-port the fix.

It will be important to cherry-pick the master change to 1.19 though, as otherwise users upgrading from 1.18 to 1.19 will get re-broken.

[allisaurus]

@atyutyunnik can you please add the unit test from #4409 to this PR as well?

[atyutyunnik]

@atyutyunnik can you please add the unit test from #4409 to this PR as well?

@allisaurus, cherry-picking doesn't seem possible. I had to extend the unit-test with:
...
then: amazonloadBalancing.describeTargetHealth({ DescribeTargetHealthRequest request ->
...

otherwise, there would be an NPE at 332 of TaskHealthCachingAgent.java:

if (describeTargetHealthResult.getTargetHealthDescriptions().isEmpty()) {

Now the unit-test is passing - please review

[allisaurus]

Thanks @atyutyunnik , you're right, there was a behavior change between 1.18.x and 1.19.x that makes mocking the call to describeTargetHealth necessary here. With that addition this LGTM!