Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --include-cluster-resources flag for restores #147

Merged
merged 2 commits into from
Oct 23, 2017

Conversation

skriss
Copy link
Member

@skriss skriss commented Oct 20, 2017

Fixes #136

@skriss skriss added this to the v0.5.0 milestone Oct 20, 2017
@skriss skriss force-pushed the restore-inc-cluster-resources-flag branch from f37fd45 to 9c4e15a Compare October 20, 2017 19:56
@@ -394,6 +401,24 @@ func TestRestoreResourceForNamespace(t *testing.T) {
restorers: map[schema.GroupResource]restorers.ResourceRestorer{schema.GroupResource{Resource: "foo-resource"}: newFakeCustomRestorer()},
expectedObjs: toUnstructured(newTestConfigMap().WithArkLabel("my-restore").ConfigMap),
},
// NEW
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove

@skriss skriss force-pushed the restore-inc-cluster-resources-flag branch from 9c4e15a to b7d8ef5 Compare October 20, 2017 20:53
@ncdc
Copy link
Contributor

ncdc commented Oct 22, 2017

@skriss I was just thinking that if you're creating a Restore by hand in yaml/json and you omit includeClusterResources, it will default to false and they won't be included. Do you think it would make more sense to change this to excludeClusterResources so the default behavior is to include them?

@ncdc
Copy link
Contributor

ncdc commented Oct 22, 2017

Or make it an optional bool (pointer)

@skriss
Copy link
Member Author

skriss commented Oct 23, 2017

Yeah, I think defaulting to include makes sense. I think I like using an optional bool that defaults to true if not specified better; it's more consistent with the corresponding flag on backups.

@ncdc
Copy link
Contributor

ncdc commented Oct 23, 2017

Yeah, let's go with the optional bool that defaults to true.

@skriss skriss force-pushed the restore-inc-cluster-resources-flag branch 2 times, most recently from 440d12c to 59b4ec9 Compare October 23, 2017 17:45
Signed-off-by: Steve Kriss <steve@heptio.com>
…rue)

Signed-off-by: Steve Kriss <steve@heptio.com>
@skriss skriss force-pushed the restore-inc-cluster-resources-flag branch from 59b4ec9 to a7cc587 Compare October 23, 2017 17:51
@ncdc ncdc merged commit a1b43c4 into vmware-tanzu:master Oct 23, 2017
@skriss skriss deleted the restore-inc-cluster-resources-flag branch October 23, 2017 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants