fix(daemon): clean VLAN subinterfaces and label when excluding node from provider network (#6521)

* fix(daemon): clean VLAN subinterfaces and label when excluding node from provider network

cleanProviderNetwork was missing two cleanup operations compared to
handleDeleteProviderNetwork: it did not call cleanupAutoCreatedVlanInterfaces
to remove auto-created VLAN subinterfaces, and did not clear the
ProviderNetworkVlanIntTemplate node label. This caused VLAN subinterface
leaks and stale labels when a node was permanently excluded from a
ProviderNetwork.

Signed-off-by: Mengxin Liu <liumengxinfly@gmail.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Mengxin Liu <liumengxinfly@gmail.com>

* test(daemon): strengthen cleanProviderNetwork tests with JSON parsing

Address review feedback: parse patch JSON and verify label values are
explicitly null (removal) instead of just checking substring presence.
Also ensure tests fail if no PatchAction is recorded.

Signed-off-by: Mengxin Liu <liumengxinfly@gmail.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Mengxin Liu <liumengxinfly@gmail.com>

---------

Signed-off-by: Mengxin Liu <liumengxinfly@gmail.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: oilbeater

pkg/daemon/controller.go

@@ -535,13 +535,18 @@ func (c *Controller) cleanProviderNetwork(pn *kubeovnv1.ProviderNetwork, node *v
 		fmt.Sprintf(util.ProviderNetworkInterfaceTemplate, pn.Name): nil,
 		fmt.Sprintf(util.ProviderNetworkMtuTemplate, pn.Name):       nil,
 		fmt.Sprintf(util.ProviderNetworkExcludeTemplate, pn.Name):   "true",
+		fmt.Sprintf(util.ProviderNetworkVlanIntTemplate, pn.Name):   nil,
 	}
 	if err := util.PatchLabels(c.config.KubeClient.CoreV1().Nodes(), node.Name, patch); err != nil {
 		klog.Errorf("failed to patch labels of node %s: %v", node.Name, err)
 		return err
 	}
 
-	return c.ovsCleanProviderNetwork(pn.Name, providerNetworkNic(pn, node.Name))
+	if err := c.ovsCleanProviderNetwork(pn.Name, providerNetworkNic(pn, node.Name)); err != nil {
+		return err
+	}
+
+	return c.cleanupAutoCreatedVlanInterfaces(pn.Name, "", nil)
 }
 
 func (c *Controller) handleDeleteProviderNetwork(pn *kubeovnv1.ProviderNetwork) error {

pkg/daemon/controller_test.go

@@ -1,14 +1,21 @@
 package daemon
 
 import (
+	"encoding/json"
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes/fake"
 	listerv1 "k8s.io/client-go/listers/core/v1"
+	k8stesting "k8s.io/client-go/testing"
 	"k8s.io/client-go/tools/cache"
 	kubevirtv1 "kubevirt.io/api/core/v1"
+
+	kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1"
+	"github.com/kubeovn/kube-ovn/pkg/util"
 )
 
 func newLauncherPod(name, namespace, vmiName string, useNewLabel bool) *v1.Pod {
@@ -125,3 +132,111 @@ func TestIsVMLauncherPodAlive(t *testing.T) {
 		})
 	}
 }
+
+// parsePatchLabels extracts the metadata.labels map from a merge-patch JSON.
+// Each key maps to a *string (nil means the label is being removed).
+func parsePatchLabels(t *testing.T, patchBytes []byte) map[string]*string {
+	t.Helper()
+	var raw struct {
+		Metadata struct {
+			Labels map[string]*string `json:"labels"`
+		} `json:"metadata"`
+	}
+	require.NoError(t, json.Unmarshal(patchBytes, &raw))
+	return raw.Metadata.Labels
+}
+
+func TestCleanProviderNetworkPatchIncludesVlanIntLabel(t *testing.T) {
+	t.Parallel()
+
+	pnName := "test-pn"
+	nodeName := "test-node"
+
+	node := &v1.Node{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: nodeName,
+			Labels: map[string]string{
+				fmt.Sprintf(util.ProviderNetworkReadyTemplate, pnName):   "true",
+				fmt.Sprintf(util.ProviderNetworkVlanIntTemplate, pnName): "true",
+			},
+		},
+	}
+	fakeClient := fake.NewSimpleClientset(node)
+
+	c := &Controller{
+		config: &Configuration{KubeClient: fakeClient},
+	}
+	pn := &kubeovnv1.ProviderNetwork{
+		ObjectMeta: metav1.ObjectMeta{Name: pnName},
+		Spec:       kubeovnv1.ProviderNetworkSpec{DefaultInterface: "eth0"},
+	}
+
+	// PatchLabels is called before ovsCleanProviderNetwork, so the patch is
+	// captured even when ovsCleanProviderNetwork fails in a test environment.
+	_ = c.cleanProviderNetwork(pn, node)
+
+	vlanIntKey := fmt.Sprintf(util.ProviderNetworkVlanIntTemplate, pnName)
+	var patchFound bool
+	for _, action := range fakeClient.Actions() {
+		pa, ok := action.(k8stesting.PatchAction)
+		if !ok {
+			continue
+		}
+		labels := parsePatchLabels(t, pa.GetPatch())
+		val, exists := labels[vlanIntKey]
+		if exists {
+			require.Nil(t, val, "VlanIntTemplate label should be set to null (removal)")
+			patchFound = true
+			break
+		}
+	}
+	require.True(t, patchFound, "patch should include %s label cleanup", vlanIntKey)
+}
+
+func TestCleanProviderNetworkLabelConsistency(t *testing.T) {
+	t.Parallel()
+
+	pnName := "test-pn"
+
+	// All labels that handleDeleteProviderNetwork cleans (except ExcludeTemplate
+	// which is set to "true" in cleanProviderNetwork instead of nil).
+	expectedCleanedLabels := []string{
+		fmt.Sprintf(util.ProviderNetworkReadyTemplate, pnName),
+		fmt.Sprintf(util.ProviderNetworkInterfaceTemplate, pnName),
+		fmt.Sprintf(util.ProviderNetworkMtuTemplate, pnName),
+		fmt.Sprintf(util.ProviderNetworkVlanIntTemplate, pnName),
+	}
+
+	labels := make(map[string]string)
+	for _, key := range expectedCleanedLabels {
+		labels[key] = "true"
+	}
+	node := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1", Labels: labels}}
+	fakeClient := fake.NewSimpleClientset(node)
+
+	c := &Controller{config: &Configuration{KubeClient: fakeClient}}
+	pn := &kubeovnv1.ProviderNetwork{
+		ObjectMeta: metav1.ObjectMeta{Name: pnName},
+		Spec:       kubeovnv1.ProviderNetworkSpec{DefaultInterface: "eth0"},
+	}
+
+	_ = c.cleanProviderNetwork(pn, node)
+
+	var patchFound bool
+	for _, action := range fakeClient.Actions() {
+		pa, ok := action.(k8stesting.PatchAction)
+		if !ok {
+			continue
+		}
+		patchFound = true
+		patchLabels := parsePatchLabels(t, pa.GetPatch())
+		for _, key := range expectedCleanedLabels {
+			val, exists := patchLabels[key]
+			require.True(t, exists,
+				"cleanProviderNetwork should clean label %s (consistent with handleDeleteProviderNetwork)", key)
+			require.Nil(t, val,
+				"label %s should be set to null (removal)", key)
+		}
+	}
+	require.True(t, patchFound, "expected at least one PatchAction to be recorded")
+}