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

Watch ConfigMap changes for related states #564

Merged
merged 1 commit into from
Jun 28, 2023

Conversation

adrianchiris
Copy link
Collaborator

Currently config map changes will not trigger reconcile. Add ConfigMap as watched resource for nicclusterpolicy controller using nv-ipam state.

@adrianchiris adrianchiris changed the title Watch ConfigMap changes for nv-ipam state [WIP] Watch ConfigMap changes for nv-ipam state Jun 26, 2023
@adrianchiris adrianchiris changed the title [WIP] Watch ConfigMap changes for nv-ipam state Watch ConfigMap changes for nv-ipam state Jun 26, 2023
@adrianchiris adrianchiris added the on hold This enhancement is currently on hold pending additional clarification and evaluation label Jun 26, 2023
Currently config map changes will not trigger reconcile.
Add ConfigMap as watched resource for nicclusterpolicy
controller for the following states:

- nv-ipam
- multus-cni
- rdma-device-plugin
- sriov-device-plugin

Signed-off-by: adrianc <adrianc@nvidia.com>
@adrianchiris adrianchiris removed the on hold This enhancement is currently on hold pending additional clarification and evaluation label Jun 28, 2023
@adrianchiris
Copy link
Collaborator Author

@ykulazhenkov @rollandf PTAL, i found that a few other states were missing watch for configmap.

p.s
technically we should watch every resource that we create (and own) but for now lets just align configmap

@adrianchiris adrianchiris changed the title Watch ConfigMap changes for nv-ipam state Watch ConfigMap changes for related states Jun 28, 2023
@ykulazhenkov ykulazhenkov self-requested a review June 28, 2023 09:57
@ykulazhenkov
Copy link
Collaborator

LGTM, thx

Copy link
Member

@rollandf rollandf left a comment

Choose a reason for hiding this comment

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

LGTM

@adrianchiris adrianchiris merged commit 5fb589e into Mellanox:master Jun 28, 2023
9 checks passed
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.

None yet

3 participants