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

Use GlobalMgr for SriovNetwork controllers #175

Merged
merged 1 commit into from
Aug 16, 2021

Conversation

oribon
Copy link
Contributor

@oribon oribon commented Aug 12, 2021

Using 2 managers for the same controllers causes inconsistency in
objects reconcilation. For example we saw a problem with some SriovNetwork
update events not reaching the controller (Reconcile was not triggered).
Using the same Mgr as the specified client for the controller solves these kind of problems.

Signed-off-by: Ori Braunshtein [email protected]

Using 2 managers for the same controllers causes inconsistency in
objects reconcilation. For example we saw a problem with some SriovNetwork
update events not reaching the controller (Reconcile was not triggered).
Using the same Mgr as the specified client for the controller solves these kind of problems.

Signed-off-by: Ori Braunshtein <[email protected]>
@SchSeba SchSeba requested review from pliurh and zshi-redhat August 12, 2021 09:28
@oribon oribon changed the title WIP: Use GlobalMgr for SriovNetwork controllers Use GlobalMgr for SriovNetwork controllers Aug 12, 2021
@SchSeba
Copy link
Collaborator

SchSeba commented Aug 12, 2021

lgtm

waiting for other maintainers to approve

@mgourin
Copy link

mgourin commented Aug 12, 2021

@oribon
Could you elaborate on the differences between "mgr" and "mgrGlobal" and how it caused the issue of not being able to delete the Sriov Networks?

@pliurh
Copy link
Collaborator

pliurh commented Aug 12, 2021

This error was introduced by https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/bc40302df612fc62e0252977011fe7f95abe02b0/main.go

The mgrGlobal have access to all namespaces, which is useful to manage the net-att-def CRs in other namespaces.

@pliurh
Copy link
Collaborator

pliurh commented Aug 12, 2021

/lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants