✨ clusterctl: Add move --to-directory and --from-directory flags #7005

oscr · 2022-08-02T16:35:55Z

What this PR does / why we need it:

Changes based on #6992

Adds deprecated warning to clusterctl restore and clusterctl backup. Example

$ clusterctl backup --directory ~/temp/clusterctl-backup/
WARNING: 'clusterctl backup' is deprecated and will be removed in a future release. Use 'clusterctl move' instead.
Performing backup...
Discovering Cluster API objects
Starting backup of Cluster API objects Clusters=1
Moving Cluster API objects ClusterClasses=1
Saving files to /home/oscar/temp/clusterctl-backup/

Adds clusterctl move flags --to-folder and --from-folder. Example

$ clusterctl move --to-folder --directory ~/temp/clusterctl-backup/
Performing backup...
Discovering Cluster API objects
Starting backup of Cluster API objects Clusters=1
Moving Cluster API objects ClusterClasses=1
Saving files to /home/oscar/temp/clusterctl-backup/

I also added they should mutually exclusive with each other.

$ clusterctl move --from-folder --to-folder
Error: if any flags in the group [from-folder to-folder] are set none of the others can be; [from-folder to-folder] were all set

$ clusterctl move --from-folder --kubeconfig=foo
Error: if any flags in the group [from-folder kubeconfig] are set none of the others can be; [from-folder kubeconfig] were all set

$ clusterctl move --to-folder --to-kubeconfig=foo
Error: if any flags in the group [to-folder to-kubeconfig] are set none of the others can be; [to-folder to-kubeconfig] were all set

Since dry-run exists for move but not for restore and backup I added logic to not perform any actions if it's set.

$ clusterctl move --to-folder --dry-run
Error: please run --to-folder without --dry-run flag
$ clusterctl move --from-folder --dry-run
Error: please run --from-folder without dry-run flag

Which issue(s) this PR fixes:
Fixes #6992

k8s-ci-robot · 2022-08-02T16:35:57Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

k8s-ci-robot · 2022-08-02T16:36:03Z

@oscr: This issue is currently awaiting triage.

If CAPI contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ykakarap

Just doing a high level review since this is a draft PR.

We consider adding a note in the documentation about the deprecation.

cmd/clusterctl/cmd/backup.go

fabriziopandini

Overall lgtm.
Deprecation should surface in https://main.cluster-api.sigs.k8s.io/developer/providers/v1.2-to-v1.3.html

cmd/clusterctl/cmd/backup.go

cmd/clusterctl/cmd/restore.go

cmd/clusterctl/cmd/move.go

cmd/clusterctl/client/move.go

cmd/clusterctl/cmd/move.go

cmd/clusterctl/client/move.go

cmd/clusterctl/cmd/move.go

oscr · 2022-08-16T15:19:58Z

A general question: right now the flags are called --to-folder and --from-folder but the folder is specified with --directory (like in restore and backup). Should we maybe change the names to be consistent?

sbueringer · 2022-08-16T17:33:31Z

A general question: right now the flags are called --to-folder and --from-folder but the folder is specified with --directory (like in restore and backup). Should we maybe change the names to be consistent?

+1 to using directory instead. We also use directory in the topology dry run cmd

ykakarap

/lgtm

fabriziopandini · 2022-09-21T19:57:03Z

looking forward to lgtm as soon as lint error is fixed
great work!

oscr · 2022-09-21T21:35:47Z

cmd/clusterctl/client/cluster/template_test.go

