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

feat(statefulset): add containerSecurityContext to init containers #178

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Johannes-
Copy link

The containerSecurityContext is already partially utilized within the init containers. Therefore, it would make sense to extend its application as a securityContext to the initContainers as well.

@@ -50,6 +50,10 @@ spec:
volumeMounts:
- name: qdrant-storage
mountPath: /qdrant/storage
{{- with .Values.containerSecurityContext }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @Johannes- , thanks for the contribution!

As mentioned here: #130 I think this would break by default since the init container needs to run as root, but using the default containerSecurityContext will make it run as user 1000.

Instead of re-using containerSecurityContext, could it be controlled by a new, separate setting called initContainerSecurityContext? May lead to duplication in peoples' values, but it allows people to assign different security contexts to each.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also in #177 I removed the second init container, consolidating them into a single init container. That's probably what the merge conflict is.

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