Conversation
Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #6692 +/- ##
==========================================
- Coverage 29.50% 29.33% -0.18%
==========================================
Files 593 598 +5
Lines 63390 63774 +384
==========================================
+ Hits 18702 18706 +4
- Misses 43243 43624 +381
+ Partials 1445 1444 -1
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:
|
| if err != nil { | ||
| return nil, nil, fmt.Errorf("failed to register piped %q (%s): %w", p.Name, p.Id, err) | ||
| } |
There was a problem hiding this comment.
Not bug but why registering pipeds exit when an error occured
while registering applications keeps adding and return number of success and fail restoration
There was a problem hiding this comment.
For example if there are 10 pipeds need to be restored, if it fail early then we need to add remain ones manually
P1
P2
P3: fail
P4: need manual adding
....
P10: need manual adding
I know this problem can be easily solved by deleting P1, P2, P3 from backup file and reapply the restore command but I still think that continuing even fail is better
There was a problem hiding this comment.
@armistcxy The number of piped is always smaller compare to the number of applications, thus I prefer to retry whole things (after delete P1->P3 on UI)
Another reason is the output of the pipectl transfer restore piped command is a piped mapping file, which will be used by the restore application step. I don't want to force users to manually update/add piped mapping to that file in order to make restore application step work. So, fail fast and delete P1->P3, then re-do from start is recommended.
armistcxy
left a comment
armistcxy
left a comment
There was a problem hiding this comment.
Overall LGTM but there're some concerns
| mappings = append(mappings, PipedMapping{ | ||
| OldPipedID: p.Id, | ||
| NewPipedID: resp.Id, | ||
| NewKey: resp.Key, | ||
| PipedName: p.Name, | ||
| }) |
There was a problem hiding this comment.
oh, I almost forget to mention that should we return Base64 encoded key here for easy copy paste
when testing this feature, I need to run "echo | base64" and copy, I would love to return the base64 encode as the UI return
There was a problem hiding this comment.
Good point, let me address that 👍
2 participants
What this PR does:
Introduce
pipectl transfercommand, this command will be used to migrate pipecd applications and pipeds from a source controlplane to a target controlplane. Useful for migrating control plane without manually recreating all applications/pipelines, everything will keep working as is after migration.This transfer command contains 2 subcommands
Note:
Why we need it:
Which issue(s) this PR fixes:
Part of #6691
Does this PR introduce a user-facing change?: