Fix removal of existing cloudrun revision tags when deploying#5900
Fix removal of existing cloudrun revision tags when deploying#5900niladrix719 wants to merge 7 commits into
Conversation
Signed-off-by: “niladrix719” <niladrix719@gmail.com>
|
@niladrix719 |
|
/check-commit f944fdff91e0f94d15656571aa3dc07df9b40949c4e8e8850fbf09d3a301d105 |
|
@niladrix719 At first, I recommend you split into functions and add tests. |
Signed-off-by: “niladrix719” <niladrix719@gmail.com>
|
Test cases and splitting function are done. Let me know if there is anything else |
|
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
|
Let's me hold this one and try add this while implementing the CloudRun plugin 👀 |
|
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
|
This PR was closed because it has been stalled for 7 days with no activity. Feel free to reopen if still applicable. |
I have investigated about the problem,
pipecd/pkg/app/piped/platformprovider/cloudrun/client.go
Lines 94 to 96 in c1ca195
The issue was that when calling
svc.ReplaceService(name, svcCfg), the tags associated with revisions in the traffic configuration were being lost.To fix this, we can retrieve the current service configuration, extracts the existing revision tags from its traffic configuration, and then copy these tags to the corresponding revisions in the new service configuration before calling ReplaceService.
But still, the tags of outdated revisions would be removed, as we are only keeping track of the new revision and the last deployed revision.
pipecd/pkg/app/piped/executor/cloudrun/deploy.go
Lines 188 to 197 in c1ca195
To solve this, we can mark the outdated revisions that have tags with 0% traffic, so it can replicate the behavior seen in Cloud Run
What this PR does:
Why we need it:
Revision tags created by the user should not be removed
Which issue(s) this PR fixes:
Fixes #4954
Does this PR introduce a user-facing change?: No