Restart device plugin pod after policy apply

[e0ne]

We need to restart device plugin pod after node policy applied
and all SR-IOV Network Config Daemon plugins finished. E.g.:

Generic plugin applies SR-IOV Network Node Policy and creates
VFs. That's mean we need to restart device plugin pod to found
newly created devices.

Related k8snetworkplumbingwg/sriov-network-device-plugin#276

[zshi-redhat]

Until k8snetworkplumbingwg/sriov-network-device-plugin#276
wiill be fixed we need to restart device plugin pod each time after
SR-IOV Network Operator plugin applied. It's needed because plugin
could change a number of VF resources even if config is not changed.

What are those changes a plugin may do without sriov policy being updated?

[adrianchiris]

I am not sure this change is needed. we have encountered some issues in our CI. but it may be env issue.
@e0ne is checking and will update.

for now this should be on hold

[e0ne]

What are those changes a plugin may do without sriov policy being updated?

If my understanding is correct, if policy is not updated this code won't be executed because the generation number will be the same

[zshi-redhat]

What are those changes a plugin may do without sriov policy being updated?

If my understanding is correct, if policy is not updated this code won't be executed because the generation number will be the same

Correct, only new generation of policy will trigger the device plugin restart.

[zshi-redhat]

What are those changes a plugin may do without sriov policy being updated?

If my understanding is correct, if policy is not updated this code won't be executed because the generation number will be the same

Correct, only new generation of policy will trigger the device plugin restart.

Do you see a situation that devices are updated but the conditional checks for reqDrain and DpConfigVersion are not met?

[e0ne]

@zshi-redhat yes, you can see logs of network-operator CI where this issue is reproduced: http://13.74.249.42/nic_operator_helm-ci/405/

[e0ne]

@zshi-redhat we've to a pretty specific use-case when we've got custom SriovOperatorConfig with configDaemonNodeSelector and labels could be dynamically added or removed

[e0ne]

@zshi-redhat I've got such steps to reproduce the issue:

• SR-IOV should be disable in a firmware, so reboot is required
• once node is rebooted and SR-IOV is enabled, we apply generic plugin but reqDrain is false and DpConfigVersion is not changed

so my patch just adds reboot each time we do changes to SR-IOV configuration

[pliurh]

@e0ne With your fix, the device plugin pod will be restarted in every reconcile loop. It will cause many unnecessary pod restarts.

The issue you mentioned only happens when the numVFs is changed from 0 to a non-zero value, as we don't need to drain the node when VFs are fresh created

sriov-network-operator/pkg/plugins/generic/generic_plugin.go

Line 177 in ac5db51

if ifaceStatus.NumVfs != 0 {

So a better solution should be to add this condition to trigger the device plugin pod restart, instead of always triggering a restart.

[e0ne]

Actually, this code will be executed only if node state is changed: https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/master/pkg/daemon/daemon.go#L438

[pliurh]

@zshi-redhat This PR looks good to me. Do you have any other comments?

[zshi-redhat]

@zshi-redhat This PR looks good to me. Do you have any other comments?

no, looks good to me.