Skip to content

fix: Move ECS CF Generation to Calamari#1971

Merged
Jtango18 merged 80 commits into
mainfrom
jt-ecs-template-builder
Jun 3, 2026
Merged

fix: Move ECS CF Generation to Calamari#1971
Jtango18 merged 80 commits into
mainfrom
jt-ecs-template-builder

Conversation

@Jtango18
Copy link
Copy Markdown
Contributor

⚠️ Does this change require a corresponding Server Change?
⚠️ If so - please add a "Requires Server Change" label to this PR!

@Jtango18 Jtango18 force-pushed the jt-ecs-template-builder branch 2 times, most recently from c43cddd to d639db3 Compare May 29, 2026 00:33
@Jtango18 Jtango18 force-pushed the jt-ecs-template-builder branch from 698f61e to a5f739d Compare June 1, 2026 03:49
@Jtango18 Jtango18 requested review from APErebus, sathvikkumar-octo and zentron and removed request for sathvikkumar-octo June 1, 2026 04:58
@Jtango18 Jtango18 force-pushed the jt-ecs-template-builder branch 2 times, most recently from bbfd24c to 513f63e Compare June 1, 2026 23:45
@Jtango18 Jtango18 force-pushed the jt-ecs-template-builder branch from 513f63e to 31f24c5 Compare June 2, 2026 00:52
New implementation consumes they existing Deploy CF Template command and we test all the other pieces around that. This test provides very little additional value
Comment thread source/Calamari.Aws/Integration/Ecs/Deploy/Cfn/ContainerDefinition.cs Outdated
Comment thread source/Calamari.Aws/Inputs/Ecs/DeployEcsCommandInputs.cs Outdated

[TestFixture]
[Category(TestCategory.RunOnceOnWindowsAndLinux)]
public class DeployEcsServiceFixture
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair bit of work to update this but I recall this was the only test that hit AWS at all so we have lost some coverage

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It tested "Does out code create a CF template when we call it". We have a test already that does that, the specifics for this are around the uniqueness of a an ECS template which is all tested and validated elsewhere.

Comment thread source/Calamari.Aws/Inputs/Ecs/ContainerSpecMappingExtensions.cs Outdated
Comment thread source/Calamari.Aws/Inputs/Ecs/ContainerSpecMappingExtensions.cs Outdated
Comment thread source/Calamari.Aws/Inputs/Ecs/ContainerSpecMappingExtensions.cs
Comment thread source/Calamari.Aws/Inputs/Ecs/ContainerSpecMappingExtensions.cs Outdated
Comment thread source/Calamari.Common/Plumbing/Variables/VariablesDeserialisationExtensions.cs Outdated
@Jtango18 Jtango18 marked this pull request as ready for review June 3, 2026 01:09
@Jtango18 Jtango18 enabled auto-merge (squash) June 3, 2026 01:15
Copy link
Copy Markdown
Contributor

@APErebus APErebus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thanks for addressing the comments.

@Jtango18 Jtango18 merged commit 0227054 into main Jun 3, 2026
35 checks passed
@Jtango18 Jtango18 deleted the jt-ecs-template-builder branch June 3, 2026 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants