Re: [kubernetes/kubernetes] pluginwatcher there is no de-register method (#64637)

[Kubernetes Submit Queue]

[MILESTONENOTIFIER] Milestone Issue Needs Approval

@sbezverk @kubernetes/sig-storage-misc

Action required: This issue must have the status/approved-for-milestone label applied by a SIG maintainer. If the label is not applied within 7 days, the issue will be moved out of the v1.11 milestone.

Issue Labels

• sig/storage: Issue will be escalated to these SIGs if needed.
• priority/important-soon: Escalate to the issue owners and SIG owner; move out of milestone after several unsuccessful escalation attempts.
• kind/bug: Fixes a bug discovered during the current release.

Help

• Additional instructions
• Commands for setting labels

—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[Saad Ali]

If a plugin is removed, we have no way to deregister it. If a plugin is uninstalled, should the kubelet be alerted so it can do some cleanup? Probably not a blocker for 1.11, but let's think about it.

CC @jiayingz @vikaschoudhary16 @vishh @vladimirvivien @sbezverk @RenaudWasTaken

[Kubernetes Submit Queue]

[MILESTONENOTIFIER] Milestone Issue Needs Attention

@sbezverk @kubernetes/sig-storage-misc

Action required: During code slush, issues in the milestone should be in progress.
If this issue is not being actively worked on, please remove it from the milestone.
If it is being worked on, please add the status/in-progress label so it can be tracked with other in-flight issues.

Note: If this issue is not resolved or labeled as priority/critical-urgent by Tuesday, June 5th it will be moved out of the v1.11 milestone.

Issue Labels

• sig/storage: Issue will be escalated to these SIGs if needed.
• priority/important-soon: Escalate to the issue owners and SIG owner; move out of milestone after several unsuccessful escalation attempts.
• kind/bug: Fixes a bug discovered during the current release.

Help

• Additional instructions
• Commands for setting labels

—

[Kubernetes Submit Queue]

[MILESTONENOTIFIER] Milestone Issue Needs Attention

@sbezverk @kubernetes/sig-storage-misc

Action required: During code slush, issues in the milestone should be in progress.
If this issue is not being actively worked on, please remove it from the milestone.
If it is being worked on, please add the status/in-progress label so it can be tracked with other in-flight issues.

Note: If this issue is not resolved or labeled as priority/critical-urgent by Tuesday, June 5th it will be moved out of the v1.11 milestone.

Issue Labels

• sig/storage: Issue will be escalated to these SIGs if needed.

• priority/important-longterm: Escalate to the issue owners; move out of the milestone after 1 attempt.

• kind/bug: Fixes a bug discovered during the current release.

[Vikas Choudhary (vikasc)]

@saad-ali we discussed it already in the first meeting. If plugin uninstalls, grpc will pick that up and kubelet component can unregister it. later when the plugin will reapear, watcher will pick it up and send a register request to kubelet component.
/cc @jiayingz

[Renaud Gaubert]

I've added a few comments to make this more explicit in PR#64621

[Kubernetes Submit Queue]

[MILESTONENOTIFIER] Milestone Issue Needs Attention

@sbezverk @kubernetes/sig-storage-misc

Action required: During code slush, issues in the milestone should be in progress.
If this issue is not being actively worked on, please remove it from the milestone.
If it is being worked on, please add the status/in-progress label so it can be tracked with other in-flight issues.

Note: If this issue is not resolved or labeled as priority/critical-urgent by Tuesday, June 5th it will be moved out of the v1.11 milestone.

Issue Labels

• sig/storage: Issue will be escalated to these SIGs if needed.
• priority/important-longterm: Escalate to the issue owners; move out of the milestone after 1 attempt.
• kind/bug: Fixes a bug discovered during the current release.

Help

• Additional instructions
• Commands for setting labels

—

[Kubernetes Submit Queue]

[Kubernetes Submit Queue]

[Serguei Bezverkhi]

@vikaschoudhary16 how pluginswatcher informs in-tree part of driver that the socket is gone and the driver needs to be de-registered?

[Kubernetes Submit Queue]

[MILESTONENOTIFIER] Milestone Removed From Issue

@sbezverk @kubernetes/sig-storage-misc

Important: Code freeze is in effect and only issues with priority/critical-urgent may remain in the v1.11 milestone.

Help

• Additional instructions
• Commands for setting labels

—

[Renaud Gaubert]

From @saad-ali:

Unregistration is expected to be handled by the consumers using gRPC (i.e:when the connection closes).

gRPC connection close does not (and should not) signal plugin uninstallation/unregsitration. The CSI implementation, for example, closes the connection after every operation. Connection could also drop for a number of reasons.

So we do need to think about the socket deletion case (as a signal of plugin uninstallation), but I agree that does not need to be addressed in this PR. Please open another issue to track that and we can handle that separately.

Connection could also drop for a number of reasons.

The connection here is on a unix socket backed by the gRPC retry mechanism. At least in the device plugin case it is usually pretty safe to adequate a gRPC close to an un-registration.

Though I do agree that removing the socket is a more explicit and consistent signal for un-registration. And we could also consider gRPC connection closing as a crashed / unhealthy device plugin.

That would allow to have a saner model that clearly makes the distinction between crash and unregistered. And thus have different behavior.

[Vikas Choudhary (vikasc)]

Will it be ok to assume that socket removal is always done by plugin intentionally and is not result of some unexpected action?

[hui luo]

/cc

[Saad Ali]

For CSI we intend to have two sockets, one that is used for registering and a second one to actually talk to the driver. As part of the registration process the path of the second socket is returned to Kubelet. After initial registration handshake the connection to the first socket is closed. Then for each operation kubelet opens a connection to the second CSI socket and closes it after that operation is complete. So the state of the socket (open or closed) is not a good signal for either "registration status" of plugin for CSI or for "plugin health". The existence of the socket would be a much better signal, so I support adding logic to monitor for socket removal and use that to trigger a "unregistration" callback.

Will it be ok to assume that socket removal is always done by plugin intentionally and is not result of some unexpected action? But agree that it is more explicit than gRPC disconnection.

Yes, that makes sense to me.

[Renaud Gaubert]

I believe this was fixed by /pull/64621

[Vladimir Vivien]

So this was implemented. Should this be closed?

[Renaud Gaubert]

/close
Is this the right command?

[k8s-ci-robot]

Closed #64637.

[k8s-ci-robot]

@RenaudWasTaken: Closing this issue.

In response to this:

/close
Is this the right command?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.