Update deployment trigger sync strategy based on planned step output#6274
Conversation
Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #6274 +/- ##
=======================================
Coverage 28.78% 28.78%
=======================================
Files 560 560
Lines 59813 59816 +3
=======================================
+ Hits 17215 17216 +1
- Misses 41279 41281 +2
Partials 1319 1319
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
t-kikuc
left a comment
There was a problem hiding this comment.
almost LGTM, let me confirm one point
|
|
||
| req := &pipedservice.ReportDeploymentPlannedRequest{ | ||
| DeploymentId: p.deployment.Id, | ||
| SyncStrategy: out.SyncStrategy, |
There was a problem hiding this comment.
[ASK] I think pipedv0 needs the same logic, is that correct?
There was a problem hiding this comment.
Not exactly. Pipedv0 can also have this logic, but the current pipedv0 stage executing steps logic doesn't depend on the deployment sync strategy value, so the same issue we faced on pipedv1 has not occurred.
There was a problem hiding this comment.
Aka. issue #6207 only occurred on the pipedv1 application deployment.
What this PR does:
SSIA
Why we need it:
If users trigger deployment under the AUTO strategy, the value stored in
model.deployment.trigger.sync_strategyremains as is while executing the deployment. That makes later logic depend on the deployment's strategy value work unexpectedly, since we only support QUICK_SYNC and PIPELINE_SYNC. The AUTO strategy will be inferred to one of QUICK or PIPELINE sync by the planning step; thus, the value in the deployment model should be updated to reflect the inferred output.Which issue(s) this PR fixes:
Fixes #6207
Does this PR introduce a user-facing change?: