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

init container cannot be run when mounting configuration values from envVars without also mounting those envVars to the init container #620

Closed
DiamondJoseph opened this issue Sep 5, 2024 · 6 comments · Fixed by #631
Assignees
Labels
bug Something isn't working helm chart

Comments

@DiamondJoseph
Copy link
Contributor

DiamondJoseph commented Sep 5, 2024

e.g. the rabbitmq password injection into the stomp config that is being done on all beamlines.

options:

  • mount all envVars on the container also on the initContainer
  • adjust how the initContainer loads the config such that it only parses the sections that are relevant for setting up the scratch area
  • separate the configuration into that required for the initContainer and for the primary container
  • allow fetching the envVar to gracefully fail (only on the initContainer? if we fail to get the pwd in the main container currently we would fail to connect to the broker and should fall over, but I believe this isn't currently the case: and it wouldn't be too clear as to why the pod had fallen over, since we cannot log the password and show that it's empty!)
@DiamondJoseph DiamondJoseph added bug Something isn't working helm chart labels Sep 5, 2024
@DiamondJoseph DiamondJoseph changed the title inint container cannot be run when mounting configuration values from envVars without also mounting those envVars to the init container init container cannot be run when mounting configuration values from envVars without also mounting those envVars to the init container Sep 5, 2024
@callumforrester
Copy link
Contributor

Relevant: #495

@stan-dot stan-dot self-assigned this Sep 10, 2024
@stan-dot
Copy link
Contributor

separate the configuration into that required for the initContainer and for the primary container

that is my preferred solution, with separation of concerns

@callumforrester
Copy link
Contributor

@stan-dot it should be quite easy to do it that way. The ConfigLoader can also load any pydantic model as configuration. If we're going to do it this way then...

Acceptance Criteria

  1. Running blueapi setup-scratch takes a path to a configuration file as an argument, the file only contains the scratch portion of the configuration
  2. When running blueapi -c <config path> serve, the configuration file here no longer contains the scratch portion of the configuration
  3. The two configurations are now separated in the helm chart's values.yaml
  4. There are two ConfigMaps in the helm chart, one is mounted into the initContainer and one into the main container

@stan-dot
Copy link
Contributor

on it

@stan-dot
Copy link
Contributor

re: point 3

p45 has a separate config to the one in the blueapi repo, what is the reasoning here?

https://github.com/epics-containers/p45-services/blob/522e29a01c3a9007a3aee802257953a200939d32/services/daq-blueapi/values.yaml#L24

@DiamondJoseph
Copy link
Contributor Author

p45 has a separate config to the one in the blueapi repo, what is the reasoning here?

The values in the blueapi repo are chosen such that blueapi can be started up with a minimal valid set without any other infrastructure (message bus, file system, devices...) or assumptions (required plans etc.). The p45 repository has the values customising it for p45's deployment.

blueapi repo: generic, start easily
ixx repo: specific, has requirements

@stan-dot stan-dot linked a pull request Sep 11, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working helm chart
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants