-
Notifications
You must be signed in to change notification settings - Fork 200
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
feat(upgrade): enable bulk upgrade for volumes and spc #1655
Conversation
Signed-off-by: shubham <shubham.bajpai@mayadata.io>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!!
cmd/upgrade/executor/cstor_spc.go
Outdated
@@ -50,11 +50,14 @@ func NewUpgradeCStorSPCJob() *cobra.Command { | |||
Long: cstorSPCUpgradeCmdHelpText, | |||
Example: `upgrade cstor-spc --spc-name <spc-name>`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this example needs a fix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
cmd/upgrade/executor/cstor_spc.go
Outdated
util.CheckErr(options.InitializeDefaults(cmd), util.Fatal) | ||
util.CheckErr(options.RunCStorSPCUpgrade(cmd), util.Fatal) | ||
if options.cstorSPC.spcName != "" { | ||
singleCstorSPCUpgrade(cmd) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cstor should be changed to CStor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
options.resourceKind = "storagePoolClaim" | ||
util.CheckErr(options.RunPreFlightChecks(cmd), util.Fatal) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't need of passing 'cmd' to RunCStorSPCUpgradeChecks and RunCStorSPCUpgrade.. same may be the case with other checks.. can we remove that if not required
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
few minor comments..
and one point to verify: hope there is fatal in code whenever upgrade fails
} | ||
|
||
return nil | ||
} | ||
|
||
// RunCStorSPCUpgrade upgrades the given Jiva Volume. | ||
func (u *UpgradeOptions) RunCStorSPCUpgrade(cmd *cobra.Command) error { | ||
klog.V(4).Infof("Started upgrading %s{%s} from %s to %s", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets have similar message for completed upgrade also
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Signed-off-by: shubham <shubham.bajpai@mayadata.io>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes are good
Signed-off-by: shubham shubham.bajpai@mayadata.io
Ref: openebs/openebs#2701
What this PR does / why we need it:
This PR enables the bulk upgrade option for upgrade jobs where multiple volumes or spc can be upgraded in one job.
Updated job yaml:
Note: The old way of accepting pv name or spc name through the flags is still supported for upgrading one volume at a time.
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close that issue when PR gets merged): fixes #Special notes for your reviewer:
Checklist:
documentation
tagbreaking-changes
tagrequires-upgrade
tag