fix: prioritize oldest uncompleted deployment for head selection - #7278
fix: prioritize oldest uncompleted deployment for head selection#7278Vishakha7-Kumari wants to merge 1 commit into
Conversation
✅ Deploy Preview for pipecd-site canceled.
|
|
👋 Hi @Vishakha7-Kumari, welcome to PipeCD and thanks for opening your first pull request! We’re really happy to have you here Before your PR gets merged, please check a few important things below. Helpful resources
DCO Sign-offAll commits must include a In case you forget to sign-off your commit(s), follow these steps: For the last commit: git commit --amend --signoff
git push --force-with-leaseFor multiple commits: git rebase --signoff origin/master
git push --force-with-leaseRun checks locallyBefore pushing updates, please run: make checkThis runs the same checks as CI and helps catch issues early. 💬 Need help?If anything is unclear, feel free to ask in this PR or join us on the CNCF Slack in the #pipecd channel. Thanks for contributing to PipeCD! ❤️ |
Signed-off-by: Vishakha7-Kumari <singhdaisy669@gmail.com>
8535219 to
8800dc3
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes pipedv1’s deployment-store head selection to align with the ListAppHeadDeployments() contract (“head = oldest uncompleted deployment”) by selecting per-application heads based on CreatedAt rather than overwrite order.
Changes:
- Update
sync()head selection to keep the oldest deployment per application by comparingCreatedAt. - Add unit tests covering cases where the running vs pending deployment is the oldest for the same application.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/app/pipedv1/apistore/deploymentstore/store.go | Adjusts head-deployment selection logic to prefer the oldest uncompleted deployment using CreatedAt. |
| pkg/app/pipedv1/apistore/deploymentstore/store_test.go | Adds test coverage for selecting the oldest head deployment across mixed pending/running states. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| updateHead := func(d *model.Deployment) { | ||
| if existing, ok := headDeployments[d.ApplicationId]; !ok || d.CreatedAt < existing.CreatedAt { | ||
| headDeployments[d.ApplicationId] = d | ||
| } | ||
| } |
What this PR does / Why we need it
This PR fixes a bug in the synchronization logic of the
pipedv1deployment store.Currently,
syncbuilds theheadDeploymentsmap by iterating sequentially over pending, planned, and running deployments. As a result, running deployments (or newer deployments processed later in the sequence) always overwrite other uncompleted ones. This contradicts the contract defined inListAppHeadDeployments():To fix this, we introduce an
updateHeadhelper which compares theCreatedAttimestamps of the deployments. It only populates or updatesheadDeploymentsif a candidate deployment has a smallerCreatedAttimestamp (i.e. is older), guaranteeing that the oldest uncompleted deployment is always selected as the head.Changes
CreatedAt.Fixes #6780