Skip to content

Commit

Permalink
Extend ecs.TaskDefinition containerDefinitions normalization (#4126)
Browse files Browse the repository at this point in the history
Fixes #1738 
Fixes #1985

containerDefinitions expects a raw JSON which may lead to spurious diff
on insignificant changes. The was pre-existing code for normalizing
environment variable order and default values. This code is patched to
extend it to handle normalizing the default value of
healthCheck.timeout=5.

---------

Co-authored-by: Florian Stadler <[email protected]>
  • Loading branch information
t0yv0 and flostadler authored Jun 28, 2024
1 parent 706903b commit 896ae14
Show file tree
Hide file tree
Showing 2 changed files with 175 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Anton Tayanovskyy <[email protected]>
Date: Thu, 27 Jun 2024 16:20:44 -0400
Subject: [PATCH] Normalize ecs.TaskDefinition containerDefinitions
healthCheck.timeout


diff --git a/internal/service/ecs/task_definition_equivalency.go b/internal/service/ecs/task_definition_equivalency.go
index 3d757997b5..efd649fbdc 100644
--- a/internal/service/ecs/task_definition_equivalency.go
+++ b/internal/service/ecs/task_definition_equivalency.go
@@ -72,6 +72,10 @@ func (cd containerDefinitions) Reduce(isAWSVPC bool) error {
if def.Essential == nil {
def.Essential = aws.Bool(true)
}
+ if def.HealthCheck != nil && def.HealthCheck.Timeout == nil {
+ five := int64(5)
+ def.HealthCheck.Timeout = &five
+ }
for j, pm := range def.PortMappings {
if pm.Protocol != nil && aws.StringValue(pm.Protocol) == "tcp" {
cd[i].PortMappings[j].Protocol = nil
153 changes: 153 additions & 0 deletions provider/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@
package provider

import (
"encoding/json"
"fmt"
"testing"

"github.com/pulumi/pulumi/sdk/v3/go/common/util/contract"
)

// Check that having manifest...retentionDays as "3650" in the state but 3650 (numeric value) in the program does not
Expand Down Expand Up @@ -60,3 +64,152 @@ func TestRegressLandingZoneDiff(t *testing.T) {
}]`
replaySequence(t, event)
}

func TestRegress1738(t *testing.T) {
containerDefinitionsOld := `
[
{
"cpu": 512,
"environment": [],
"essential": true,
"healthCheck": {
"command": [
"CMD-SHELL",
"curl -f http://localhost:8080/health || exit 1"
],
"interval": 5,
"retries": 10,
"timeout": 5
},
"image": "nginx",
"logConfiguration": {
"logDriver": "awslogs",
"options": {
"awslogs-group": "foo-bar-e196c99",
"awslogs-region": "us-east-1",
"awslogs-stream-prefix": "nginx"
}
},
"memory": 2048,
"mountPoints": [],
"name": "nginx",
"portMappings": [],
"startTimeout": 10,
"systemControls": [],
"volumesFrom": []
}
]`

containerDefinitionsNew := `
[
{
"cpu": 512,
"environment": [],
"healthCheck": {
"command": [
"CMD-SHELL",
"curl -f http://localhost:8080/health || exit 1"
],
"interval": 5,
"retries": 10
},
"image": "nginx",
"memory": 2048,
"name": "nginx",
"startTimeout": 10,
"logConfiguration": {
"logDriver": "awslogs",
"options": {
"awslogs-group": "foo-bar-e196c99",
"awslogs-region": "us-east-1",
"awslogs-stream-prefix": "nginx"
}
}
}
]`

j := func(x any) string {
bytes, err := json.Marshal(x)
contract.AssertNoErrorf(err, "json.Marshal failure")
return string(bytes)
}

replaySequence(t, fmt.Sprintf(`
[{
"method": "/pulumirpc.ResourceProvider/Diff",
"request": {
"id": "foo-bar-c7f12716",
"urn": "urn:pulumi:dev::repro::awsx:ecs:FargateService$awsx:ecs:FargateTaskDefinition$aws:ecs/taskDefinition:TaskDefinition::foo-bar",
"olds": {
"__meta": "{\"schema_version\":\"1\"}",
"arn": "arn:aws:ecs:us-east-1:616138583583:task-definition/foo-bar-c7f12716:1",
"arnWithoutRevision": "arn:aws:ecs:us-east-1:616138583583:task-definition/foo-bar-c7f12716",
"containerDefinitions": %s,
"cpu": "512",
"ephemeralStorage": null,
"executionRoleArn": "arn:aws:iam::616138583583:role/foo-bar-execution-694a131",
"family": "foo-bar-c7f12716",
"id": "foo-bar-c7f12716",
"inferenceAccelerators": [],
"ipcMode": "",
"memory": "2048",
"networkMode": "awsvpc",
"pidMode": "",
"placementConstraints": [],
"proxyConfiguration": null,
"requiresCompatibilities": [
"FARGATE"
],
"revision": 1,
"runtimePlatform": null,
"skipDestroy": false,
"tags": {},
"tagsAll": {},
"taskRoleArn": "arn:aws:iam::616138583583:role/foo-bar-task-77ab295",
"trackLatest": false,
"volumes": []
},
"news": {
"__defaults": [
"skipDestroy",
"trackLatest"
],
"containerDefinitions": %s,
"cpu": "512",
"executionRoleArn": "arn:aws:iam::616138583583:role/foo-bar-execution-694a131",
"family": "foo-bar-c7f12716",
"memory": "2048",
"networkMode": "awsvpc",
"requiresCompatibilities": [
"FARGATE"
],
"skipDestroy": false,
"taskRoleArn": "arn:aws:iam::616138583583:role/foo-bar-task-77ab295",
"trackLatest": false
},
"oldInputs": {
"__defaults": [
"skipDestroy",
"trackLatest"
],
"containerDefinitions": %s,
"cpu": "512",
"executionRoleArn": "arn:aws:iam::616138583583:role/foo-bar-execution-694a131",
"family": "foo-bar-c7f12716",
"memory": "2048",
"networkMode": "awsvpc",
"requiresCompatibilities": [
"FARGATE"
],
"skipDestroy": false,
"taskRoleArn": "arn:aws:iam::616138583583:role/foo-bar-task-77ab295",
"trackLatest": false
}
},
"response": {
"changes": "DIFF_NONE",
"stables": "*",
"hasDetailedDiff": true
}
}]`, j(containerDefinitionsOld), j(containerDefinitionsNew), j(containerDefinitionsNew)))
}

0 comments on commit 896ae14

Please sign in to comment.