Description
Found through source review and reproduced at the informer handler.
Label-based discovery does not handle DeletedFinalStateUnknown in DeleteNode. If a Node deletion is delivered as a tombstone, the object is rejected and topology discovery is not queued.
This can leave the deleted Node in generated HyperNodes until another relevant Node or HyperNode event triggers reconciliation.
Steps to reproduce the issue
- Enable label-based HyperNode discovery and add a Node with the configured topology labels.
- Wait for the corresponding HyperNode to be generated.
- Deliver the Node delete event as
cache.DeletedFinalStateUnknown. This is the normal client-go path when a delete is missed during a watch disconnect.
- Check the discovery queue and generated HyperNode.
A minimal handler reproducer is:
discoverer.DeleteNode(cache.DeletedFinalStateUnknown{
Key: "node-1",
Obj: labeledNode,
})
Evidence and production path
The Node informer registers DeleteNode directly as its delete handler:
|
func (l *labelDiscoverer) Start() (chan []*topologyv1alpha1.HyperNode, error) { |
|
l.nodeInformer.Informer().AddEventHandler( |
|
cache.ResourceEventHandlerFuncs{ |
|
AddFunc: l.AddNode, |
|
UpdateFunc: l.UpdateNode, |
|
DeleteFunc: l.DeleteNode, |
|
}, |
|
) |
DeleteNode passes the event object directly to getNodeNetworkTopologyLabels:
|
// DeleteNode Reconstruct the hyperNode when the node changes. |
|
func (l *labelDiscoverer) DeleteNode(obj interface{}) { |
|
labelMap := l.getNodeNetworkTopologyLabels(obj) |
|
if len(labelMap) > 0 { |
|
l.enqueue() |
|
} |
|
} |
That helper only accepts *v1.Node. A tombstone returns an empty label map, so enqueue() is skipped:
|
// getLabelMap get the labelMap on the node used to construct the hyperNode |
|
func (l *labelDiscoverer) getNodeNetworkTopologyLabels(obj interface{}) map[string]string { |
|
tempMap := make(map[string]string) |
|
node, ok := obj.(*v1.Node) |
|
if !ok { |
|
klog.Errorf("Cannot convert to *v1.Node: %v", obj) |
|
return tempMap |
|
} |
|
labelMap := node.Labels |
|
for _, labelKey := range l.networkTopologyRecord { |
|
for _, key := range labelKey { |
|
value, exist := labelMap[key] |
Client-go documents that delete handlers receive DeletedFinalStateUnknown when a delete event is missed during a watch disconnect:
https://pkg.go.dev/k8s.io/client-go/tools/cache#DeletedFinalStateUnknown
The neighboring DeleteHyperNode handler already unwraps the same tombstone type:
|
// DeleteHyperNode Reconstruct the hyperNode when the hyperNode has been deleted. |
|
func (l *labelDiscoverer) DeleteHyperNode(obj interface{}) { |
|
_, ok := obj.(*topologyv1alpha1.HyperNode) |
|
if !ok { |
|
// If we reached here it means the HyperNode was deleted but its final state is unrecorded. |
|
tombstones, ok := obj.(cache.DeletedFinalStateUnknown) |
|
if !ok { |
|
klog.Errorf("Couldn't get object from tombstone %#v", obj) |
|
return |
|
} |
|
_, ok = tombstones.Obj.(*topologyv1alpha1.HyperNode) |
|
if !ok { |
|
klog.Errorf("Tombstone contained object that is not a HyperNode: %#v", obj) |
|
return |
|
} |
|
} |
|
l.enqueue() |
|
} |
The handler was introduced with label-based discovery in:
3346a39
Focused reproducer output:
Cannot convert to *v1.Node
expected node tombstone to trigger topology discovery, queue length is 0
Describe the results you received and expected
Actual: the tombstone is rejected, no discovery is queued, and stale Node membership can remain in the generated HyperNodes.
Expected: DeleteNode should unwrap the Node from DeletedFinalStateUnknown and enqueue topology discovery.
What version of Volcano are you using?
master (8a2c34c)
Any other relevant information
Reproduced with a focused Go test using client-go v0.36.1. This was not observed against a release image in a live cluster.
Description
Found through source review and reproduced at the informer handler.
Label-based discovery does not handle
DeletedFinalStateUnknowninDeleteNode. If a Node deletion is delivered as a tombstone, the object is rejected and topology discovery is not queued.This can leave the deleted Node in generated HyperNodes until another relevant Node or HyperNode event triggers reconciliation.
Steps to reproduce the issue
cache.DeletedFinalStateUnknown. This is the normal client-go path when a delete is missed during a watch disconnect.A minimal handler reproducer is:
Evidence and production path
The Node informer registers
DeleteNodedirectly as its delete handler:volcano/pkg/controllers/hypernode/discovery/label/label.go
Lines 91 to 98 in 8a2c34c
DeleteNodepasses the event object directly togetNodeNetworkTopologyLabels:volcano/pkg/controllers/hypernode/discovery/label/label.go
Lines 310 to 316 in 8a2c34c
That helper only accepts
*v1.Node. A tombstone returns an empty label map, soenqueue()is skipped:volcano/pkg/controllers/hypernode/discovery/label/label.go
Lines 337 to 348 in 8a2c34c
Client-go documents that delete handlers receive
DeletedFinalStateUnknownwhen a delete event is missed during a watch disconnect:https://pkg.go.dev/k8s.io/client-go/tools/cache#DeletedFinalStateUnknown
The neighboring
DeleteHyperNodehandler already unwraps the same tombstone type:volcano/pkg/controllers/hypernode/discovery/label/label.go
Lines 318 to 335 in 8a2c34c
The handler was introduced with label-based discovery in:
3346a39
Focused reproducer output:
Describe the results you received and expected
Actual: the tombstone is rejected, no discovery is queued, and stale Node membership can remain in the generated HyperNodes.
Expected:
DeleteNodeshould unwrap the Node fromDeletedFinalStateUnknownand enqueue topology discovery.What version of Volcano are you using?
master (8a2c34c)
Any other relevant information
Reproduced with a focused Go test using client-go v0.36.1. This was not observed against a release image in a live cluster.