Skip to content

Conversation

@khanhtc1202
Copy link
Member

@khanhtc1202 khanhtc1202 commented Nov 6, 2025

What this PR does:

Add ECS QuickSync forceNewDeployment config. The configuration would be like:

apiVersion: pipecd.dev/v1beta1
kind: ECSApp
spec:
  quickSync:
    forceNewDeployment: true

Why we need it:

It requires forcing a new deployment when updating the ECS service configuration from LaunchType to Capacity provider strategy. So, my plan is to support switching from launch type to capacity provider strategy on existing service by enabling force new deployment on the updated service in case of ECS quick sync deployment triggered.

Error message

InvalidParameterException: When switching from launch type to capacity provider strategy on an existing service, or making a change to a capacity provider strategy on a service that is already using one, you must force a new deployment.

Which issue(s) this PR fixes:

Follow PR #6331

Does this PR introduce a user-facing change?:

  • How are users affected by this change:
  • Is this breaking change:
  • How to migrate (if breaking change):

Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>
@khanhtc1202 khanhtc1202 requested a review from a team as a code owner November 6, 2025 07:30
@codecov
Copy link

codecov bot commented Nov 6, 2025

Codecov Report

❌ Patch coverage is 0% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 28.81%. Comparing base (1db6e9b) to head (025460c).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
pkg/app/piped/executor/ecs/ecs.go 0.00% 10 Missing ⚠️
pkg/app/piped/executor/ecs/deploy.go 0.00% 6 Missing ⚠️
pkg/app/piped/executor/ecs/rollback.go 0.00% 6 Missing ⚠️
pkg/app/piped/platformprovider/ecs/client.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6336      +/-   ##
==========================================
- Coverage   28.83%   28.81%   -0.02%     
==========================================
  Files         560      560              
  Lines       59922    59937      +15     
==========================================
- Hits        17277    17271       -6     
- Misses      41324    41345      +21     
  Partials     1321     1321              
Flag Coverage Δ
. 23.26% <0.00%> (-0.03%) ⬇️
.-pkg-app-pipedv1-plugin-analysis 32.64% <ø> (ø)
.-pkg-app-pipedv1-plugin-kubernetes 58.54% <ø> (ø)
.-pkg-app-pipedv1-plugin-kubernetes_multicluster 67.63% <ø> (ø)
.-pkg-app-pipedv1-plugin-scriptrun 54.83% <ø> (ø)
.-pkg-app-pipedv1-plugin-terraform 38.65% <ø> (ø)
.-pkg-app-pipedv1-plugin-wait 33.92% <ø> (ø)
.-pkg-app-pipedv1-plugin-waitapproval 52.71% <ø> (ø)
.-pkg-plugin-sdk 50.34% <ø> (ø)
.-tool-actions-gh-release 19.23% <ø> (ø)
.-tool-actions-plan-preview 25.51% <ø> (ø)
.-tool-codegen-protoc-gen-auth 0.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

t-kikuc
t-kikuc previously approved these changes Nov 7, 2025
Copy link
Member

@t-kikuc t-kikuc left a comment

Choose a reason for hiding this comment

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

LGTM, let's try

forceNewDeployment := e.appCfg.QuickSync.ForceNewDeployment

// Store force new deployment flag to metadata store.
e.Input.MetadataStore.Shared().Put(ctx, "force-new-deployment", strconv.FormatBool(forceNewDeployment))
Copy link
Member

Choose a reason for hiding this comment

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

nit: use a const for the key

Copy link
Member Author

Choose a reason for hiding this comment

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

@t-kikuc Thanks, fixed by 025460c. Please re-approve 🙏

Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>
@khanhtc1202 khanhtc1202 enabled auto-merge (squash) November 7, 2025 03:32
@khanhtc1202 khanhtc1202 merged commit cac601e into master Nov 7, 2025
46 checks passed
@khanhtc1202 khanhtc1202 deleted the ecs-support-force-new-deployment branch November 7, 2025 03:32
khanhtc1202 added a commit that referenced this pull request Dec 23, 2025
khanhtc1202 added a commit that referenced this pull request Dec 23, 2025
This reverts commit cac601e.

Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>
khanhtc1202 added a commit that referenced this pull request Dec 23, 2025
…6384)

This reverts commit cac601e.

Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>
@github-actions github-actions bot mentioned this pull request Dec 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants