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

PWX-33887: storkctl to support namespacemapping during applicationrestore creation. #1545

Merged
merged 2 commits into from
Oct 31, 2023

Conversation

diptiranjanpx
Copy link
Contributor

@diptiranjanpx diptiranjanpx commented Oct 26, 2023

Signed-Off-By: Diptiranjan

What type of PR is this?

improvement

What this PR does / why we need it:
Added the param for creating applicationrestore with namespacemapping with storkctl.

Does this PR change a user-facing CRD or CLI?:

Is a release note needed?:

Issue: Applicationrestore created with storkctl was always restoring to namespace with same name as source.
User Impact: User could not restore to a different namespace with appliactionrestore created with storkctl.
Resolution: Storkctl now has support to accept namespace mapping as a parameter for restoring to different namespace.

Does this change need to be cherry-picked to a release branch?:

Test
updated in ticket PWX-33887.

@diptiranjanpx diptiranjanpx added the unit-test The change adds or updates a unit test label Oct 26, 2023
@cnbu-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

Comment on lines +256 to +259
cmdArgs := []string{"create", "apprestores", "-n", "test", "restore1", "--backupLocation", "backuplocation1", "--backupName", "backup1", "--namespaceMapping", "namespace1:namespace2"}
expected := "ApplicationRestore restore1 started successfully\n"
testCommon(t, cmdArgs, nil, expected, false)

Copy link
Contributor

Choose a reason for hiding this comment

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

We can enhance this test to pass comma separated 2 namespaces so we cover all the bases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@codecov
Copy link

codecov bot commented Oct 30, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9f6cbb6) 66.14% compared to head (9252279) 66.23%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1545      +/-   ##
==========================================
+ Coverage   66.14%   66.23%   +0.09%     
==========================================
  Files          43       43              
  Lines        5390     5405      +15     
==========================================
+ Hits         3565     3580      +15     
  Misses       1491     1491              
  Partials      334      334              
Files Coverage Δ
pkg/storkctl/applicationrestore.go 83.93% <100.00%> (+1.02%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

nsMapping := strings.Split(inputMapping, ":")
if len(nsMapping) == 2 {
inputMappingMap[nsMapping[0]] = nsMapping[1]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to print any errormsg, if the namespace entry was not in the form x:y?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@diptiranjanpx diptiranjanpx merged commit baefe05 into master Oct 31, 2023
7 checks passed
@diptiranjanpx diptiranjanpx deleted the PWX-33887 branch May 27, 2024 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement unit-test The change adds or updates a unit test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants