Multiple SriovNetworkNodePolicy creation fails

[e0ne]

Environment details:

• the latest SR-IOV Network Operator
• at least two worker nodes with 2 SR-IOV NICs on each of them: worker1 nic1, worker1 nic2, worker2 nic1, worker2 nic2
• SR-IOV is disabled on NICs

Steps to reproduce:

• create two yaml files with policies definition:
  • file1.yaml: it should contain policies for worker1 nic1 and worker2 nic1
  • file2.yaml: it should contain policies for worker1 nic2 and worker2 nic2
• apply file1.yaml
• wait until worker1 starts reboot
• apply file2.yaml
• wait until worker1 started

Expected results:
All policies are applied

Actual results:
Part of policies applied. Leader elected for worker 2. Worker1 still has 'Draining' annotation, so config daemons do not proceed any configurations

[SchSeba]

Hi @e0ne can you please check the sriov-network-config-daemon logs?

[adrianchiris]

I believe we essentially hitting a deadlock in case worker1, after reboot, needs to drain the node and worker 2 is holding the drain lock (is leader and waiting for draining annotation to be removed from node1).

Or generally speaking, any case where nodeStateSyncHandler is called in sriov-network-config-daemon while node has draining annot and during execution reqDrain is true and disableDrain is false.

[zshi-redhat]

I believe we essentially hitting a deadlock in case worker1, after reboot, needs to drain the node and worker 2 is holding the drain lock (is leader and waiting for draining annotation to be removed from node1).

Wondering why worker-2 is able to get the drain lock while worker-1 still holds Draining annotation?

[adrianchiris]

Wondering why worker-2 is able to get the drain lock while worker-1 still holds Draining annotation?

as far as i saw there is no place in the daemon code that prevents it. as soon as nodeStateSyncHandler() config daemon releases leadership

[zshi-redhat]

Wondering why worker-2 is able to get the drain lock while worker-1 still holds Draining annotation?

as far as i saw there is no place in the daemon code that prevents it. as soon as nodeStateSyncHandler() config daemon releases leadership

It gets the drainLock when dn.drainable is equal to true, but dn.drainable requires other nodes to not have Draining annotation.

[adrianchiris]

It gets the drainLock when dn.drainable is equal to true, but dn.drainable requires other nodes to not have Draining annotation.

i see in daemon.go-L#803 that its called within OnStartedLeading so at this point it already got the lock i.e started leading.

it will loop until it becomes drainable or context is canceled which never happens in this case.

am i missing something?

[zshi-redhat]

It gets the drainLock when dn.drainable is equal to true, but dn.drainable requires other nodes to not have Draining annotation.

i see in daemon.go-L#803 that its called within OnStartedLeading so at this point it already got the lock i.e started leading.

it will loop until it becomes drainable or context is canceled which never happens in this case.

dn.drainable will be set once other nodes complete drain, right? so it will eventually be able to proceed.

am i missing something?

[adrianchiris]

dn.drainable will be set once other nodes complete drain, right? so it will eventually be able to proceed.

yes but other nodes will not be able to complete drain if it also attempt to get drain lock (get leadership)

PR #232 addresses that by skipping the lock in daemon nodeStateSyncHandler() in case the node is already draining which will allow daemon to complete the drain

[pliurh]

I've reproduce this issue on OCP.