Operator's shutdown logic redsign

[zeeke]

I'm starting this thread to discuss how to simplify the shutdown logic in the operator [1]
The procedure includes the following steps:

a. Set ResourfceInjectorWebhook.FailurePolicy = Ignore (mutating webhook for pods)
this can be safely removed, as the resource injector failure policy is Ignore by default.

b. Set ValidationWebhook.FailurePolicy = Ignore for sriovoperatorconfigs and sriovnetworknodepolicies.

c. Remove finalizers from SriovNetworks
Finalizers are used to delete NetworkAttachmentDefinition when a SriovNetwork is deleted. We can't use OwnerReference because net-attach-def can be in a different namespace than the SriovNetwork. BTW, I'm not getting exactly which scenario we are trying to save with them.

I'm inclined to get rid of all of those step. Does anyone has some insights on this topic?

[1] https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/a3a5ba993eb0fc407a8df67ce6d3a1e5dfa3c9c6/pkg/utils/shutdown.go

[adrianchiris]

Adding feedback from today's community meeting:

Shutdown logic:
1 - Modify the webhook failurePolicy to Ignore
2 - Delete finalizer from SriovNetwork
SchSeba idea: make the sriov-network-operator reconcile loop handle the deletion of its own Deployment for deletion. operator’s should put finalizers on Deployment
Adrian idea: Moving (Ib)SriovNetwork object to user namespaces and link them to NetAttachDef via OwnerReference.

[zeeke]

Going into details:

SchSeba idea: make the sriov-network-operator reconcile loop handle the deletion of its own Deployment for deletion. operator’s should put finalizers on Deployment

With this solution, we're going to create a new controller (lets call it SelfDeploymentController) that watches for the operator's deployment, in order to trigger specific logic during uninstallation. We're assuming here that the deployment gets deleted if and only if the user is uninstalling the operator (fair enough assumption IMO). The uninstall procedure would include:
a. Delete every NetworkAttachmentDefinition created for Sriov{Ib}Networks
b. Delete/Inhibit all the validating and mutating webhooks
c. Remove the finalizer from the operator's deployment, which has been added be the SelfDeploymentController at startup.

With the changes described above, no finalizers are needed for the NetworkAttachmentDefinition.

Adrian idea: Moving (Ib)SriovNetwork object to user namespaces and link them to NetAttachDef via OwnerReference.

The main problem here is the RBAC for the SriovNetwork resources. We're supposing our users are implemented by two different personas:
p1. The cluster admin, which creates SriovNetworkNodePolicies and SriovNetworks, as he knows which hardware is available on every node.
p2. The application user: which is responsible for deploying the workload. The only thing he knows about the cluster is the NetAttachDef name and the resource name to consume Virtual Functions.

@adrianchiris @SchSeba Am I missing anything here?

Bonus question: since we implemented

• Stop default creation of SriovOperatorConfig and Policy #592

The shutdown/uninstall procedure can be simplified by:
u1. The user deletes the SriovOperatorConfig/default. This ensure all the SriovNetworkNodeState, all the webhooks, device-plugin and config-daemon are cleaned out.
u2. The user deletes the deployment.

If we ensure the above procedure (either by documentation or by configuration on helm) we can get rid of all the tricks we're trying to solve. Right?