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 ReadOnly option to volumeMount in Pods #415

Merged
merged 3 commits into from
Jul 3, 2024

Conversation

irumaru
Copy link
Contributor

@irumaru irumaru commented Jun 12, 2024

Added ReadOnly to the volumeMount option in Pods to support mounting of ReadOnlyMany(ROX) PVC.
#279

The default is ReadOnly: false.
The reason was to avoid destructive changes, considering the possibility that some users may already be writing to the volume in the init container.

Here are the results of a simple test.
files.zip

Test Contents

  • kubectl apply -f pvc.yaml to create pods to add pods and scenarios
  • kubectl exec -it vp -- /bin/bash to add main.js to /mnt/vp in pods
  • Create TestRun with kubectl apply -f main.yaml

result
describe-log.txt and get-log.txt are the execution results.
You can confirm that ReadOnly for k6-test-volume is true.

Copy link
Collaborator

@yorugac yorugac left a comment

Choose a reason for hiding this comment

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

@irumaru, sorry for the delay!
Thanks for working on this 🙌 PR LGTM: there's just a minor suggestion to improve comments. And one more thing: could you please create a folder in config/samples with code files from your archive?

pkg/types/script.go Outdated Show resolved Hide resolved
irumaru and others added 2 commits July 2, 2024 17:24
Add comment.

Co-authored-by: Olha Yevtushenko <yorugac@users.noreply.github.com>
@irumaru
Copy link
Contributor Author

irumaru commented Jul 2, 2024

@yorugac , Thanks for the review.

We have made the following two corrections.

  • Added config/samples/k6_v1alpha1_k6_with_readOnlyVolumeClaim.yaml.
  • Added comment to pkg/types/script.go as suggested.

Please review.

Copy link
Collaborator

@yorugac yorugac left a comment

Choose a reason for hiding this comment

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

Thank you for the update @irumaru! 🙌

@yorugac
Copy link
Collaborator

yorugac commented Jul 3, 2024

For info, failing CI for Helm on PRs is a known current issue: #419

@yorugac yorugac merged commit eaf0128 into grafana:main Jul 3, 2024
6 of 8 checks passed
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