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.

Sign up for GitHub

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

Jump to bottom

ironic watcher #7188

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

ironic watcher #7188

wants to merge 3 commits into from
+26 −16

Conversation

sandzwerg
Copy link
Contributor

@sandzwerg sandzwerg commented Oct 8, 2024 •
edited
Loading

  • ironic: enable watcher in testing

@sandzwerg
ironic: enable watcher in testing
3d41d9f 
Fix whitespace controlling so there is a space between the middleware and
watcher block.
Enable watcher in testing
@sandzwerg
Copy link
Contributor Author

OK, @stefanhipfel reasons that the conductor has no things that the watcher needs to watch, so we should just remove it altogether. The issue is that the same ironic.conf is used for API and conductors.

@fwiesel
Copy link
Member

fwiesel commented Oct 8, 2024

You could add an ironic-api.conf. By default that one should be loaded automatically and merged the "global" ironic.conf by the ironic-api process, but not ironic-conductor

@sandzwerg
ironic: enable watcher only in API pods
3a46a30 
This puts the watcher config in it's own config file below ironic.conf.d
That allows us to only include the watcher config in API deployments.
The conductors don't need the watcher config but had it via the shared
ironic.conf and were missing the watcher.yaml which lead to warnings.
As the ironic-conductors don't need the watcher middleware we remove it
from the conductors.
@sandzwerg
ironic: enable audit in tests
7bcfc2d 
Minor indentation changes that should result in no empty line when
audit is disabled. Also put the mountpath first, as this is what
distinguish the different files, not the configmap, that is always the
same.
@sandzwerg
Copy link
Contributor Author

The latest commits should have done something similar. I moved the watcher config in its own file as suggested by @joker-at-work Please check if everything fits. I did my best but I can't rule out I missed some reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fwiesel fwiesel fwiesel approved these changes

@stefanhipfel stefanhipfel Awaiting requested review from stefanhipfel stefanhipfel is a code owner

@BerndKue BerndKue Awaiting requested review from BerndKue BerndKue is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sandzwerg @fwiesel