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

Support readOnlyRootFilesystem #648

Merged
merged 3 commits into from
Nov 9, 2023

Conversation

radu-gheorghe
Copy link
Contributor

This should be the first part (or at least an initial attempt at it) for #624

It adds /tmp as emptyDir to the zk-init init container. It also sets readOnlyRootFilesystem to it. For testing and also why not 🙂 I've tested it with Solr 9.3 and a chroot and I see the chroot and I see readOnlyRootFilesystem in its container definition.

Any feedback is welcome. Once we agree about this, I'd like to check if Solr itself can run as readOnlyRootFilesystem and if not, add support for that, check for issues and iron them out.

@HoustonPutman HoustonPutman linked an issue Oct 30, 2023 that may be closed by this pull request
@radu-gheorghe
Copy link
Contributor Author

I just pushed a commit that should fix the breaking tests.

I've also experimented with the Solr container using readOnlyRootFilesystem by doing kubectl edit on the StatefulSet. Solr seems to work just fine (I created a collection, added some docs...). Nothing seems to break, at least at first. Maybe this fixes your problem in #624 @thomaswoeckinger ?

@thomaswoeckinger
Copy link

I think so, i will test it on my side and give you feedback then.

@radu-gheorghe
Copy link
Contributor Author

Sounds good, thanks @thomaswoeckinger !

@thomaswoeckinger
Copy link

From my point of view, at least some common request for collection creation, modification, deletion and sharding is working, so i think this is good to go

@HoustonPutman
Copy link
Contributor

Let's keep it just the zk-init container for now (or all the init containers we do). That way we don't break use cases we don't know about. Not sure there's a giant benefit for us to set it by default right now.

@radu-gheorghe
Copy link
Contributor Author

Sounds good to me! Does this mean that this PR is good to merge, or should I change something?

@HoustonPutman
Copy link
Contributor

I added a changelog entry. Once the tests pass, I'll merge. Thanks for the great work!

@HoustonPutman HoustonPutman merged commit 268764c into apache:main Nov 9, 2023
3 checks passed
@radu-gheorghe
Copy link
Contributor Author

Awesome! Thanks for checking and merging!

HoustonPutman pushed a commit that referenced this pull request Apr 3, 2024
Add /tmp as emptyDir to zk-init and make it readOnlyRootFilesystem
smoldenhauer-ish added a commit to intershop/solr-operator that referenced this pull request Dec 9, 2024
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.

Possibility to run as readOnlyRootFilesysystem
3 participants