@@ -310,7 +310,7 @@ func Test_templateClient_GetFromURL(t *testing.T) {
 	// redirect stdin
 	saveStdin := os.Stdin
 	defer func() { os.Stdin = saveStdin }()
-	os.Stdin, err = os.Open(path)
+	os.Stdin, err = os.Open(path) //nolint:gosec


I was surprised to see the gosec linter fail on this line since it wasn't changed in the pr. I nolint'ed it since it's a test anyway. But it's odd

These flags perform the same actions clusterctl backup and restore commands. Also adds deprecation warning to clusterctl backup and restore.

k8s-ci-robot · 2022-09-21T22:42:49Z

@oscr: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cluster-api-apidiff-main	`44f9fd9`	link	false	`/test pull-cluster-api-apidiff-main`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

fabriziopandini · 2022-09-22T19:57:12Z

/lgtm

ykakarap

/lgtm

fabriziopandini · 2022-09-23T09:20:46Z

/approve
Yay!

k8s-ci-robot · 2022-09-23T09:20:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [fabriziopandini]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

oscr · 2022-09-23T09:28:11Z

@ykakarap @fabriziopandini I would like to thank you both for all the help and feedback with this pr. I am grateful for your help 🙏

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Aug 2, 2022

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 2, 2022

k8s-ci-robot requested review from JoelSpeed and stmcginnis August 2, 2022 16:36

ykakarap reviewed Aug 4, 2022

View reviewed changes

cmd/clusterctl/cmd/backup.go Outdated Show resolved Hide resolved

oscr force-pushed the clusterctl-add-move-flags branch 2 times, most recently from 12cf02c to dd1c76a Compare August 4, 2022 08:56

fabriziopandini reviewed Aug 4, 2022

View reviewed changes

cmd/clusterctl/cmd/backup.go Outdated Show resolved Hide resolved

cmd/clusterctl/cmd/restore.go Outdated Show resolved Hide resolved

fabriziopandini reviewed Aug 4, 2022

View reviewed changes

cmd/clusterctl/cmd/move.go Outdated Show resolved Hide resolved

oscr force-pushed the clusterctl-add-move-flags branch from dd1c76a to 780af74 Compare August 4, 2022 15:06

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 4, 2022

oscr requested review from ykakarap and fabriziopandini August 4, 2022 15:21

ykakarap reviewed Aug 10, 2022

View reviewed changes

cmd/clusterctl/cmd/move.go Outdated Show resolved Hide resolved

cmd/clusterctl/cmd/move.go Outdated Show resolved Hide resolved

oscr force-pushed the clusterctl-add-move-flags branch from 780af74 to d8cfe70 Compare August 10, 2022 12:24

oscr marked this pull request as ready for review August 10, 2022 12:25

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 10, 2022

k8s-ci-robot requested a review from killianmuldoon August 10, 2022 12:25

ykakarap reviewed Aug 10, 2022

View reviewed changes

cmd/clusterctl/client/move.go Outdated Show resolved Hide resolved

oscr force-pushed the clusterctl-add-move-flags branch from d8cfe70 to 1d2c370 Compare August 11, 2022 16:24

ykakarap reviewed Aug 16, 2022

View reviewed changes

oscr force-pushed the clusterctl-add-move-flags branch from 1d2c370 to d496a18 Compare August 17, 2022 11:54

oscr changed the title ~~✨ clusterctl: Add move --to-folder and --from-folder flags~~ ✨ clusterctl: Add move --to-directory and --from-directory flags Aug 17, 2022

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 17, 2022

oscr force-pushed the clusterctl-add-move-flags branch from 82d35a2 to 7a18359 Compare September 20, 2022 19:25

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 20, 2022

ykakarap approved these changes Sep 21, 2022

View reviewed changes

k8s-ci-robot assigned ykakarap Sep 21, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 21, 2022

oscr force-pushed the clusterctl-add-move-flags branch from 7a18359 to c798ad1 Compare September 21, 2022 20:52

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Sep 21, 2022

oscr force-pushed the clusterctl-add-move-flags branch from c798ad1 to 37b7e73 Compare September 21, 2022 21:06

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 21, 2022

oscr force-pushed the clusterctl-add-move-flags branch from 37b7e73 to c52c421 Compare September 21, 2022 21:29

oscr commented Sep 21, 2022

View reviewed changes

clusterctl: Add move --to-directory and --from-directory flags

44f9fd9

These flags perform the same actions clusterctl backup and restore commands. Also adds deprecation warning to clusterctl backup and restore.

oscr force-pushed the clusterctl-add-move-flags branch from c52c421 to 44f9fd9 Compare September 21, 2022 22:39

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 22, 2022

ykakarap approved these changes Sep 23, 2022

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 23, 2022

k8s-ci-robot merged commit 186dd09 into kubernetes-sigs:main Sep 23, 2022

k8s-ci-robot added this to the v1.3 milestone Sep 23, 2022

oscr deleted the clusterctl-add-move-flags branch September 23, 2022 09:40

fabriziopandini mentioned this pull request Nov 28, 2022

clusterctl backup does not backup worker node CRs #7625

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ clusterctl: Add move --to-directory and --from-directory flags #7005

✨ clusterctl: Add move --to-directory and --from-directory flags #7005

oscr commented Aug 2, 2022

k8s-ci-robot commented Aug 2, 2022

k8s-ci-robot commented Aug 2, 2022

ykakarap left a comment

fabriziopandini left a comment

oscr commented Aug 16, 2022

sbueringer commented Aug 16, 2022

ykakarap left a comment

fabriziopandini commented Sep 21, 2022

oscr Sep 21, 2022

k8s-ci-robot commented Sep 21, 2022

fabriziopandini commented Sep 22, 2022

ykakarap left a comment

fabriziopandini commented Sep 23, 2022

k8s-ci-robot commented Sep 23, 2022

oscr commented Sep 23, 2022

✨ clusterctl: Add move --to-directory and --from-directory flags #7005

✨ clusterctl: Add move --to-directory and --from-directory flags #7005

Conversation

oscr commented Aug 2, 2022

k8s-ci-robot commented Aug 2, 2022

k8s-ci-robot commented Aug 2, 2022

ykakarap left a comment

Choose a reason for hiding this comment

fabriziopandini left a comment

Choose a reason for hiding this comment

oscr commented Aug 16, 2022

sbueringer commented Aug 16, 2022

ykakarap left a comment

Choose a reason for hiding this comment

fabriziopandini commented Sep 21, 2022

oscr Sep 21, 2022

Choose a reason for hiding this comment

k8s-ci-robot commented Sep 21, 2022

fabriziopandini commented Sep 22, 2022

ykakarap left a comment

Choose a reason for hiding this comment

fabriziopandini commented Sep 23, 2022

k8s-ci-robot commented Sep 23, 2022

oscr commented Sep 23, 2022