-
Notifications
You must be signed in to change notification settings - Fork 18
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
build: add securityContext and fix rabbitMQ cookie #233
base: dev
Are you sure you want to change the base?
Conversation
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.
Thanks @viktoriaas. I would like to see some comment about this being added to the deployment instructions, as I understand that (some of) this is optional. So basically mention what can be enabled, how it can be enabled, and under which circumstances this would be necessary/recommended.
Otherwise it looks fine to me. However, I'm probably not the right person for a more technical review. Maybe @zagganas and @lvarin could have a look? Also to see if this breaks anything on OpenShift.
- /bin/sh | ||
- -c | ||
- | | ||
chmod g-rw /var/lib/rabbitmq/.erlang.cookie; |
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.
What is this for? why is it necessary?
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 already explained in issue #233 . If rabbitMQ deployment is restarted (e.g. due to cluster failure) rabbitMQ cookie has incorrect permissions after restart ( rw-rw----
instead of rw-------
). This is solved by chmod in main container before calling rabbitmq.
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 idea is to put the information about this workaround in the code itself (deployment/templates/rabbitmq/rabbitmq-deployment.yaml). Otherwise no one will ever find this information.
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.
comment added, is it ok now?
@lvarin @zagganas @uniqueg |
Details
For details see issue #232
Note
PR includes old commits from my repo. However, they are not relevant anymore and I removed the changes as they were not good. Only commits from 17 March 2022 are relevant.