-
Notifications
You must be signed in to change notification settings - Fork 19
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(migrate): add support for cstor volume migration #9
feat(migrate): add support for cstor volume migration #9
Conversation
Signed-off-by: shubham <[email protected]>
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.
Yet to review from migratePV
}, | ||
} | ||
if targetDeploy.Spec.Template.Spec.Affinity != nil { | ||
tempPolicy.Spec.Target.PodAffinity = targetDeploy.Spec.Template.Spec.Affinity.PodAffinity |
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.
Do we need to handle nodeAffinity also?
cc: @prateekpandey14
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.
Provided comments
if pvObj.Spec.PersistentVolumeSource.CSI == nil { | ||
csiPV := &corev1.PersistentVolume{} | ||
csiPV.Name = pvObj.Name | ||
csiPV.Annotations = map[string]string{ |
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.
If user-configured labels and annotations for some other purpose we will lose them
- adds code to patch skip-validation annotation - improves wait mechanism for validations after migration due to time taking rebuilds - address review comments Signed-off-by: shubham <[email protected]>
@mittachaitu Addressed the review comments. Will take label and annotation literal changes as well as snapshot migration in separate PR. |
Signed-off-by: shubham <[email protected]>
27d9c91
to
5f5c9e3
Compare
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
} | ||
if cvcObj.Status.Phase != cstor.CStorVolumeConfigPhaseBound { | ||
klog.Infof("Waiting for cvc %s to become Bound, got: %s", v.PVName, cvcObj.Status.Phase) | ||
time.Sleep(10 * time.Second) |
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 is sleep time is much high we can change it to 3 seconds
Signed-off-by: shubham <[email protected]>
Signed-off-by: shubham <[email protected]>
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.
lgtm
Signed-off-by: shubham [email protected]
This PR adds support Cstor volume migration from external provisioner volume spec to CSI volume spec.
Example yaml:
Example logs: