-
Notifications
You must be signed in to change notification settings - Fork 152
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
Support of read only mount of filesystem volumes added #2365
Support of read only mount of filesystem volumes added #2365
Conversation
I agree to the DCO for all the commits in this PR. |
9add8ee
to
d04ee0b
Compare
521cdc4
to
7ba0910
Compare
These changes needed for adding support of Ceph-CSI shallow volumes. |
7ba0910
to
fa335cd
Compare
Signed-off-by: Sergey Aksenov <sergey.aksenov@veeam.com>
Signed-off-by: Sergey Aksenov <sergey.aksenov@veeam.com>
fa335cd
to
2178d7f
Compare
Signed-off-by: Sergey Aksenov <sergey.aksenov@veeam.com>
@viveksinghggits please continue review |
validatedVols := make(map[string]kube.VolumeMountOptions) | ||
for pvcName, mountPoint := range vols { | ||
pvc, err := cli.CoreV1().PersistentVolumeClaims(namespace).Get(ctx, pvcName, metav1.GetOptions{}) | ||
if err != nil { | ||
return nil, errors.Wrapf(err, "Failed to retrieve PVC. Namespace %s, Name %s", namespace, pvcName) | ||
} | ||
|
||
validatedVols[pvcName] = kube.VolumeMountOptions{ | ||
MountPath: mountPoint, | ||
ReadOnly: kube.PVCContainsReadOnlyAccessMode(pvc), |
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.
Since this is duplicate of code from, maybe it worth to extract it and reuse ?
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'm afraid it's too late to push another commits for this review. But I can prepare another PR with refactoring if it's crucial to 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.
Not crucial, I was too late :D We can refactor this piece of code next time.
validatedVols := make(map[string]kube.VolumeMountOptions) | ||
// Validate volumes | ||
for pvcName, mountPoint := range vols { | ||
pvc, err := cli.CoreV1().PersistentVolumeClaims(namespace).Get(ctx, pvcName, metav1.GetOptions{}) | ||
if err != nil { | ||
return nil, errors.Wrapf(err, "Failed to retrieve PVC. Namespace %s, Name %s", namespace, pvcName) | ||
} | ||
|
||
validatedVols[pvcName] = kube.VolumeMountOptions{ | ||
MountPath: mountPoint, | ||
ReadOnly: kube.PVCContainsReadOnlyAccessMode(pvc), |
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.
The same is here.
clientset kubernetes.Interface | ||
} | ||
|
||
// NewJob creates a new Job object. | ||
func NewJob(clientset kubernetes.Interface, jobName string, namespace string, serviceAccount string, image string, vols map[string]string, command ...string) (*Job, error) { | ||
func NewJob(clientset kubernetes.Interface, |
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.
func NewJob(clientset kubernetes.Interface, | |
func NewJob( | |
clientset kubernetes.Interface, |
* Support of read only mount added Signed-off-by: Sergey Aksenov <sergey.aksenov@veeam.com> * review comments fixed Signed-off-by: Sergey Aksenov <sergey.aksenov@veeam.com> * renamings regarding review comments Signed-off-by: Sergey Aksenov <sergey.aksenov@veeam.com> --------- Signed-off-by: Sergey Aksenov <sergey.aksenov@veeam.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Change Overview
This PR adds support of read-only mount required by Ceph-CSI shallow volumes feature, only for filesystem volumes.
PR series:
minio
#2378make test
locally usingminio
&minikube
fixed #2379Pull request type
Please check the type of change your PR introduces:
Issues
Test Plan