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

Make it easier to run in Kubernetes #203

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jhuntwork
Copy link

These changes make running in Kubernetes simpler. The main goal here is to split out nginx, php-fpm and crond as separate processes that execute as PID 1 in their own container within a pod. This lets Kubernetes itself supervise those processes instead of the intermediary supervisord. For example, it allows Kubernetes to restart nginx or php-fpm individually in a container if that becomes necessary.

We tried to make these changes as unobtrusive as possible, so that it will be easier to test and doesn't interrupt existing functionality. We did this by testing for the presence of the KUBERNETES_SERVICE_HOST variable which will be present if this container is running in that environment. We then added new k8s specific entrypoint files that source and generally follow the logic of the existing entrypoint files, with the exception two major rules:

  • Let the php-fpm container configure the application specific settings, since that is coupled tightly with php
  • Don't try to manage any current running processes (like sending reload signals). Instead use 'exec' at the end so that the respective service becomes PID 1, and rely on eventual consistency to grant that the whole pod will reach a ready state once all configuration is done and all services are running.

This is also designed to use the same image for all three containers, but just modify their entrypoint script slightly for each case.

There are still a few outstanding things that should probably be improved:

  1. It's unclear if the nginx container and php-fpm both need to write to /var/www/MISP (the app store). If they do, then this should be a shared volume between the two pods, which is easy enough. In my own implementation I build a custom image that moves /var/www/MISP to /MISP at build time and then at run time (after the shared mount is created) rsync /MISP to /var/www/MISP. There is probably a better solution but this is what we currently have.
  2. Getting all logging to output to stdout which is standard for containers, especially in kubernetes. This lets us do things like send to a full-featured logging service and do filtering/routing there.
  3. Re-working the background workers into something different. I understand this is an upstream feature change, but the current implementation expects that the workers will be running on the same host as the main php process and are discoverable via PID which does not hold true if they are containerized/scalable elsewhere.

Looking forward to feedback and happy to adjust where ever it makes sense.

tonalitech702 and others added 2 commits January 8, 2025 10:03
- Let the php container run the inet supervisord for the bg workers
  still
- Properly configure the cron container to exec cron
- Add configuration to optionally change the sock file location for
  php-fpm, allows us to specify a shared file between containers in a
  pod
- make new entrypoint files executable
- Set the php config value for `session.cookie_domain` so that it
  doesn't use the default of ''. When empty it falls back to the
  hostname which will be different per pod, meaning that each pod will
  handle session requests separately, which breaks things like OIDC.
@ostefano
Copy link
Collaborator

ostefano commented Jan 9, 2025

Thank you for this PR @jhuntwork , this is awesome stuff 🎄

I will need a bit of time due to day-job constraints, but will get to it eventually.

Feel free to reach out to me on Gitter, so we can iterate quickly on this.

@ostefano ostefano self-requested a review January 9, 2025 09:02
@ostefano ostefano added the enhancement New feature or request label Jan 9, 2025
@ostefano
Copy link
Collaborator

@oivindoh can you give it a look? Looks reasonable to be, but I would like this not to clash with your current way of doing things on K8S. Likewise, if you have suggestions, please add them here.

@ostefano ostefano added the help wanted Extra attention is needed label Jan 12, 2025
@ostefano ostefano linked an issue Jan 12, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Helm Chart
3 participants