From 95a2626099d811e8decf5514a3fc45ebf9516383 Mon Sep 17 00:00:00 2001 From: Matteo Bunino <48362942+matbun@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:12:42 +0200 Subject: [PATCH 1/9] fix: keep the shadow of a pod recreated under the same name Shadow resources are named after the pod. When a pod is deleted and recreated under the same name (a StatefulSet replica, a Job retry with a fixed name), the new pod's shadow can be created before the old pod's cleanup runs, and that cleanup deleted the new shadow by name: the new pod ran without its tunnel. Every shadow resource is now labelled interlink.eu/shadow-owner-uid with the UID of the pod it serves, and the cleanup leaves alone resources stamped with another UID. Deletes are conditional on the resource's own UID, so a replacement created between the read and the delete survives. Unstamped resources, from earlier versions, are deleted as before. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matteo Bunino <48362942+matbun@users.noreply.github.com> --- pkg/virtualkubelet/shadow.go | 9 +- pkg/virtualkubelet/shadow_test.go | 59 ++++++++- pkg/virtualkubelet/virtualkubelet.go | 125 +++++++++++------- .../virtualkubelet_mesh_test.go | 4 +- 4 files changed, 142 insertions(+), 55 deletions(-) diff --git a/pkg/virtualkubelet/shadow.go b/pkg/virtualkubelet/shadow.go index a3ac3c1b..5df7cefb 100644 --- a/pkg/virtualkubelet/shadow.go +++ b/pkg/virtualkubelet/shadow.go @@ -153,6 +153,10 @@ func uniqueTruncate(s string, maxLen int, full string) string { type shadowResourceIdentity struct { Name string Namespace string + // OwnerUID is the UID of the pod the shadow serves. Shadow resources are named + // after the pod, so a pod recreated under the same name reuses those names; the + // UID, stamped as a label, tells which pod a resource belongs to. + OwnerUID string } func isShadowSameNamespace(pod *v1.Pod) bool { @@ -177,7 +181,7 @@ func computeShadowResourceIdentity(pod *v1.Pod) (shadowResourceIdentity, error) name, namespace = computeShadowResourceNames(pod.Name, pod.Namespace) } - identity := shadowResourceIdentity{Name: name, Namespace: namespace} + identity := shadowResourceIdentity{Name: name, Namespace: namespace, OwnerUID: string(pod.UID)} if len(fmt.Sprintf("%s-%s", identity.Name, identity.Namespace)) > 63 { return shadowResourceIdentity{}, fmt.Errorf("shadow ingress hostname label %q exceeds 63 characters; shorten pod/namespace or disable interlink.eu/shadow-same-ns", fmt.Sprintf("%s-%s", identity.Name, identity.Namespace)) } @@ -304,6 +308,7 @@ func (p *Provider) resetShadowNodeConfigMap(ctx context.Context, identity shadow }, Data: map[string]string{shadowNodeNameKey: "", shadowSocketDirKey: ""}, } + stampShadowOwner(cm, identity.OwnerUID) _, err := p.clientSet.CoreV1().ConfigMaps(identity.Namespace).Create(ctx, cm, metav1.CreateOptions{}) if err == nil { @@ -398,7 +403,7 @@ func (p *Provider) releaseShadowOfEndedPod(ctx context.Context, pod *v1.Pod) { return } log.G(ctx).Infof("Pod %s/%s is %s: removing its shadow", pod.Namespace, pod.Name, pod.Status.Phase) - p.cleanupShadowResources(ctx, identity.Name, identity.Namespace) + p.cleanupShadowResources(ctx, identity) p.forgetShadowNodeName(pod) } diff --git a/pkg/virtualkubelet/shadow_test.go b/pkg/virtualkubelet/shadow_test.go index 9282c9e0..4af2588e 100644 --- a/pkg/virtualkubelet/shadow_test.go +++ b/pkg/virtualkubelet/shadow_test.go @@ -205,7 +205,7 @@ func TestCleanupShadowResourcesRemovesNodeConfigMap(t *testing.T) { }) p := tunnelProvider(client) - p.cleanupShadowResources(t.Context(), name, ns) + p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns}) _, err := client.CoreV1().ConfigMaps(ns).Get(t.Context(), shadowNodeConfigMapName(name), metav1.GetOptions{}) assert.True(t, apierrors.IsNotFound(err), "the node configmap should be deleted with the rest of the shadow") @@ -387,3 +387,60 @@ func TestReleaseShadowIgnoresPodsWithoutAShadow(t *testing.T) { assert.Zero(t, deletes(client)) } + +// TestCleanupKeepsShadowOfRecreatedPod covers a pod deleted and recreated under the same +// name: the new pod's shadow has the same names, and the old pod's cleanup can run after +// it was created. Only resources stamped with the old pod's UID may go. +func TestCleanupKeepsShadowOfRecreatedPod(t *testing.T) { + const name = testPodDefaultName + ns := testNamespaceDefault + stamped := func(uid string) *appsv1.Deployment { + return &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{ + Name: name, Namespace: ns, Labels: map[string]string{shadowOwnerUIDLabel: uid}, + }} + } + + t.Run("the old pod's cleanup keeps the new pod's shadow", func(t *testing.T) { + client := fake.NewSimpleClientset(stamped("uid-new")) + p := tunnelProvider(client) + + p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: "uid-old"}) + + _, err := client.AppsV1().Deployments(ns).Get(t.Context(), name, metav1.GetOptions{}) + assert.NoError(t, err) + }) + + t.Run("the owner's cleanup removes it", func(t *testing.T) { + client := fake.NewSimpleClientset(stamped("uid-new")) + p := tunnelProvider(client) + + p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: "uid-new"}) + + _, err := client.AppsV1().Deployments(ns).Get(t.Context(), name, metav1.GetOptions{}) + assert.True(t, apierrors.IsNotFound(err)) + }) + + t.Run("an unstamped shadow from an older virtual kubelet is removed", func(t *testing.T) { + client := fake.NewSimpleClientset(&appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns}}) + p := tunnelProvider(client) + + p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: "uid-old"}) + + _, err := client.AppsV1().Deployments(ns).Get(t.Context(), name, metav1.GetOptions{}) + assert.True(t, apierrors.IsNotFound(err)) + }) +} + +func TestShadowIdentityAndNodeConfigMapCarryOwner(t *testing.T) { + pod := podWithPort("nb", testNamespaceDefault, "uid-7") + identity, err := computeShadowResourceIdentity(pod) + require.NoError(t, err) + assert.Equal(t, "uid-7", identity.OwnerUID) + + client := fake.NewSimpleClientset() + p := tunnelProvider(client) + require.NoError(t, p.resetShadowNodeConfigMap(t.Context(), identity)) + cm, err := client.CoreV1().ConfigMaps(identity.Namespace).Get(t.Context(), shadowNodeConfigMapName(identity.Name), metav1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, "uid-7", cm.Labels[shadowOwnerUIDLabel]) +} diff --git a/pkg/virtualkubelet/virtualkubelet.go b/pkg/virtualkubelet/virtualkubelet.go index e4b9b199..08b19fda 100644 --- a/pkg/virtualkubelet/virtualkubelet.go +++ b/pkg/virtualkubelet/virtualkubelet.go @@ -102,6 +102,7 @@ const ( annDisableOffloadInitContainers = "interlink.eu/disable-offload-init-containers" // comma-separated init container names annMeshNetworkDisabled = "interlink.eu/mesh-network" // set to "disabled" to opt out of mesh networking annShadowSocketPorts = "interlink.eu/shadow-socket-ports" // comma-separated exposed TCP ports served on unix sockets + shadowOwnerUIDLabel = "interlink.eu/shadow-owner-uid" ) type ShadowTemplateData struct { @@ -123,6 +124,8 @@ type ShadowTemplateData struct { IngressTLS bool IngressClusterIssuer string FullMesh bool + // OwnerUID is stamped on every shadow resource (see shadowResourceIdentity). + OwnerUID string // NodeConfigMap is the ConfigMap the shadow can mount to learn which remote // compute node the workload landed on. It exists from the moment the shadow is // created but holds an empty NodeNameKey until the plugin reports the node. @@ -1042,6 +1045,7 @@ func (p *Provider) createShadowPod(ctx context.Context, originalPod *v1.Pod) (*v IngressTLS: p.config.Network.IngressTLS, IngressClusterIssuer: p.config.Network.IngressClusterIssuer, FullMesh: fullMeshEnabledForPod, + OwnerUID: identity.OwnerUID, NodeConfigMap: shadowNodeConfigMapName(identity.Name), NodeNameKey: shadowNodeNameKey, SocketDirKey: shadowSocketDirKey, @@ -1391,6 +1395,9 @@ func (p *Provider) applyShadowManifests(ctx context.Context, manifestYAML string log.G(ctx).Warningf("Failed to decode resource: %v", err) continue } + if meta, ok := obj.(metav1.Object); ok && td != nil { + stampShadowOwner(meta, td.OwnerUID) + } switch o := obj.(type) { case *appsv1.Deployment: @@ -1484,61 +1491,82 @@ func (p *Provider) waitForDeploymentPod(ctx context.Context, deploymentName, nam return nil, fmt.Errorf("no pod found for deployment %s within timeout", deploymentName) } -// cleanupShadowResources removes all shadow resources for a given name and namespace -func (p *Provider) cleanupShadowResources(ctx context.Context, shadowName, namespace string) { +// cleanupShadowResources removes the shadow resources of the pod identity names. A +// resource stamped with another pod's UID is left alone: it belongs to a pod recreated +// under the same name, whose shadow can exist before this pod's cleanup runs. +func (p *Provider) cleanupShadowResources(ctx context.Context, identity shadowResourceIdentity) { + shadowName, namespace := identity.Name, identity.Namespace log.G(ctx).Infof("Cleaning up shadow resources for %s/%s", namespace, shadowName) - // Delete deployment - err := p.clientSet.AppsV1().Deployments(namespace).Delete(ctx, shadowName, metav1.DeleteOptions{}) - if err != nil { - log.G(ctx).Warningf("Failed to delete shadow deployment %s/%s: %v", namespace, shadowName, err) - } else { - log.G(ctx).Infof("Successfully deleted shadow deployment %s/%s", namespace, shadowName) - } + deployments := p.clientSet.AppsV1().Deployments(namespace) + p.deleteOwnedShadowResource(ctx, identity, "deployment", shadowName, + func(name string) (metav1.Object, error) { return deployments.Get(ctx, name, metav1.GetOptions{}) }, + func(name string, opts metav1.DeleteOptions) error { return deployments.Delete(ctx, name, opts) }) - // Delete service - err = p.clientSet.CoreV1().Services(namespace).Delete(ctx, shadowName, metav1.DeleteOptions{}) - if err != nil { - log.G(ctx).Warningf("Failed to delete shadow service %s/%s: %v", namespace, shadowName, err) - } else { - log.G(ctx).Infof("Successfully deleted shadow service %s/%s", namespace, shadowName) - } + services := p.clientSet.CoreV1().Services(namespace) + p.deleteOwnedShadowResource(ctx, identity, "service", shadowName, + func(name string) (metav1.Object, error) { return services.Get(ctx, name, metav1.GetOptions{}) }, + func(name string, opts metav1.DeleteOptions) error { return services.Delete(ctx, name, opts) }) - // Delete ingress - err = p.clientSet.NetworkingV1().Ingresses(namespace).Delete(ctx, shadowName, metav1.DeleteOptions{}) - if err != nil { - log.G(ctx).Warningf("Failed to delete shadow ingress %s/%s: %v", namespace, shadowName, err) - } else { - log.G(ctx).Infof("Successfully deleted shadow ingress %s/%s", namespace, shadowName) + ingresses := p.clientSet.NetworkingV1().Ingresses(namespace) + p.deleteOwnedShadowResource(ctx, identity, "ingress", shadowName, + func(name string) (metav1.Object, error) { return ingresses.Get(ctx, name, metav1.GetOptions{}) }, + func(name string, opts metav1.DeleteOptions) error { return ingresses.Delete(ctx, name, opts) }) + + configMaps := p.clientSet.CoreV1().ConfigMaps(namespace) + for _, name := range []string{shadowName + "-wg-config", shadowNodeConfigMapName(shadowName)} { + p.deleteOwnedShadowResource(ctx, identity, "configmap", name, + func(name string) (metav1.Object, error) { return configMaps.Get(ctx, name, metav1.GetOptions{}) }, + func(name string, opts metav1.DeleteOptions) error { return configMaps.Delete(ctx, name, opts) }) } - // Delete configmap - err = p.clientSet.CoreV1().ConfigMaps(namespace).Delete(ctx, shadowName+"-wg-config", metav1.DeleteOptions{}) - if err != nil { - log.G(ctx).Warningf("Failed to delete shadow configmap %s/%s: %v", namespace, shadowName+"-wg-config", err) - } else { - log.G(ctx).Infof("Successfully deleted shadow configmap %s/%s", namespace, shadowName+"-wg-config") + // Delete cert-manager-provisioned TLS secret. + if p.config.Network.IngressTLS { + secrets := p.clientSet.CoreV1().Secrets(namespace) + p.deleteOwnedShadowResource(ctx, identity, "secret", shadowName+"-tls", + func(name string) (metav1.Object, error) { return secrets.Get(ctx, name, metav1.GetOptions{}) }, + func(name string, opts metav1.DeleteOptions) error { return secrets.Delete(ctx, name, opts) }) } +} - // Delete the compute node configmap - nodeCM := shadowNodeConfigMapName(shadowName) - err = p.clientSet.CoreV1().ConfigMaps(namespace).Delete(ctx, nodeCM, metav1.DeleteOptions{}) +// deleteOwnedShadowResource deletes one shadow resource unless it is stamped with a +// pod UID other than identity.OwnerUID. The delete is conditional on the resource's +// own UID, so a replacement created between the read and the delete survives. +// Unstamped resources (from before the stamp existed) are deleted as before. +func (p *Provider) deleteOwnedShadowResource(ctx context.Context, identity shadowResourceIdentity, kind, name string, + get func(string) (metav1.Object, error), del func(string, metav1.DeleteOptions) error) { + obj, err := get(name) if err != nil { - log.G(ctx).Warningf("Failed to delete shadow configmap %s/%s: %v", namespace, nodeCM, err) - } else { - log.G(ctx).Infof("Successfully deleted shadow configmap %s/%s", namespace, nodeCM) + if !apierrors.IsNotFound(err) { + log.G(ctx).Warningf("Failed to read shadow %s %s/%s: %v", kind, identity.Namespace, name, err) + } + return } - - // Delete cert-manager-provisioned TLS secret. - if p.config.Network.IngressTLS { - secretName := shadowName + "-tls" - err = p.clientSet.CoreV1().Secrets(namespace).Delete(ctx, secretName, metav1.DeleteOptions{}) - if err != nil { - log.G(ctx).Warningf("Failed to delete shadow TLS secret %s/%s: %v", namespace, secretName, err) - } else { - log.G(ctx).Infof("Successfully deleted shadow TLS secret %s/%s", namespace, secretName) + if owner := obj.GetLabels()[shadowOwnerUIDLabel]; owner != "" && identity.OwnerUID != "" && owner != identity.OwnerUID { + log.G(ctx).Infof("Keeping shadow %s %s/%s: it belongs to pod UID %s, not %s", kind, identity.Namespace, name, owner, identity.OwnerUID) + return + } + uid := obj.GetUID() + if err := del(name, metav1.DeleteOptions{Preconditions: &metav1.Preconditions{UID: &uid}}); err != nil { + if !apierrors.IsNotFound(err) && !apierrors.IsConflict(err) { + log.G(ctx).Warningf("Failed to delete shadow %s %s/%s: %v", kind, identity.Namespace, name, err) } + return + } + log.G(ctx).Infof("Successfully deleted shadow %s %s/%s", kind, identity.Namespace, name) +} + +// stampShadowOwner labels a shadow resource with the UID of the pod it serves. +func stampShadowOwner(obj metav1.Object, ownerUID string) { + if ownerUID == "" { + return } + labels := obj.GetLabels() + if labels == nil { + labels = map[string]string{} + } + labels[shadowOwnerUIDLabel] = ownerUID + obj.SetLabels(labels) } // cleanupPartialShadowResources removes specific resources that were created before a failure @@ -1833,7 +1861,7 @@ func (p *Provider) handleShadowCreation(ctx context.Context, pod *v1.Pod) (strin if err != nil { log.G(ctx).Errorf("Failed to create shadow infrastructure for %s/%s: %v", pod.Namespace, pod.Name, err) // Clean up any partially created resources - p.cleanupShadowResources(ctx, identity.Name, identity.Namespace) + p.cleanupShadowResources(ctx, identity) return "", fmt.Errorf("failed to create shadow infrastructure for exposed ports: %w", err) } @@ -1845,10 +1873,7 @@ func (p *Provider) handleShadowCreation(ctx context.Context, pod *v1.Pod) (strin } } - podIP, err := p.waitForShadowPodIP(ctx, shadowPod, timeout, shadowResourceIdentity{ - Name: templateData.Name, - Namespace: templateData.Namespace, - }) + podIP, err := p.waitForShadowPodIP(ctx, shadowPod, timeout, identity) if err != nil { log.G(ctx).Errorf("Failed to get shadow pod IP for %s/%s: %v", pod.Namespace, pod.Name, err) return "", err @@ -1891,7 +1916,7 @@ func (p *Provider) waitForShadowPodIP(ctx context.Context, shadowPod *v1.Pod, ti } // Clean up the shadow infrastructure since it didn't get an IP - p.cleanupShadowResources(ctx, identity.Name, identity.Namespace) + p.cleanupShadowResources(ctx, identity) return "", fmt.Errorf("shadow pod %s/%s failed to get an IP within %v timeout", shadowPod.Namespace, shadowPod.Name, timeout) } @@ -2145,7 +2170,7 @@ func (p *Provider) DeletePod(ctx context.Context, pod *v1.Pod) (err error) { if identityErr != nil { log.G(ctx).Warningf("Failed to compute shadow resource identity for %s/%s: %v", pod.Namespace, pod.Name, identityErr) } else { - p.cleanupShadowResources(ctx, identity.Name, identity.Namespace) + p.cleanupShadowResources(ctx, identity) } } p.forgetShadowNodeName(pod) diff --git a/pkg/virtualkubelet/virtualkubelet_mesh_test.go b/pkg/virtualkubelet/virtualkubelet_mesh_test.go index 488d9a8d..d37f5a41 100644 --- a/pkg/virtualkubelet/virtualkubelet_mesh_test.go +++ b/pkg/virtualkubelet/virtualkubelet_mesh_test.go @@ -312,7 +312,7 @@ func TestCleanupShadowResources(t *testing.T) { client := newClient() p := &Provider{clientSet: client, config: Config{Network: Network{IngressTLS: true}}} - p.cleanupShadowResources(t.Context(), name, ns) + p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns}) _, err := client.AppsV1().Deployments(ns).Get(t.Context(), name, metav1.GetOptions{}) assert.True(t, apierrors.IsNotFound(err), "deployment should be deleted") @@ -330,7 +330,7 @@ func TestCleanupShadowResources(t *testing.T) { client := newClient() p := &Provider{clientSet: client, config: Config{Network: Network{IngressTLS: false}}} - p.cleanupShadowResources(t.Context(), name, ns) + p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns}) // Core resources are still removed regardless of TLS... _, err := client.AppsV1().Deployments(ns).Get(t.Context(), name, metav1.GetOptions{}) From 8a481a35f59ca75c769cbfddffcd50616a97cf00 Mon Sep 17 00:00:00 2001 From: Matteo Bunino <48362942+matbun@users.noreply.github.com> Date: Fri, 23 Oct 2026 12:01:10 +0200 Subject: [PATCH 2/9] fix: keep a recreated pod's TLS secret and in-place updated resources Two cases where the old pod's cleanup could still remove what the recreated pod's shadow uses: - cert-manager creates the -tls Secret without the owner label, so it counted as a legacy resource and was deleted even when the Ingress it serves belongs to the new pod. The Secret now follows its Ingress: it is kept when the Ingress is kept. - The applyOrUpdate helpers update a resource in place for a recreated pod, keeping its UID. A delete conditional on the UID alone still succeeded if the update landed between the read and the delete; the delete now also requires the resourceVersion that was read. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matteo Bunino <48362942+matbun@users.noreply.github.com> --- pkg/virtualkubelet/shadow_test.go | 62 +++++++++++++++++++++++++--- pkg/virtualkubelet/virtualkubelet.go | 29 +++++++------ 2 files changed, 74 insertions(+), 17 deletions(-) diff --git a/pkg/virtualkubelet/shadow_test.go b/pkg/virtualkubelet/shadow_test.go index 4af2588e..ba79e760 100644 --- a/pkg/virtualkubelet/shadow_test.go +++ b/pkg/virtualkubelet/shadow_test.go @@ -8,8 +8,10 @@ import ( "github.com/stretchr/testify/require" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" k8stypes "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/fake" k8stesting "k8s.io/client-go/testing" @@ -393,6 +395,7 @@ func TestReleaseShadowIgnoresPodsWithoutAShadow(t *testing.T) { // it was created. Only resources stamped with the old pod's UID may go. func TestCleanupKeepsShadowOfRecreatedPod(t *testing.T) { const name = testPodDefaultName + const oldUID, newUID = "uid-old", "uid-new" ns := testNamespaceDefault stamped := func(uid string) *appsv1.Deployment { return &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{ @@ -401,20 +404,20 @@ func TestCleanupKeepsShadowOfRecreatedPod(t *testing.T) { } t.Run("the old pod's cleanup keeps the new pod's shadow", func(t *testing.T) { - client := fake.NewSimpleClientset(stamped("uid-new")) + client := fake.NewSimpleClientset(stamped(newUID)) p := tunnelProvider(client) - p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: "uid-old"}) + p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: oldUID}) _, err := client.AppsV1().Deployments(ns).Get(t.Context(), name, metav1.GetOptions{}) assert.NoError(t, err) }) t.Run("the owner's cleanup removes it", func(t *testing.T) { - client := fake.NewSimpleClientset(stamped("uid-new")) + client := fake.NewSimpleClientset(stamped(newUID)) p := tunnelProvider(client) - p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: "uid-new"}) + p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: newUID}) _, err := client.AppsV1().Deployments(ns).Get(t.Context(), name, metav1.GetOptions{}) assert.True(t, apierrors.IsNotFound(err)) @@ -424,11 +427,60 @@ func TestCleanupKeepsShadowOfRecreatedPod(t *testing.T) { client := fake.NewSimpleClientset(&appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns}}) p := tunnelProvider(client) - p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: "uid-old"}) + p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: oldUID}) _, err := client.AppsV1().Deployments(ns).Get(t.Context(), name, metav1.GetOptions{}) assert.True(t, apierrors.IsNotFound(err)) }) + + // cert-manager creates the TLS Secret without the owner label: it follows its Ingress. + tlsObjects := func(owner string) []runtime.Object { + return []runtime.Object{ + &networkingv1.Ingress{ObjectMeta: metav1.ObjectMeta{ + Name: name, Namespace: ns, Labels: map[string]string{shadowOwnerUIDLabel: owner}, + }}, + &v1.Secret{ObjectMeta: metav1.ObjectMeta{Name: name + "-tls", Namespace: ns}}, + } + } + for _, tc := range []struct { + cleanup string + kept bool + }{{cleanup: oldUID, kept: true}, {cleanup: newUID, kept: false}} { + t.Run("the TLS secret follows its ingress, cleanup by "+tc.cleanup, func(t *testing.T) { + client := fake.NewSimpleClientset(tlsObjects(newUID)...) + p := tunnelProvider(client) + p.config.Network.IngressTLS = true + + p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: tc.cleanup}) + + _, err := client.CoreV1().Secrets(ns).Get(t.Context(), name+"-tls", metav1.GetOptions{}) + if tc.kept { + assert.NoError(t, err) + } else { + assert.True(t, apierrors.IsNotFound(err)) + } + }) + } + + t.Run("the delete is conditional on the resource version that was read", func(t *testing.T) { + deployment := stamped(oldUID) + deployment.UID, deployment.ResourceVersion = "deployment-uid", "41" + client := fake.NewSimpleClientset(deployment) + var preconditions *metav1.Preconditions + client.PrependReactor("delete", "deployments", func(action k8stesting.Action) (bool, runtime.Object, error) { + preconditions = action.(k8stesting.DeleteAction).GetDeleteOptions().Preconditions + return false, nil, nil + }) + p := tunnelProvider(client) + + p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: oldUID}) + + require.NotNil(t, preconditions) + require.NotNil(t, preconditions.UID) + require.NotNil(t, preconditions.ResourceVersion) + assert.Equal(t, k8stypes.UID("deployment-uid"), *preconditions.UID) + assert.Equal(t, "41", *preconditions.ResourceVersion) + }) } func TestShadowIdentityAndNodeConfigMapCarryOwner(t *testing.T) { diff --git a/pkg/virtualkubelet/virtualkubelet.go b/pkg/virtualkubelet/virtualkubelet.go index 08b19fda..1eac5f36 100644 --- a/pkg/virtualkubelet/virtualkubelet.go +++ b/pkg/virtualkubelet/virtualkubelet.go @@ -1509,7 +1509,7 @@ func (p *Provider) cleanupShadowResources(ctx context.Context, identity shadowRe func(name string, opts metav1.DeleteOptions) error { return services.Delete(ctx, name, opts) }) ingresses := p.clientSet.NetworkingV1().Ingresses(namespace) - p.deleteOwnedShadowResource(ctx, identity, "ingress", shadowName, + ingressKept := p.deleteOwnedShadowResource(ctx, identity, "ingress", shadowName, func(name string) (metav1.Object, error) { return ingresses.Get(ctx, name, metav1.GetOptions{}) }, func(name string, opts metav1.DeleteOptions) error { return ingresses.Delete(ctx, name, opts) }) @@ -1520,8 +1520,9 @@ func (p *Provider) cleanupShadowResources(ctx context.Context, identity shadowRe func(name string, opts metav1.DeleteOptions) error { return configMaps.Delete(ctx, name, opts) }) } - // Delete cert-manager-provisioned TLS secret. - if p.config.Network.IngressTLS { + // Delete cert-manager-provisioned TLS secret. cert-manager creates it without the + // owner label, so it goes with its Ingress: kept when another pod's Ingress is. + if p.config.Network.IngressTLS && !ingressKept { secrets := p.clientSet.CoreV1().Secrets(namespace) p.deleteOwnedShadowResource(ctx, identity, "secret", shadowName+"-tls", func(name string) (metav1.Object, error) { return secrets.Get(ctx, name, metav1.GetOptions{}) }, @@ -1530,30 +1531,34 @@ func (p *Provider) cleanupShadowResources(ctx context.Context, identity shadowRe } // deleteOwnedShadowResource deletes one shadow resource unless it is stamped with a -// pod UID other than identity.OwnerUID. The delete is conditional on the resource's -// own UID, so a replacement created between the read and the delete survives. -// Unstamped resources (from before the stamp existed) are deleted as before. +// pod UID other than identity.OwnerUID, and reports whether it was kept for that +// reason. The delete is conditional on the resource's UID and resourceVersion, so +// neither a replacement created nor an in-place update for a recreated pod made +// between the read and the delete is removed. Unstamped resources (from before the +// stamp existed) are deleted as before. func (p *Provider) deleteOwnedShadowResource(ctx context.Context, identity shadowResourceIdentity, kind, name string, - get func(string) (metav1.Object, error), del func(string, metav1.DeleteOptions) error) { + get func(string) (metav1.Object, error), del func(string, metav1.DeleteOptions) error) bool { obj, err := get(name) if err != nil { if !apierrors.IsNotFound(err) { log.G(ctx).Warningf("Failed to read shadow %s %s/%s: %v", kind, identity.Namespace, name, err) } - return + return false } if owner := obj.GetLabels()[shadowOwnerUIDLabel]; owner != "" && identity.OwnerUID != "" && owner != identity.OwnerUID { log.G(ctx).Infof("Keeping shadow %s %s/%s: it belongs to pod UID %s, not %s", kind, identity.Namespace, name, owner, identity.OwnerUID) - return + return true } - uid := obj.GetUID() - if err := del(name, metav1.DeleteOptions{Preconditions: &metav1.Preconditions{UID: &uid}}); err != nil { + uid, resourceVersion := obj.GetUID(), obj.GetResourceVersion() + preconditions := &metav1.Preconditions{UID: &uid, ResourceVersion: &resourceVersion} + if err := del(name, metav1.DeleteOptions{Preconditions: preconditions}); err != nil { if !apierrors.IsNotFound(err) && !apierrors.IsConflict(err) { log.G(ctx).Warningf("Failed to delete shadow %s %s/%s: %v", kind, identity.Namespace, name, err) } - return + return false } log.G(ctx).Infof("Successfully deleted shadow %s %s/%s", kind, identity.Namespace, name) + return false } // stampShadowOwner labels a shadow resource with the UID of the pod it serves. From 78f7a151bb678d0772f8bbe70e813bb642a05b14 Mon Sep 17 00:00:00 2001 From: Matteo Bunino <48362942+matbun@users.noreply.github.com> Date: Fri, 23 Oct 2026 13:14:53 +0200 Subject: [PATCH 3/9] fix: re-check the owner when a shadow delete conflicts With the resourceVersion precondition, any update between the read and the delete made the delete fail, including the Deployment status updates the controller keeps writing, so the old pod's shadow could be left behind. A conflict now re-reads the resource and checks its owner again: it is deleted if it is still the pod's own, and kept if it now serves the recreated pod. A conflict counts as kept for the TLS Secret too, which was deleted when the Ingress changed between the read and the delete. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matteo Bunino <48362942+matbun@users.noreply.github.com> --- pkg/virtualkubelet/shadow_test.go | 48 +++++++++++++++++++++++++ pkg/virtualkubelet/virtualkubelet.go | 53 ++++++++++++++++------------ 2 files changed, 78 insertions(+), 23 deletions(-) diff --git a/pkg/virtualkubelet/shadow_test.go b/pkg/virtualkubelet/shadow_test.go index ba79e760..6183e67e 100644 --- a/pkg/virtualkubelet/shadow_test.go +++ b/pkg/virtualkubelet/shadow_test.go @@ -1,6 +1,7 @@ package virtualkubelet import ( + "errors" "strings" "testing" @@ -12,6 +13,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" k8stypes "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/fake" k8stesting "k8s.io/client-go/testing" @@ -462,6 +464,52 @@ func TestCleanupKeepsShadowOfRecreatedPod(t *testing.T) { }) } + // conflictOnce makes the first delete of resource fail with a conflict, after + // changing the stored object with change, as an update landing between read and delete. + conflictOnce := func(client *fake.Clientset, resource string, gvr schema.GroupVersionResource, change func(metav1.Object)) { + first := true + client.PrependReactor("delete", resource, func(k8stesting.Action) (bool, runtime.Object, error) { + if !first { + return false, nil, nil + } + first = false + obj, err := client.Tracker().Get(gvr, ns, name) + require.NoError(t, err) + change(obj.(metav1.Object)) + require.NoError(t, client.Tracker().Update(gvr, obj, ns)) + return true, nil, apierrors.NewConflict(gvr.GroupResource(), name, errors.New("resourceVersion changed")) + }) + } + + t.Run("an ingress taken over between read and delete keeps it and its TLS secret", func(t *testing.T) { + client := fake.NewSimpleClientset(tlsObjects(oldUID)...) + conflictOnce(client, "ingresses", networkingv1.SchemeGroupVersion.WithResource("ingresses"), func(o metav1.Object) { + o.SetLabels(map[string]string{shadowOwnerUIDLabel: newUID}) + }) + p := tunnelProvider(client) + p.config.Network.IngressTLS = true + + p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: oldUID}) + + _, err := client.NetworkingV1().Ingresses(ns).Get(t.Context(), name, metav1.GetOptions{}) + assert.NoError(t, err) + _, err = client.CoreV1().Secrets(ns).Get(t.Context(), name+"-tls", metav1.GetOptions{}) + assert.NoError(t, err) + }) + + t.Run("any other change between read and delete is retried", func(t *testing.T) { + client := fake.NewSimpleClientset(stamped(oldUID)) + conflictOnce(client, "deployments", appsv1.SchemeGroupVersion.WithResource("deployments"), func(o metav1.Object) { + o.SetAnnotations(map[string]string{"status": "updated"}) + }) + p := tunnelProvider(client) + + p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: oldUID}) + + _, err := client.AppsV1().Deployments(ns).Get(t.Context(), name, metav1.GetOptions{}) + assert.True(t, apierrors.IsNotFound(err)) + }) + t.Run("the delete is conditional on the resource version that was read", func(t *testing.T) { deployment := stamped(oldUID) deployment.UID, deployment.ResourceVersion = "deployment-uid", "41" diff --git a/pkg/virtualkubelet/virtualkubelet.go b/pkg/virtualkubelet/virtualkubelet.go index 1eac5f36..7ee922e4 100644 --- a/pkg/virtualkubelet/virtualkubelet.go +++ b/pkg/virtualkubelet/virtualkubelet.go @@ -39,6 +39,7 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/clientcmd" + "k8s.io/client-go/util/retry" stats "k8s.io/kubelet/pkg/apis/stats/v1alpha1" types "github.com/interlink-hq/interlink/pkg/interlink" @@ -1531,34 +1532,40 @@ func (p *Provider) cleanupShadowResources(ctx context.Context, identity shadowRe } // deleteOwnedShadowResource deletes one shadow resource unless it is stamped with a -// pod UID other than identity.OwnerUID, and reports whether it was kept for that -// reason. The delete is conditional on the resource's UID and resourceVersion, so -// neither a replacement created nor an in-place update for a recreated pod made -// between the read and the delete is removed. Unstamped resources (from before the -// stamp existed) are deleted as before. +// pod UID other than identity.OwnerUID, and reports whether it was kept because it +// serves another pod. The delete is conditional on the UID and resourceVersion that +// were read, so a replacement or an in-place update for a recreated pod made in +// between is never removed: on a conflict the resource is read again and the owner +// checked again. Unstamped resources (from before the stamp existed) are deleted as +// before. func (p *Provider) deleteOwnedShadowResource(ctx context.Context, identity shadowResourceIdentity, kind, name string, get func(string) (metav1.Object, error), del func(string, metav1.DeleteOptions) error) bool { - obj, err := get(name) - if err != nil { - if !apierrors.IsNotFound(err) { - log.G(ctx).Warningf("Failed to read shadow %s %s/%s: %v", kind, identity.Namespace, name, err) + kept := false + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + obj, err := get(name) + if err != nil { + return err } - return false - } - if owner := obj.GetLabels()[shadowOwnerUIDLabel]; owner != "" && identity.OwnerUID != "" && owner != identity.OwnerUID { - log.G(ctx).Infof("Keeping shadow %s %s/%s: it belongs to pod UID %s, not %s", kind, identity.Namespace, name, owner, identity.OwnerUID) - return true - } - uid, resourceVersion := obj.GetUID(), obj.GetResourceVersion() - preconditions := &metav1.Preconditions{UID: &uid, ResourceVersion: &resourceVersion} - if err := del(name, metav1.DeleteOptions{Preconditions: preconditions}); err != nil { - if !apierrors.IsNotFound(err) && !apierrors.IsConflict(err) { - log.G(ctx).Warningf("Failed to delete shadow %s %s/%s: %v", kind, identity.Namespace, name, err) + if owner := obj.GetLabels()[shadowOwnerUIDLabel]; owner != "" && identity.OwnerUID != "" && owner != identity.OwnerUID { + log.G(ctx).Infof("Keeping shadow %s %s/%s: it belongs to pod UID %s, not %s", kind, identity.Namespace, name, owner, identity.OwnerUID) + kept = true + return nil } - return false + uid, resourceVersion := obj.GetUID(), obj.GetResourceVersion() + return del(name, metav1.DeleteOptions{Preconditions: &metav1.Preconditions{UID: &uid, ResourceVersion: &resourceVersion}}) + }) + switch { + case err == nil: + if !kept { + log.G(ctx).Infof("Successfully deleted shadow %s %s/%s", kind, identity.Namespace, name) + } + case apierrors.IsConflict(err): + log.G(ctx).Warningf("Keeping shadow %s %s/%s: it kept changing while being deleted", kind, identity.Namespace, name) + kept = true + case !apierrors.IsNotFound(err): + log.G(ctx).Warningf("Failed to delete shadow %s %s/%s: %v", kind, identity.Namespace, name, err) } - log.G(ctx).Infof("Successfully deleted shadow %s %s/%s", kind, identity.Namespace, name) - return false + return kept } // stampShadowOwner labels a shadow resource with the UID of the pod it serves. From 957e5d0ec318ece7dc5e7c67a1b557acc9657a76 Mon Sep 17 00:00:00 2001 From: Matteo Bunino <48362942+matbun@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:56:22 +0200 Subject: [PATCH 4/9] fix: serialise shadow cleanup with the shadow of a same-name successor The cleanup of a deleted pod could delete the old Ingress, let the pod recreated under the same name create its new one, and then delete the TLS Secret that new Ingress uses: the Secret carries no owner label and creating the Ingress does not change it. Applying a shadow's manifests and cleaning them up now hold a lock per shadow name, one of a fixed set of 64, so the two never interleave in the virtual kubelet. The wait for the shadow pod stays outside the lock. An error other than NotFound while reading or deleting the Ingress now counts as kept too, so the TLS Secret is not deleted when its Ingress's owner is unknown. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matteo Bunino <48362942+matbun@users.noreply.github.com> --- pkg/virtualkubelet/shadow_test.go | 61 ++++++++++++++++++++++++ pkg/virtualkubelet/virtualkubelet.go | 69 ++++++++++++++++++++-------- 2 files changed, 112 insertions(+), 18 deletions(-) diff --git a/pkg/virtualkubelet/shadow_test.go b/pkg/virtualkubelet/shadow_test.go index 6183e67e..2383c70c 100644 --- a/pkg/virtualkubelet/shadow_test.go +++ b/pkg/virtualkubelet/shadow_test.go @@ -4,6 +4,7 @@ import ( "errors" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -510,6 +511,20 @@ func TestCleanupKeepsShadowOfRecreatedPod(t *testing.T) { assert.True(t, apierrors.IsNotFound(err)) }) + t.Run("the TLS secret stays when its ingress cannot be read", func(t *testing.T) { + client := fake.NewSimpleClientset(tlsObjects(newUID)...) + client.PrependReactor("get", "ingresses", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewInternalError(errors.New("etcd timeout")) + }) + p := tunnelProvider(client) + p.config.Network.IngressTLS = true + + p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: oldUID}) + + _, err := client.CoreV1().Secrets(ns).Get(t.Context(), name+"-tls", metav1.GetOptions{}) + assert.NoError(t, err) + }) + t.Run("the delete is conditional on the resource version that was read", func(t *testing.T) { deployment := stamped(oldUID) deployment.UID, deployment.ResourceVersion = "deployment-uid", "41" @@ -531,6 +546,52 @@ func TestCleanupKeepsShadowOfRecreatedPod(t *testing.T) { }) } +// The cleanup of a deleted pod and the shadow of its same-name successor must not +// interleave: the new Ingress could otherwise appear between the old Ingress and the +// TLS Secret being deleted, and lose the Secret. +func TestShadowCleanupAndApplyAreSerialised(t *testing.T) { + const name = testPodDefaultName + ns := testNamespaceDefault + waitsForLock := func(t *testing.T, p *Provider, run func()) { + t.Helper() + unlock := p.lockShadow(ns, name) + done := make(chan struct{}) + go func() { run(); close(done) }() + select { + case <-done: + unlock() + t.Fatal("ran while the shadow's lock was held") + case <-time.After(100 * time.Millisecond): + } + unlock() + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("did not run once the lock was released") + } + } + + t.Run("cleanup", func(t *testing.T) { + client := fake.NewSimpleClientset() + p := tunnelProvider(client) + waitsForLock(t, p, func() { + p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns}) + }) + }) + + t.Run("apply", func(t *testing.T) { + client := fake.NewSimpleClientset() + p := tunnelProvider(client) + manifest := "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: " + name + "\n namespace: " + ns + "\n" + waitsForLock(t, p, func() { + _, _, err := p.applyShadowObjects(t.Context(), manifest, &ShadowTemplateData{Name: name, Namespace: ns}) + assert.NoError(t, err) + }) + _, err := client.CoreV1().ConfigMaps(ns).Get(t.Context(), name, metav1.GetOptions{}) + assert.NoError(t, err) + }) +} + func TestShadowIdentityAndNodeConfigMapCarryOwner(t *testing.T) { pod := podWithPort("nb", testNamespaceDefault, "uid-7") identity, err := computeShadowResourceIdentity(pod) diff --git a/pkg/virtualkubelet/virtualkubelet.go b/pkg/virtualkubelet/virtualkubelet.go index db47dec9..bd89fb8d 100644 --- a/pkg/virtualkubelet/virtualkubelet.go +++ b/pkg/virtualkubelet/virtualkubelet.go @@ -9,6 +9,7 @@ import ( "encoding/hex" "encoding/json" "fmt" + "hash/fnv" "io" mathrand "math/rand" "net" @@ -205,7 +206,10 @@ type Provider struct { shadowsReleased sync.Map // keptInitWaits holds, per pod UID, the cancel function of a pending wait for the // pod's kept init containers (see stopKeptInitWait). - keptInitWaits sync.Map + keptInitWaits sync.Map + // shadowLocks serialise applying a shadow's manifests and cleaning them up, per + // shadow name (see lockShadow). + shadowLocks [64]sync.Mutex pendingDeletes map[string]*pendingDelete pendingDeletesMu sync.Mutex deleteLoopOnce sync.Once @@ -1413,6 +1417,24 @@ func (p *Provider) applyOrUpdateSecret(ctx context.Context, o *v1.Secret) error // applyShadowManifests applies the generated manifests and returns the first created pod func (p *Provider) applyShadowManifests(ctx context.Context, manifestYAML string, td *ShadowTemplateData) (*v1.Pod, error) { + deploymentName, namespace, err := p.applyShadowObjects(ctx, manifestYAML, td) + if err != nil { + return nil, err + } + if deploymentName != "" { + return p.waitForDeploymentPod(ctx, deploymentName, namespace) + } + + return nil, fmt.Errorf("no deployment found in manifests") +} + +// applyShadowObjects creates or updates the objects of the shadow manifests and returns +// the name and namespace of its Deployment. It holds the shadow's lock meanwhile, so the +// cleanup of a deleted pod with the same name never runs in between. +func (p *Provider) applyShadowObjects(ctx context.Context, manifestYAML string, td *ShadowTemplateData) (string, string, error) { + if td != nil { + defer p.lockShadow(td.Namespace, td.Name)() + } resources := strings.Split(manifestYAML, "---") decoder := serializer.NewCodecFactory(scheme.Scheme).UniversalDeserializer() var deploymentName string @@ -1439,7 +1461,7 @@ func (p *Provider) applyShadowManifests(ctx context.Context, manifestYAML string name, ns, err := p.applyOrUpdateDeployment(ctx, o, td) if err != nil { p.cleanupPartialShadowResources(ctx, createdResources, o.Namespace) - return nil, err + return "", "", err } deploymentName = name namespace = ns @@ -1448,28 +1470,28 @@ func (p *Provider) applyShadowManifests(ctx context.Context, manifestYAML string case *v1.Service: if err := p.applyOrUpdateService(ctx, o); err != nil { p.cleanupPartialShadowResources(ctx, createdResources, o.Namespace) - return nil, err + return "", "", err } createdResources = append(createdResources, "service:"+o.Name) case *networkingv1.Ingress: if err := p.applyOrUpdateIngress(ctx, o); err != nil { p.cleanupPartialShadowResources(ctx, createdResources, o.Namespace) - return nil, err + return "", "", err } createdResources = append(createdResources, "ingress:"+o.Name) case *v1.ConfigMap: if err := p.applyOrUpdateConfigMap(ctx, o); err != nil { p.cleanupPartialShadowResources(ctx, createdResources, o.Namespace) - return nil, err + return "", "", err } createdResources = append(createdResources, "configmap:"+o.Name) case *v1.Secret: if err := p.applyOrUpdateSecret(ctx, o); err != nil { p.cleanupPartialShadowResources(ctx, createdResources, o.Namespace) - return nil, err + return "", "", err } createdResources = append(createdResources, "secret:"+o.Name) @@ -1478,11 +1500,7 @@ func (p *Provider) applyShadowManifests(ctx context.Context, manifestYAML string } } - if deploymentName != "" { - return p.waitForDeploymentPod(ctx, deploymentName, namespace) - } - - return nil, fmt.Errorf("no deployment found in manifests") + return deploymentName, namespace, nil } // waitForDeploymentPod waits for a deployment to create a pod and returns it. @@ -1528,9 +1546,11 @@ func (p *Provider) waitForDeploymentPod(ctx context.Context, deploymentName, nam // cleanupShadowResources removes the shadow resources of the pod identity names. A // resource stamped with another pod's UID is left alone: it belongs to a pod recreated -// under the same name, whose shadow can exist before this pod's cleanup runs. +// under the same name, whose shadow can exist before this pod's cleanup runs. The +// shadow's lock keeps that shadow from being applied while the cleanup runs. func (p *Provider) cleanupShadowResources(ctx context.Context, identity shadowResourceIdentity) { shadowName, namespace := identity.Name, identity.Namespace + defer p.lockShadow(namespace, shadowName)() log.G(ctx).Infof("Cleaning up shadow resources for %s/%s", namespace, shadowName) deployments := p.clientSet.AppsV1().Deployments(namespace) @@ -1566,12 +1586,12 @@ func (p *Provider) cleanupShadowResources(ctx context.Context, identity shadowRe } // deleteOwnedShadowResource deletes one shadow resource unless it is stamped with a -// pod UID other than identity.OwnerUID, and reports whether it was kept because it -// serves another pod. The delete is conditional on the UID and resourceVersion that -// were read, so a replacement or an in-place update for a recreated pod made in -// between is never removed: on a conflict the resource is read again and the owner -// checked again. Unstamped resources (from before the stamp existed) are deleted as -// before. +// pod UID other than identity.OwnerUID, and reports whether it may still serve another +// pod: kept for its owner, changed meanwhile, or left in place by an error. The delete +// is conditional on the UID and resourceVersion that were read, so a replacement or an +// in-place update for a recreated pod made in between is never removed: on a conflict +// the resource is read again and the owner checked again. Unstamped resources (from +// before the stamp existed) are deleted as before. func (p *Provider) deleteOwnedShadowResource(ctx context.Context, identity shadowResourceIdentity, kind, name string, get func(string) (metav1.Object, error), del func(string, metav1.DeleteOptions) error) bool { kept := false @@ -1597,11 +1617,24 @@ func (p *Provider) deleteOwnedShadowResource(ctx context.Context, identity shado log.G(ctx).Warningf("Keeping shadow %s %s/%s: it kept changing while being deleted", kind, identity.Namespace, name) kept = true case !apierrors.IsNotFound(err): + // Its owner is unknown, so it may serve another pod. log.G(ctx).Warningf("Failed to delete shadow %s %s/%s: %v", kind, identity.Namespace, name, err) + kept = true } return kept } +// lockShadow locks the shadow named name in namespace and returns the unlock function. +// The lock is one of a fixed set, chosen by name, so it is shared with unrelated shadows +// now and then; it only orders a cleanup and an apply within this virtual kubelet. +func (p *Provider) lockShadow(namespace, name string) func() { + h := fnv.New32a() + _, _ = h.Write([]byte(namespace + "/" + name)) + mu := &p.shadowLocks[h.Sum32()%uint32(len(p.shadowLocks))] + mu.Lock() + return mu.Unlock +} + // stampShadowOwner labels a shadow resource with the UID of the pod it serves. func stampShadowOwner(obj metav1.Object, ownerUID string) { if ownerUID == "" { From 5313636bce4c92bf0e0b46eb88a4524a16306cc7 Mon Sep 17 00:00:00 2001 From: Matteo Bunino <48362942+matbun@users.noreply.github.com> Date: Fri, 23 Oct 2026 16:13:24 +0200 Subject: [PATCH 5/9] fix: protect the node ConfigMap of a pod recreated under the same name Two paths still touched the successor's node ConfigMap by name: - resetShadowNodeConfigMap runs before the shadow's manifests are applied, outside the shadow's lock. When the old pod's cleanup deleted the ConfigMap between its Create (AlreadyExists) and its Update, the Update failed with NotFound and the new pod's creation failed, permanently for a restartPolicy Never pod. It now creates the ConfigMap again. - A deleted pod stays tracked while its remote deletion runs, after its published node is forgotten, so the status loop could write the old job's node and socket directory into the successor's ConfigMap and point its tunnel at the old job. publishShadowNode now reads the ConfigMap, skips one stamped with another pod's UID, and patches with the resourceVersion it read. Tests also cover that applying a shadow stamps every object with the pod's UID and relabels an object left by the previous pod. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matteo Bunino <48362942+matbun@users.noreply.github.com> --- pkg/virtualkubelet/shadow.go | 63 +++++++++++++++++-------- pkg/virtualkubelet/shadow_test.go | 77 +++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 20 deletions(-) diff --git a/pkg/virtualkubelet/shadow.go b/pkg/virtualkubelet/shadow.go index 8c4c4241..ae5fb707 100644 --- a/pkg/virtualkubelet/shadow.go +++ b/pkg/virtualkubelet/shadow.go @@ -299,7 +299,9 @@ func shadowNodeConfigMapName(shadowName string) string { // resetShadowNodeConfigMap creates, or blanks, the per-shadow node ConfigMap. // The shadow is rendered before the remote batch system has allocated anything, // so the ConfigMap has to exist - and be mountable - while still empty. Blanking -// an existing one stops a previous run's node from being tunnelled to. +// an existing one stops a previous run's node from being tunnelled to. The cleanup +// of a deleted pod with the same name can remove an existing one in between, in +// which case it is created again. func (p *Provider) resetShadowNodeConfigMap(ctx context.Context, identity shadowResourceIdentity) error { cm := &v1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ @@ -310,17 +312,23 @@ func (p *Provider) resetShadowNodeConfigMap(ctx context.Context, identity shadow } stampShadowOwner(cm, identity.OwnerUID) - _, err := p.clientSet.CoreV1().ConfigMaps(identity.Namespace).Create(ctx, cm, metav1.CreateOptions{}) - if err == nil { - return nil - } - if !apierrors.IsAlreadyExists(err) { - return fmt.Errorf("failed to create shadow node configmap %s/%s: %w", identity.Namespace, cm.Name, err) - } - if _, err := p.clientSet.CoreV1().ConfigMaps(identity.Namespace).Update(ctx, cm, metav1.UpdateOptions{}); err != nil { - return fmt.Errorf("failed to reset shadow node configmap %s/%s: %w", identity.Namespace, cm.Name, err) + configMaps := p.clientSet.CoreV1().ConfigMaps(identity.Namespace) + for attempt := 0; ; attempt++ { + _, err := configMaps.Create(ctx, cm, metav1.CreateOptions{}) + if err == nil { + return nil + } + if !apierrors.IsAlreadyExists(err) { + return fmt.Errorf("failed to create shadow node configmap %s/%s: %w", identity.Namespace, cm.Name, err) + } + _, err = configMaps.Update(ctx, cm, metav1.UpdateOptions{}) + if err == nil { + return nil + } + if !apierrors.IsNotFound(err) || attempt == 2 { + return fmt.Errorf("failed to reset shadow node configmap %s/%s: %w", identity.Namespace, cm.Name, err) + } } - return nil } // publishShadowNode records the remote compute node a pod was allocated on, and @@ -332,7 +340,9 @@ func (p *Provider) resetShadowNodeConfigMap(ctx context.Context, identity shadow // patch, so the shadow never sees a new node with the previous job's directory. // // Repeated calls with unchanged values are dropped, so the status loop does not -// write on every poll. +// write on every poll. A ConfigMap stamped with another pod's UID belongs to a pod +// recreated under the same name and is left alone; the patch carries the +// resourceVersion read, so it fails rather than overwrite a change made meanwhile. func (p *Provider) publishShadowNode(ctx context.Context, pod *v1.Pod, nodeName, socketDir string) { if nodeName == "" || !p.hasShadow(pod) { return @@ -360,19 +370,32 @@ func (p *Provider) publishShadowNode(ctx context.Context, pod *v1.Pod, nodeName, return } - patch, err := json.Marshal(map[string]map[string]string{"data": { - shadowNodeNameKey: nodeName, - shadowSocketDirKey: socketDir, - }}) + name := shadowNodeConfigMapName(identity.Name) + configMaps := p.clientSet.CoreV1().ConfigMaps(identity.Namespace) + current, err := configMaps.Get(ctx, name, metav1.GetOptions{}) + if err != nil { + log.G(ctx).Warningf("Failed to read shadow configmap %s/%s: %v", identity.Namespace, name, err) + return + } + if owner := current.Labels[shadowOwnerUIDLabel]; owner != "" && identity.OwnerUID != "" && owner != identity.OwnerUID { + log.G(ctx).Infof("Not publishing compute node %q for %s/%s: shadow configmap %s belongs to pod UID %s", + nodeName, pod.Namespace, pod.Name, name, owner) + return + } + + patch, err := json.Marshal(map[string]map[string]string{ + "metadata": {"resourceVersion": current.ResourceVersion}, + "data": { + shadowNodeNameKey: nodeName, + shadowSocketDirKey: socketDir, + }, + }) if err != nil { log.G(ctx).Warningf("Failed to marshal shadow node patch for %s/%s: %v", pod.Namespace, pod.Name, err) return } - name := shadowNodeConfigMapName(identity.Name) - _, err = p.clientSet.CoreV1().ConfigMaps(identity.Namespace).Patch( - ctx, name, k8stypes.StrategicMergePatchType, patch, metav1.PatchOptions{}, - ) + _, err = configMaps.Patch(ctx, name, k8stypes.StrategicMergePatchType, patch, metav1.PatchOptions{}) if err != nil { log.G(ctx).Warningf("Failed to publish compute node %q to shadow configmap %s/%s: %v", nodeName, identity.Namespace, name, err) return diff --git a/pkg/virtualkubelet/shadow_test.go b/pkg/virtualkubelet/shadow_test.go index 2383c70c..3bbf6975 100644 --- a/pkg/virtualkubelet/shadow_test.go +++ b/pkg/virtualkubelet/shadow_test.go @@ -84,6 +84,30 @@ func TestResetShadowNodeConfigMap(t *testing.T) { "a stale node would make the shadow tunnel to a host whose job is gone") assert.Equal(t, "", cm.Data[shadowSocketDirKey]) }) + + // The cleanup of a deleted pod with the same name removes its ConfigMap between + // this Create (AlreadyExists) and the Update. + t.Run("creates it again when it is deleted before the update", func(t *testing.T) { + const previousOwner, owner = "uid-previous", "uid-current" + client := fake.NewSimpleClientset(&v1.ConfigMap{ObjectMeta: metav1.ObjectMeta{ + Name: shadowNodeConfigMapName(identity.Name), Namespace: identity.Namespace, + Labels: map[string]string{shadowOwnerUIDLabel: previousOwner}, + }}) + gvr := v1.SchemeGroupVersion.WithResource("configmaps") + client.PrependReactor("update", "configmaps", func(k8stesting.Action) (bool, runtime.Object, error) { + require.NoError(t, client.Tracker().Delete(gvr, identity.Namespace, shadowNodeConfigMapName(identity.Name))) + return true, nil, apierrors.NewNotFound(gvr.GroupResource(), shadowNodeConfigMapName(identity.Name)) + }) + p := tunnelProvider(client) + current := identity + current.OwnerUID = owner + + require.NoError(t, p.resetShadowNodeConfigMap(t.Context(), current)) + + cm, err := client.CoreV1().ConfigMaps(identity.Namespace).Get(t.Context(), shadowNodeConfigMapName(identity.Name), metav1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, owner, cm.Labels[shadowOwnerUIDLabel]) + }) } func TestPublishShadowNode(t *testing.T) { @@ -189,6 +213,35 @@ func TestPublishShadowNode(t *testing.T) { assert.Empty(t, configMapPatches(client)) }) + // A deleted pod stays tracked while its remote deletion runs, after its cache entry + // is forgotten: it must not write its node into its same-name successor's ConfigMap. + t.Run("a configmap of a pod recreated under the same name is left alone", func(t *testing.T) { + client := fake.NewSimpleClientset(&v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: cmName, Namespace: identity.Namespace, + Labels: map[string]string{shadowOwnerUIDLabel: "uid-successor"}}, + Data: map[string]string{shadowNodeNameKey: ""}, + }) + p := tunnelProvider(client) + + p.publishShadowNode(t.Context(), pod, testComputeNode, testSocketDir) + + assert.Empty(t, configMapPatches(client)) + }) + + t.Run("the patch carries the resource version that was read", func(t *testing.T) { + client := fake.NewSimpleClientset(&v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: cmName, Namespace: identity.Namespace, ResourceVersion: "7"}, + Data: map[string]string{shadowNodeNameKey: ""}, + }) + p := tunnelProvider(client) + + p.publishShadowNode(t.Context(), pod, testComputeNode, "") + + patches := configMapPatches(client) + require.Len(t, patches, 1) + assert.Contains(t, string(patches[0].(k8stesting.PatchAction).GetPatch()), `"resourceVersion":"7"`) + }) + t.Run("forgetting a pod lets the same node be published again", func(t *testing.T) { client := newClient() p := tunnelProvider(client) @@ -592,6 +645,30 @@ func TestShadowCleanupAndApplyAreSerialised(t *testing.T) { }) } +// Applying a shadow stamps every object with the pod's UID, and an object left by a +// deleted pod with the same name is updated in place with the new pod's UID. +func TestApplyShadowStampsOwner(t *testing.T) { + const name, previousOwner, owner = testPodDefaultName, "uid-before", "uid-after" + ns := testNamespaceDefault + client := fake.NewSimpleClientset(&v1.ConfigMap{ObjectMeta: metav1.ObjectMeta{ + Name: name, Namespace: ns, Labels: map[string]string{shadowOwnerUIDLabel: previousOwner}, + }}) + p := tunnelProvider(client) + manifest := "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: " + name + "\n namespace: " + ns + "\n" + + "---\napiVersion: v1\nkind: Service\nmetadata:\n name: " + name + "\n namespace: " + ns + "\n" + + "spec:\n ports:\n - port: 80\n" + + _, _, err := p.applyShadowObjects(t.Context(), manifest, &ShadowTemplateData{Name: name, Namespace: ns, OwnerUID: owner}) + require.NoError(t, err) + + cm, err := client.CoreV1().ConfigMaps(ns).Get(t.Context(), name, metav1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, owner, cm.Labels[shadowOwnerUIDLabel], "the previous pod's object is relabelled") + svc, err := client.CoreV1().Services(ns).Get(t.Context(), name, metav1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, owner, svc.Labels[shadowOwnerUIDLabel]) +} + func TestShadowIdentityAndNodeConfigMapCarryOwner(t *testing.T) { pod := podWithPort("nb", testNamespaceDefault, "uid-7") identity, err := computeShadowResourceIdentity(pod) From bb59d48a26657e50bb2484b2cd1f61b0baad3a51 Mon Sep 17 00:00:00 2001 From: Matteo Bunino <48362942+matbun@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:22:31 +0200 Subject: [PATCH 6/9] fix: tell a recreated pod's shadow pods and leftovers from the previous pod's When a pod recreated under the same name takes the shadow Deployment over in place, the Deployment keeps its creation time, so the previous pod's shadow pod is not older than it. waitForDeploymentPod could return it, giving the new pod the old pod's IP, or a pod about to be replaced, and currentShadowPod could take the previous pod's completed kept init containers for the new pod's, submitting it before its own had run. Both now skip shadow pods labelled interlink.eu/shadow-of for another pod. A resource stamped with another pod's UID was kept for good, so the leftovers of a pod whose cleanup never ran (deleted while the virtual kubelet was down) survived its successor's cleanup too. It is now kept only while that UID is the pod that has the name; when the pod cannot be read it is kept. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matteo Bunino <48362942+matbun@users.noreply.github.com> --- pkg/virtualkubelet/kept_containers.go | 2 +- pkg/virtualkubelet/kept_containers_test.go | 22 +++++++++++++ pkg/virtualkubelet/shadow.go | 36 +++++++++++++++++++++- pkg/virtualkubelet/shadow_pod_test.go | 26 ++++++++++++++-- pkg/virtualkubelet/shadow_test.go | 36 ++++++++++++++++++++++ pkg/virtualkubelet/virtualkubelet.go | 18 ++++++++--- 6 files changed, 132 insertions(+), 8 deletions(-) diff --git a/pkg/virtualkubelet/kept_containers.go b/pkg/virtualkubelet/kept_containers.go index dd0b26d3..b727fdd5 100644 --- a/pkg/virtualkubelet/kept_containers.go +++ b/pkg/virtualkubelet/kept_containers.go @@ -159,7 +159,7 @@ func (p *Provider) currentShadowPod(ctx context.Context, identity shadowResource var newest *v1.Pod for i := range pods.Items { pod := &pods.Items[i] - if pod.DeletionTimestamp != nil { + if pod.DeletionTimestamp != nil || !isShadowPodOf(pod, identity.OwnerUID) { continue } if newest == nil || newest.CreationTimestamp.Before(&pod.CreationTimestamp) { diff --git a/pkg/virtualkubelet/kept_containers_test.go b/pkg/virtualkubelet/kept_containers_test.go index c600a678..f43da6d6 100644 --- a/pkg/virtualkubelet/kept_containers_test.go +++ b/pkg/virtualkubelet/kept_containers_test.go @@ -270,6 +270,28 @@ func TestWaitForKeptInitContainers(t *testing.T) { assert.Equal(t, "/delete", path.Load()) }) + // The recreated pod took the shadow over in place: the previous pod's shadow pod, + // whose kept init container already completed, must not count for this pod. + t.Run("a shadow pod of the previous pod with the same name does not count", func(t *testing.T) { + pod := podKeepingInit(v1.RestartPolicyNever) + previous := shadowOf(t, pod, exited(0)) + previous.Labels[shadowOfLabel] = "uid-previous-generation" + client := fake.NewSimpleClientset(previous) + p := keptProvider(client, pod) + + ctx := p.trackKeptInitWait(t.Context(), pod) + done := make(chan error, 1) + go func() { done <- p.waitForKeptInitContainers(ctx, pod) }() + + select { + case err := <-done: + t.Fatalf("returned on the previous pod's shadow: %v", err) + case <-time.After(100 * time.Millisecond): + } + p.stopKeptInitWait(string(pod.UID)) + <-done + }) + t.Run("no kept init container: nothing to wait for", func(t *testing.T) { pod := podKeepingInit(v1.RestartPolicyNever) delete(pod.Annotations, annDisableOffloadInitContainers) diff --git a/pkg/virtualkubelet/shadow.go b/pkg/virtualkubelet/shadow.go index c3d965c5..da1946e4 100644 --- a/pkg/virtualkubelet/shadow.go +++ b/pkg/virtualkubelet/shadow.go @@ -163,6 +163,10 @@ type shadowResourceIdentity struct { // after the pod, so a pod recreated under the same name reuses those names; the // UID, stamped as a label, tells which pod a resource belongs to. OwnerUID string + // PodName and PodNamespace name that pod, to tell whether another UID stamped on a + // resource is the pod that now has the name (see ownerIsCurrentPod). + PodName string + PodNamespace string } func isShadowSameNamespace(pod *v1.Pod) bool { @@ -187,7 +191,10 @@ func computeShadowResourceIdentity(pod *v1.Pod) (shadowResourceIdentity, error) name, namespace = computeShadowResourceNames(pod.Name, pod.Namespace) } - identity := shadowResourceIdentity{Name: name, Namespace: namespace, OwnerUID: string(pod.UID)} + identity := shadowResourceIdentity{ + Name: name, Namespace: namespace, + OwnerUID: string(pod.UID), PodName: pod.Name, PodNamespace: pod.Namespace, + } if len(fmt.Sprintf("%s-%s", identity.Name, identity.Namespace)) > 63 { return shadowResourceIdentity{}, fmt.Errorf("shadow ingress hostname label %q exceeds 63 characters; shorten pod/namespace or disable interlink.eu/shadow-same-ns", fmt.Sprintf("%s-%s", identity.Name, identity.Namespace)) } @@ -439,6 +446,33 @@ func (p *Provider) releaseShadowOfEndedPod(ctx context.Context, pod *v1.Pod) { // indexShadowPods groups the shadow pods by the UID of the pod they stand for, read // from shadowOfLabel. The pods of a shadow rendered before that label existed are // left out, so their virtual pod keeps the IP it was created with. +// isShadowPodOf reports whether a shadow pod may serve the pod with UID podUID: it is +// labelled for that pod, or it predates shadowOfLabel, or the UID is unknown. +func isShadowPodOf(shadowPod *v1.Pod, podUID string) bool { + of := shadowPod.Labels[shadowOfLabel] + return of == "" || podUID == "" || of == podUID +} + +// ownerIsCurrentPod reports whether owner, the UID stamped on a shadow resource, is +// the UID of the pod that now has the name the shadow stands for. Any other UID is a +// pod that no longer exists, whose cleanup never ran (it was deleted while the virtual +// kubelet was down, say): its leftovers are deleted. When the pod cannot be read, the +// resource is kept. +func (p *Provider) ownerIsCurrentPod(ctx context.Context, identity shadowResourceIdentity, owner string) bool { + if identity.PodName == "" { + return true + } + pod, err := p.clientSet.CoreV1().Pods(identity.PodNamespace).Get(ctx, identity.PodName, metav1.GetOptions{}) + if apierrors.IsNotFound(err) { + return false + } + if err != nil { + log.G(ctx).Warningf("Failed to read pod %s/%s: %v", identity.PodNamespace, identity.PodName, err) + return true + } + return string(pod.UID) == owner +} + func indexShadowPods(pods []v1.Pod) map[string][]*v1.Pod { index := make(map[string][]*v1.Pod) for i := range pods { diff --git a/pkg/virtualkubelet/shadow_pod_test.go b/pkg/virtualkubelet/shadow_pod_test.go index 69c71747..5bf89396 100644 --- a/pkg/virtualkubelet/shadow_pod_test.go +++ b/pkg/virtualkubelet/shadow_pod_test.go @@ -41,7 +41,7 @@ func TestWaitForDeploymentPodIgnoresPodsOfAPreviousDeployment(t *testing.T) { p := &Provider{clientSet: fake.NewSimpleClientset(deployment, previous, current)} - got, err := p.waitForDeploymentPod(context.Background(), name, testNamespace) + got, err := p.waitForDeploymentPod(context.Background(), name, testNamespace, "") require.NoError(t, err) assert.Equal(t, current.Name, got.Name) } @@ -61,7 +61,29 @@ func TestWaitForDeploymentPodIgnoresTerminatingPods(t *testing.T) { p := &Provider{clientSet: fake.NewSimpleClientset(deployment, terminating, current)} - got, err := p.waitForDeploymentPod(context.Background(), name, testNamespace) + got, err := p.waitForDeploymentPod(context.Background(), name, testNamespace, "") + require.NoError(t, err) + assert.Equal(t, current.Name, got.Name) +} + +// When the recreated pod takes the Deployment over in place, the Deployment keeps its +// creation time, so the previous pod's shadow pod is not older than it. The label +// naming the pod each shadow pod stands for tells them apart. +func TestWaitForDeploymentPodIgnoresPodsOfAnotherPod(t *testing.T) { + const name, previousUID, currentUID = "shadow-nb-0-ns", "uid-p1", "uid-p2" + now := time.Now() + + deployment := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{ + Name: name, Namespace: testNamespace, CreationTimestamp: metav1.NewTime(now.Add(-time.Hour)), + }} + previous := shadowTestPod(name+"-6d5c666bc8-zzzzz", name, "10.0.0.146", now) + previous.Labels[shadowOfLabel] = previousUID + current := shadowTestPod(name+"-7f8d9a0b1c-aaaaa", name, "10.0.0.141", now.Add(-time.Minute)) + current.Labels[shadowOfLabel] = currentUID + + p := &Provider{clientSet: fake.NewSimpleClientset(deployment, previous, current)} + + got, err := p.waitForDeploymentPod(context.Background(), name, testNamespace, currentUID) require.NoError(t, err) assert.Equal(t, current.Name, got.Name) } diff --git a/pkg/virtualkubelet/shadow_test.go b/pkg/virtualkubelet/shadow_test.go index 3bbf6975..8ed8a75d 100644 --- a/pkg/virtualkubelet/shadow_test.go +++ b/pkg/virtualkubelet/shadow_test.go @@ -599,6 +599,42 @@ func TestCleanupKeepsShadowOfRecreatedPod(t *testing.T) { }) } +// A resource stamped with another pod's UID is kept only while that pod is the one +// that has the name; the leftovers of a pod whose cleanup never ran are deleted. +func TestCleanupKeepsOnlyTheCurrentPodsResources(t *testing.T) { + const name, podName, staleUID, currentUID = testPodDefaultName, "nb-0", "uid-stale", "uid-live" + ns := testNamespaceDefault + identity := shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: "uid-cleaning", PodName: podName, PodNamespace: ns} + stampedService := func(owner string) *v1.Service { + return &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns, Labels: map[string]string{shadowOwnerUIDLabel: owner}}} + } + livePod := &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: podName, Namespace: ns, UID: k8stypes.UID(currentUID)}} + + t.Run("kept for the pod that now has the name", func(t *testing.T) { + client := fake.NewSimpleClientset(stampedService(currentUID), livePod) + tunnelProvider(client).cleanupShadowResources(t.Context(), identity) + _, err := client.CoreV1().Services(ns).Get(t.Context(), name, metav1.GetOptions{}) + assert.NoError(t, err) + }) + + t.Run("deleted when its pod no longer exists", func(t *testing.T) { + client := fake.NewSimpleClientset(stampedService(staleUID), livePod) + tunnelProvider(client).cleanupShadowResources(t.Context(), identity) + _, err := client.CoreV1().Services(ns).Get(t.Context(), name, metav1.GetOptions{}) + assert.True(t, apierrors.IsNotFound(err), "a leftover of a pod that is gone stays forever otherwise") + }) + + t.Run("kept when the pod cannot be read", func(t *testing.T) { + client := fake.NewSimpleClientset(stampedService(staleUID)) + client.PrependReactor("get", "pods", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewInternalError(errors.New("etcd timeout")) + }) + tunnelProvider(client).cleanupShadowResources(t.Context(), identity) + _, err := client.CoreV1().Services(ns).Get(t.Context(), name, metav1.GetOptions{}) + assert.NoError(t, err) + }) +} + // The cleanup of a deleted pod and the shadow of its same-name successor must not // interleave: the new Ingress could otherwise appear between the old Ingress and the // TLS Secret being deleted, and lose the Secret. diff --git a/pkg/virtualkubelet/virtualkubelet.go b/pkg/virtualkubelet/virtualkubelet.go index d9cf30ac..24f09a43 100644 --- a/pkg/virtualkubelet/virtualkubelet.go +++ b/pkg/virtualkubelet/virtualkubelet.go @@ -1428,7 +1428,11 @@ func (p *Provider) applyShadowManifests(ctx context.Context, manifestYAML string return nil, err } if deploymentName != "" { - return p.waitForDeploymentPod(ctx, deploymentName, namespace) + podUID := "" + if td != nil { + podUID = td.PodUID + } + return p.waitForDeploymentPod(ctx, deploymentName, namespace, podUID) } return nil, fmt.Errorf("no deployment found in manifests") @@ -1514,8 +1518,10 @@ func (p *Provider) applyShadowObjects(ctx context.Context, manifestYAML string, // A pod recreated under the same name (a StatefulSet replica) gets a Deployment with the // same name and pod selector as the one just deleted, and the garbage collector // removes that one's pods asynchronously. Pods that are terminating, or older -// than the current Deployment, belong to the previous one and are skipped. -func (p *Provider) waitForDeploymentPod(ctx context.Context, deploymentName, namespace string) (*v1.Pod, error) { +// than the current Deployment, belong to the previous one and are skipped. So are +// pods labelled for another pod than podUID: when the recreated pod takes the +// Deployment over in place, the previous pod's shadow pod is newer than it. +func (p *Provider) waitForDeploymentPod(ctx context.Context, deploymentName, namespace, podUID string) (*v1.Pod, error) { timeout := 30 * time.Second start := time.Now() @@ -1541,6 +1547,9 @@ func (p *Provider) waitForDeploymentPod(ctx context.Context, deploymentName, nam if pod.DeletionTimestamp != nil || pod.CreationTimestamp.Before(&deployment.CreationTimestamp) { continue } + if !isShadowPodOf(pod, podUID) { + continue + } return pod, nil } @@ -1606,7 +1615,8 @@ func (p *Provider) deleteOwnedShadowResource(ctx context.Context, identity shado if err != nil { return err } - if owner := obj.GetLabels()[shadowOwnerUIDLabel]; owner != "" && identity.OwnerUID != "" && owner != identity.OwnerUID { + if owner := obj.GetLabels()[shadowOwnerUIDLabel]; owner != "" && identity.OwnerUID != "" && owner != identity.OwnerUID && + p.ownerIsCurrentPod(ctx, identity, owner) { log.G(ctx).Infof("Keeping shadow %s %s/%s: it belongs to pod UID %s, not %s", kind, identity.Namespace, name, owner, identity.OwnerUID) kept = true return nil From 7fd70b90fedfdc3ad0df5a5c9e795d5e6916834b Mon Sep 17 00:00:00 2001 From: Matteo Bunino <48362942+matbun@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:39:50 +0200 Subject: [PATCH 7/9] fix: look a shadow resource's owner up by its own name ownerIsCurrentPod looked the owner up under the name of the pod being cleaned up. Two live pods can share a shadow name, since sanitising maps web.0 and web-0 (or namespaces a--b and a-b) to the same one, so deleting one of them deleted the shadow the other had taken over. Shadow resources now also carry the owner pod's namespace/name in the annotation interlink.eu/shadow-owner-pod, and the owner is looked up by that name. The cleanup tests now give the pods names and create the successor pod, so they take the path production takes instead of the no-name shortcut, and a test covers a pod that no longer exists. isShadowPodOf no longer sits between indexShadowPods and its comment. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matteo Bunino <48362942+matbun@users.noreply.github.com> --- pkg/virtualkubelet/shadow.go | 45 +++++++---- pkg/virtualkubelet/shadow_test.go | 111 +++++++++++++++++---------- pkg/virtualkubelet/virtualkubelet.go | 24 ++++-- 3 files changed, 119 insertions(+), 61 deletions(-) diff --git a/pkg/virtualkubelet/shadow.go b/pkg/virtualkubelet/shadow.go index da1946e4..106ebd0b 100644 --- a/pkg/virtualkubelet/shadow.go +++ b/pkg/virtualkubelet/shadow.go @@ -163,12 +163,19 @@ type shadowResourceIdentity struct { // after the pod, so a pod recreated under the same name reuses those names; the // UID, stamped as a label, tells which pod a resource belongs to. OwnerUID string - // PodName and PodNamespace name that pod, to tell whether another UID stamped on a - // resource is the pod that now has the name (see ownerIsCurrentPod). + // PodName and PodNamespace name that pod. PodName string PodNamespace string } +// ownerPod is the namespace/name stamped as shadowOwnerPodAnnotation. +func (identity shadowResourceIdentity) ownerPod() string { + if identity.PodName == "" { + return "" + } + return identity.PodNamespace + "/" + identity.PodName +} + func isShadowSameNamespace(pod *v1.Pod) bool { if pod == nil || pod.Annotations == nil { return false @@ -323,7 +330,7 @@ func (p *Provider) resetShadowNodeConfigMap(ctx context.Context, identity shadow }, Data: map[string]string{shadowNodeNameKey: "", shadowSocketDirKey: ""}, } - stampShadowOwner(cm, identity.OwnerUID) + stampShadowOwner(cm, identity.OwnerUID, identity.ownerPod()) configMaps := p.clientSet.CoreV1().ConfigMaps(identity.Namespace) for attempt := 0; ; attempt++ { @@ -443,9 +450,6 @@ func (p *Provider) releaseShadowOfEndedPod(ctx context.Context, pod *v1.Pod) { p.forgetShadowNodeName(pod) } -// indexShadowPods groups the shadow pods by the UID of the pod they stand for, read -// from shadowOfLabel. The pods of a shadow rendered before that label existed are -// left out, so their virtual pod keeps the IP it was created with. // isShadowPodOf reports whether a shadow pod may serve the pod with UID podUID: it is // labelled for that pod, or it predates shadowOfLabel, or the UID is unknown. func isShadowPodOf(shadowPod *v1.Pod, podUID string) bool { @@ -453,26 +457,35 @@ func isShadowPodOf(shadowPod *v1.Pod, podUID string) bool { return of == "" || podUID == "" || of == podUID } -// ownerIsCurrentPod reports whether owner, the UID stamped on a shadow resource, is -// the UID of the pod that now has the name the shadow stands for. Any other UID is a -// pod that no longer exists, whose cleanup never ran (it was deleted while the virtual -// kubelet was down, say): its leftovers are deleted. When the pod cannot be read, the -// resource is kept. -func (p *Provider) ownerIsCurrentPod(ctx context.Context, identity shadowResourceIdentity, owner string) bool { - if identity.PodName == "" { +// ownerIsCurrentPod reports whether the pod a shadow resource is stamped for still +// exists: the pod named by its shadowOwnerPodAnnotation (the pod of identity when it +// has none) has the UID of its shadowOwnerUIDLabel. The name comes from the resource +// because two pods can share a shadow name: sanitising maps web.0 and web-0 to one. +// Otherwise the owner is gone and its cleanup never ran (it was deleted while the +// virtual kubelet was down, say), so its leftovers are deleted. When the pod cannot +// be read, the resource is kept. +func (p *Provider) ownerIsCurrentPod(ctx context.Context, identity shadowResourceIdentity, obj metav1.Object) bool { + namespace, name, found := strings.Cut(obj.GetAnnotations()[shadowOwnerPodAnnotation], "/") + if !found { + namespace, name = identity.PodNamespace, identity.PodName + } + if name == "" { return true } - pod, err := p.clientSet.CoreV1().Pods(identity.PodNamespace).Get(ctx, identity.PodName, metav1.GetOptions{}) + pod, err := p.clientSet.CoreV1().Pods(namespace).Get(ctx, name, metav1.GetOptions{}) if apierrors.IsNotFound(err) { return false } if err != nil { - log.G(ctx).Warningf("Failed to read pod %s/%s: %v", identity.PodNamespace, identity.PodName, err) + log.G(ctx).Warningf("Failed to read pod %s/%s: %v", namespace, name, err) return true } - return string(pod.UID) == owner + return string(pod.UID) == obj.GetLabels()[shadowOwnerUIDLabel] } +// indexShadowPods groups the shadow pods by the UID of the pod they stand for, read +// from shadowOfLabel. The pods of a shadow rendered before that label existed are +// left out, so their virtual pod keeps the IP it was created with. func indexShadowPods(pods []v1.Pod) map[string][]*v1.Pod { index := make(map[string][]*v1.Pod) for i := range pods { diff --git a/pkg/virtualkubelet/shadow_test.go b/pkg/virtualkubelet/shadow_test.go index 8ed8a75d..86abf810 100644 --- a/pkg/virtualkubelet/shadow_test.go +++ b/pkg/virtualkubelet/shadow_test.go @@ -450,40 +450,51 @@ func TestReleaseShadowIgnoresPodsWithoutAShadow(t *testing.T) { // name: the new pod's shadow has the same names, and the old pod's cleanup can run after // it was created. Only resources stamped with the old pod's UID may go. func TestCleanupKeepsShadowOfRecreatedPod(t *testing.T) { - const name = testPodDefaultName + const name, podName = testPodDefaultName, "nb-0" const oldUID, newUID = "uid-old", "uid-new" ns := testNamespaceDefault + // Every pod is named podName: the successor (newUID) exists, the old pod is gone. + cleanup := func(uid string) shadowResourceIdentity { + return shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: uid, PodName: podName, PodNamespace: ns} + } + owned := func(objectName, uid string) metav1.ObjectMeta { + meta := metav1.ObjectMeta{Name: objectName, Namespace: ns} + stampShadowOwner(&meta, uid, ns+"/"+podName) + return meta + } + withSuccessor := func(objects ...runtime.Object) *fake.Clientset { + successor := &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: podName, Namespace: ns, UID: k8stypes.UID(newUID)}} + return fake.NewSimpleClientset(append(objects, successor)...) + } stamped := func(uid string) *appsv1.Deployment { - return &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{ - Name: name, Namespace: ns, Labels: map[string]string{shadowOwnerUIDLabel: uid}, - }} + return &appsv1.Deployment{ObjectMeta: owned(name, uid)} } t.Run("the old pod's cleanup keeps the new pod's shadow", func(t *testing.T) { - client := fake.NewSimpleClientset(stamped(newUID)) + client := withSuccessor(stamped(newUID)) p := tunnelProvider(client) - p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: oldUID}) + p.cleanupShadowResources(t.Context(), cleanup(oldUID)) _, err := client.AppsV1().Deployments(ns).Get(t.Context(), name, metav1.GetOptions{}) assert.NoError(t, err) }) t.Run("the owner's cleanup removes it", func(t *testing.T) { - client := fake.NewSimpleClientset(stamped(newUID)) + client := withSuccessor(stamped(newUID)) p := tunnelProvider(client) - p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: newUID}) + p.cleanupShadowResources(t.Context(), cleanup(newUID)) _, err := client.AppsV1().Deployments(ns).Get(t.Context(), name, metav1.GetOptions{}) assert.True(t, apierrors.IsNotFound(err)) }) t.Run("an unstamped shadow from an older virtual kubelet is removed", func(t *testing.T) { - client := fake.NewSimpleClientset(&appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns}}) + client := withSuccessor(&appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns}}) p := tunnelProvider(client) - p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: oldUID}) + p.cleanupShadowResources(t.Context(), cleanup(oldUID)) _, err := client.AppsV1().Deployments(ns).Get(t.Context(), name, metav1.GetOptions{}) assert.True(t, apierrors.IsNotFound(err)) @@ -492,9 +503,7 @@ func TestCleanupKeepsShadowOfRecreatedPod(t *testing.T) { // cert-manager creates the TLS Secret without the owner label: it follows its Ingress. tlsObjects := func(owner string) []runtime.Object { return []runtime.Object{ - &networkingv1.Ingress{ObjectMeta: metav1.ObjectMeta{ - Name: name, Namespace: ns, Labels: map[string]string{shadowOwnerUIDLabel: owner}, - }}, + &networkingv1.Ingress{ObjectMeta: owned(name, owner)}, &v1.Secret{ObjectMeta: metav1.ObjectMeta{Name: name + "-tls", Namespace: ns}}, } } @@ -503,11 +512,11 @@ func TestCleanupKeepsShadowOfRecreatedPod(t *testing.T) { kept bool }{{cleanup: oldUID, kept: true}, {cleanup: newUID, kept: false}} { t.Run("the TLS secret follows its ingress, cleanup by "+tc.cleanup, func(t *testing.T) { - client := fake.NewSimpleClientset(tlsObjects(newUID)...) + client := withSuccessor(tlsObjects(newUID)...) p := tunnelProvider(client) p.config.Network.IngressTLS = true - p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: tc.cleanup}) + p.cleanupShadowResources(t.Context(), cleanup(tc.cleanup)) _, err := client.CoreV1().Secrets(ns).Get(t.Context(), name+"-tls", metav1.GetOptions{}) if tc.kept { @@ -536,14 +545,14 @@ func TestCleanupKeepsShadowOfRecreatedPod(t *testing.T) { } t.Run("an ingress taken over between read and delete keeps it and its TLS secret", func(t *testing.T) { - client := fake.NewSimpleClientset(tlsObjects(oldUID)...) + client := withSuccessor(tlsObjects(oldUID)...) conflictOnce(client, "ingresses", networkingv1.SchemeGroupVersion.WithResource("ingresses"), func(o metav1.Object) { o.SetLabels(map[string]string{shadowOwnerUIDLabel: newUID}) }) p := tunnelProvider(client) p.config.Network.IngressTLS = true - p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: oldUID}) + p.cleanupShadowResources(t.Context(), cleanup(oldUID)) _, err := client.NetworkingV1().Ingresses(ns).Get(t.Context(), name, metav1.GetOptions{}) assert.NoError(t, err) @@ -552,27 +561,27 @@ func TestCleanupKeepsShadowOfRecreatedPod(t *testing.T) { }) t.Run("any other change between read and delete is retried", func(t *testing.T) { - client := fake.NewSimpleClientset(stamped(oldUID)) + client := withSuccessor(stamped(oldUID)) conflictOnce(client, "deployments", appsv1.SchemeGroupVersion.WithResource("deployments"), func(o metav1.Object) { o.SetAnnotations(map[string]string{"status": "updated"}) }) p := tunnelProvider(client) - p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: oldUID}) + p.cleanupShadowResources(t.Context(), cleanup(oldUID)) _, err := client.AppsV1().Deployments(ns).Get(t.Context(), name, metav1.GetOptions{}) assert.True(t, apierrors.IsNotFound(err)) }) t.Run("the TLS secret stays when its ingress cannot be read", func(t *testing.T) { - client := fake.NewSimpleClientset(tlsObjects(newUID)...) + client := withSuccessor(tlsObjects(newUID)...) client.PrependReactor("get", "ingresses", func(k8stesting.Action) (bool, runtime.Object, error) { return true, nil, apierrors.NewInternalError(errors.New("etcd timeout")) }) p := tunnelProvider(client) p.config.Network.IngressTLS = true - p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: oldUID}) + p.cleanupShadowResources(t.Context(), cleanup(oldUID)) _, err := client.CoreV1().Secrets(ns).Get(t.Context(), name+"-tls", metav1.GetOptions{}) assert.NoError(t, err) @@ -581,7 +590,7 @@ func TestCleanupKeepsShadowOfRecreatedPod(t *testing.T) { t.Run("the delete is conditional on the resource version that was read", func(t *testing.T) { deployment := stamped(oldUID) deployment.UID, deployment.ResourceVersion = "deployment-uid", "41" - client := fake.NewSimpleClientset(deployment) + client := withSuccessor(deployment) var preconditions *metav1.Preconditions client.PrependReactor("delete", "deployments", func(action k8stesting.Action) (bool, runtime.Object, error) { preconditions = action.(k8stesting.DeleteAction).GetDeleteOptions().Preconditions @@ -589,7 +598,7 @@ func TestCleanupKeepsShadowOfRecreatedPod(t *testing.T) { }) p := tunnelProvider(client) - p.cleanupShadowResources(t.Context(), shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: oldUID}) + p.cleanupShadowResources(t.Context(), cleanup(oldUID)) require.NotNil(t, preconditions) require.NotNil(t, preconditions.UID) @@ -605,33 +614,53 @@ func TestCleanupKeepsOnlyTheCurrentPodsResources(t *testing.T) { const name, podName, staleUID, currentUID = testPodDefaultName, "nb-0", "uid-stale", "uid-live" ns := testNamespaceDefault identity := shadowResourceIdentity{Name: name, Namespace: ns, OwnerUID: "uid-cleaning", PodName: podName, PodNamespace: ns} - stampedService := func(owner string) *v1.Service { - return &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns, Labels: map[string]string{shadowOwnerUIDLabel: owner}}} + stampedService := func(owner, ownerPod string) *v1.Service { + meta := metav1.ObjectMeta{Name: name, Namespace: ns} + stampShadowOwner(&meta, owner, ownerPod) + return &v1.Service{ObjectMeta: meta} } - livePod := &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: podName, Namespace: ns, UID: k8stypes.UID(currentUID)}} - - t.Run("kept for the pod that now has the name", func(t *testing.T) { - client := fake.NewSimpleClientset(stampedService(currentUID), livePod) + pod := func(podName, uid string) *v1.Pod { + return &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: podName, Namespace: ns, UID: k8stypes.UID(uid)}} + } + serviceLeft := func(t *testing.T, client *fake.Clientset) bool { + t.Helper() tunnelProvider(client).cleanupShadowResources(t.Context(), identity) _, err := client.CoreV1().Services(ns).Get(t.Context(), name, metav1.GetOptions{}) - assert.NoError(t, err) + if err != nil && !apierrors.IsNotFound(err) { + t.Fatal(err) + } + return err == nil + } + + t.Run("kept for the pod that now has the name", func(t *testing.T) { + client := fake.NewSimpleClientset(stampedService(currentUID, ns+"/"+podName), pod(podName, currentUID)) + assert.True(t, serviceLeft(t, client)) }) - t.Run("deleted when its pod no longer exists", func(t *testing.T) { - client := fake.NewSimpleClientset(stampedService(staleUID), livePod) - tunnelProvider(client).cleanupShadowResources(t.Context(), identity) - _, err := client.CoreV1().Services(ns).Get(t.Context(), name, metav1.GetOptions{}) - assert.True(t, apierrors.IsNotFound(err), "a leftover of a pod that is gone stays forever otherwise") + t.Run("deleted when another pod now has the name", func(t *testing.T) { + client := fake.NewSimpleClientset(stampedService(staleUID, ns+"/"+podName), pod(podName, currentUID)) + assert.False(t, serviceLeft(t, client), "a leftover of a pod that is gone stays forever otherwise") + }) + + t.Run("deleted when no pod has the name", func(t *testing.T) { + client := fake.NewSimpleClientset(stampedService(staleUID, ns+"/"+podName)) + assert.False(t, serviceLeft(t, client)) }) t.Run("kept when the pod cannot be read", func(t *testing.T) { - client := fake.NewSimpleClientset(stampedService(staleUID)) + client := fake.NewSimpleClientset(stampedService(staleUID, ns+"/"+podName)) client.PrependReactor("get", "pods", func(k8stesting.Action) (bool, runtime.Object, error) { return true, nil, apierrors.NewInternalError(errors.New("etcd timeout")) }) - tunnelProvider(client).cleanupShadowResources(t.Context(), identity) - _, err := client.CoreV1().Services(ns).Get(t.Context(), name, metav1.GetOptions{}) - assert.NoError(t, err) + assert.True(t, serviceLeft(t, client)) + }) + + // web.0 and web-0 get the same shadow name: deleting one must not take the + // shadow the other one, still running, took over. + t.Run("kept for a live pod of another name that shares the shadow name", func(t *testing.T) { + const other, otherUID = "nb.0", "uid-other" + client := fake.NewSimpleClientset(stampedService(otherUID, ns+"/"+other), pod(podName, currentUID), pod(other, otherUID)) + assert.True(t, serviceLeft(t, client)) }) } @@ -694,15 +723,17 @@ func TestApplyShadowStampsOwner(t *testing.T) { "---\napiVersion: v1\nkind: Service\nmetadata:\n name: " + name + "\n namespace: " + ns + "\n" + "spec:\n ports:\n - port: 80\n" - _, _, err := p.applyShadowObjects(t.Context(), manifest, &ShadowTemplateData{Name: name, Namespace: ns, OwnerUID: owner}) + _, _, err := p.applyShadowObjects(t.Context(), manifest, &ShadowTemplateData{Name: name, Namespace: ns, OwnerUID: owner, OwnerPod: ns + "/nb-0"}) require.NoError(t, err) cm, err := client.CoreV1().ConfigMaps(ns).Get(t.Context(), name, metav1.GetOptions{}) require.NoError(t, err) assert.Equal(t, owner, cm.Labels[shadowOwnerUIDLabel], "the previous pod's object is relabelled") + assert.Equal(t, ns+"/nb-0", cm.Annotations[shadowOwnerPodAnnotation]) svc, err := client.CoreV1().Services(ns).Get(t.Context(), name, metav1.GetOptions{}) require.NoError(t, err) assert.Equal(t, owner, svc.Labels[shadowOwnerUIDLabel]) + assert.Equal(t, ns+"/nb-0", svc.Annotations[shadowOwnerPodAnnotation]) } func TestShadowIdentityAndNodeConfigMapCarryOwner(t *testing.T) { diff --git a/pkg/virtualkubelet/virtualkubelet.go b/pkg/virtualkubelet/virtualkubelet.go index 24f09a43..d0327c47 100644 --- a/pkg/virtualkubelet/virtualkubelet.go +++ b/pkg/virtualkubelet/virtualkubelet.go @@ -105,6 +105,7 @@ const ( annMeshNetworkDisabled = "interlink.eu/mesh-network" // set to "disabled" to opt out of mesh networking annShadowSocketPorts = "interlink.eu/shadow-socket-ports" // comma-separated exposed TCP ports served on unix sockets shadowOwnerUIDLabel = "interlink.eu/shadow-owner-uid" + shadowOwnerPodAnnotation = "interlink.eu/shadow-owner-pod" // namespace/name of the pod in shadowOwnerUIDLabel ) type ShadowTemplateData struct { @@ -133,8 +134,10 @@ type ShadowTemplateData struct { // in a container named PauseName, a name no kept init container uses. PauseImage string PauseName string - // OwnerUID is stamped on every shadow resource (see shadowResourceIdentity). + // OwnerUID and OwnerPod are stamped on every shadow resource (see + // shadowResourceIdentity). OwnerUID string + OwnerPod string // NodeConfigMap is the ConfigMap the shadow can mount to learn which remote // compute node the workload landed on. It exists from the moment the shadow is // created but holds an empty NodeNameKey until the plugin reports the node. @@ -1083,6 +1086,7 @@ func (p *Provider) createShadowPod(ctx context.Context, originalPod *v1.Pod) (*v PauseImage: p.shadowPauseImage(), PauseName: shadowPauseName(localInitContainers), OwnerUID: identity.OwnerUID, + OwnerPod: identity.ownerPod(), NodeConfigMap: shadowNodeConfigMapName(identity.Name), NodeNameKey: shadowNodeNameKey, SocketDirKey: shadowSocketDirKey, @@ -1463,7 +1467,7 @@ func (p *Provider) applyShadowObjects(ctx context.Context, manifestYAML string, continue } if meta, ok := obj.(metav1.Object); ok && td != nil { - stampShadowOwner(meta, td.OwnerUID) + stampShadowOwner(meta, td.OwnerUID, td.OwnerPod) } switch o := obj.(type) { @@ -1616,7 +1620,7 @@ func (p *Provider) deleteOwnedShadowResource(ctx context.Context, identity shado return err } if owner := obj.GetLabels()[shadowOwnerUIDLabel]; owner != "" && identity.OwnerUID != "" && owner != identity.OwnerUID && - p.ownerIsCurrentPod(ctx, identity, owner) { + p.ownerIsCurrentPod(ctx, identity, obj) { log.G(ctx).Infof("Keeping shadow %s %s/%s: it belongs to pod UID %s, not %s", kind, identity.Namespace, name, owner, identity.OwnerUID) kept = true return nil @@ -1651,8 +1655,9 @@ func (p *Provider) lockShadow(namespace, name string) func() { return mu.Unlock } -// stampShadowOwner labels a shadow resource with the UID of the pod it serves. -func stampShadowOwner(obj metav1.Object, ownerUID string) { +// stampShadowOwner labels a shadow resource with the UID of the pod it serves and +// annotates it with that pod's namespace/name. +func stampShadowOwner(obj metav1.Object, ownerUID, ownerPod string) { if ownerUID == "" { return } @@ -1662,6 +1667,15 @@ func stampShadowOwner(obj metav1.Object, ownerUID string) { } labels[shadowOwnerUIDLabel] = ownerUID obj.SetLabels(labels) + if ownerPod == "" { + return + } + annotations := obj.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + annotations[shadowOwnerPodAnnotation] = ownerPod + obj.SetAnnotations(annotations) } // cleanupPartialShadowResources removes specific resources that were created before a failure From 12379a144c61c6f07495db6e5edc3b9dbe8ec7d8 Mon Sep 17 00:00:00 2001 From: Matteo Bunino <48362942+matbun@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:51:28 +0200 Subject: [PATCH 8/9] test: cover the owner annotation, a shared shadow namespace and old stamps The node ConfigMap test checks that interlink.eu/shadow-owner-pod is written. Two cleanup cases are new: the owner lives in another namespace that maps to the same shadow names, and a resource stamped before the annotation existed is looked up under the cleaning pod's name. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matteo Bunino <48362942+matbun@users.noreply.github.com> --- pkg/virtualkubelet/shadow_test.go | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/pkg/virtualkubelet/shadow_test.go b/pkg/virtualkubelet/shadow_test.go index 86abf810..a112c530 100644 --- a/pkg/virtualkubelet/shadow_test.go +++ b/pkg/virtualkubelet/shadow_test.go @@ -619,9 +619,10 @@ func TestCleanupKeepsOnlyTheCurrentPodsResources(t *testing.T) { stampShadowOwner(&meta, owner, ownerPod) return &v1.Service{ObjectMeta: meta} } - pod := func(podName, uid string) *v1.Pod { - return &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: podName, Namespace: ns, UID: k8stypes.UID(uid)}} + podIn := func(namespace, podName, uid string) *v1.Pod { + return &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: podName, Namespace: namespace, UID: k8stypes.UID(uid)}} } + pod := func(podName, uid string) *v1.Pod { return podIn(ns, podName, uid) } serviceLeft := func(t *testing.T, client *fake.Clientset) bool { t.Helper() tunnelProvider(client).cleanupShadowResources(t.Context(), identity) @@ -655,6 +656,21 @@ func TestCleanupKeepsOnlyTheCurrentPodsResources(t *testing.T) { assert.True(t, serviceLeft(t, client)) }) + // Stamped before the annotation existed: the owner is looked up under the + // cleaning pod's name. + t.Run("without the annotation, deleted when another pod now has the name", func(t *testing.T) { + client := fake.NewSimpleClientset(stampedService(staleUID, ""), pod(podName, currentUID)) + assert.False(t, serviceLeft(t, client)) + }) + + // Namespaces a--b and a-b get the same shadow names too. + t.Run("kept for a live pod of another namespace that shares the shadow name", func(t *testing.T) { + const otherNamespace, otherUID = "other-ns", "uid-elsewhere" + client := fake.NewSimpleClientset(stampedService(otherUID, otherNamespace+"/"+podName), + pod(podName, currentUID), podIn(otherNamespace, podName, otherUID)) + assert.True(t, serviceLeft(t, client)) + }) + // web.0 and web-0 get the same shadow name: deleting one must not take the // shadow the other one, still running, took over. t.Run("kept for a live pod of another name that shares the shadow name", func(t *testing.T) { @@ -748,4 +764,5 @@ func TestShadowIdentityAndNodeConfigMapCarryOwner(t *testing.T) { cm, err := client.CoreV1().ConfigMaps(identity.Namespace).Get(t.Context(), shadowNodeConfigMapName(identity.Name), metav1.GetOptions{}) require.NoError(t, err) assert.Equal(t, "uid-7", cm.Labels[shadowOwnerUIDLabel]) + assert.Equal(t, testNamespaceDefault+"/nb", cm.Annotations[shadowOwnerPodAnnotation]) } From 9f3b4295e3e28b1e3c059260c8203e7aeb2fa090 Mon Sep 17 00:00:00 2001 From: Matteo Bunino <48362942+matbun@users.noreply.github.com> Date: Fri, 23 Oct 2026 18:18:20 +0200 Subject: [PATCH 9/9] refactor: build the node ConfigMap patch from a struct A third "metadata" string literal in the package, as the reverse tunnel adds one, makes goconst fail; the patch is now a typed struct, with the same JSON. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matteo Bunino <48362942+matbun@users.noreply.github.com> --- pkg/virtualkubelet/shadow.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/pkg/virtualkubelet/shadow.go b/pkg/virtualkubelet/shadow.go index 106ebd0b..71b27412 100644 --- a/pkg/virtualkubelet/shadow.go +++ b/pkg/virtualkubelet/shadow.go @@ -403,13 +403,15 @@ func (p *Provider) publishShadowNode(ctx context.Context, pod *v1.Pod, nodeName, return } - patch, err := json.Marshal(map[string]map[string]string{ - "metadata": {"resourceVersion": current.ResourceVersion}, - "data": { - shadowNodeNameKey: nodeName, - shadowSocketDirKey: socketDir, - }, - }) + var nodePatch struct { + Metadata struct { + ResourceVersion string `json:"resourceVersion"` + } `json:"metadata"` + Data map[string]string `json:"data"` + } + nodePatch.Metadata.ResourceVersion = current.ResourceVersion + nodePatch.Data = map[string]string{shadowNodeNameKey: nodeName, shadowSocketDirKey: socketDir} + patch, err := json.Marshal(nodePatch) if err != nil { log.G(ctx).Warningf("Failed to marshal shadow node patch for %s/%s: %v", pod.Namespace, pod.Name, err) return