Skip to content

Commit 39226c1

Browse files
committed
Fix watch mapping for Sandboxes with adopted pods
When a Sandbox was adopting a pod from a WarmPool, the Sandbox controller was not properly watching the pod because its name didn't match the Sandbox name. This commit fixes the controller to properly watch adopted pods. Signed-off-by: Anton Ippolitov <[email protected]>
1 parent 36c411a commit 39226c1

File tree

4 files changed

+173
-2
lines changed

4 files changed

+173
-2
lines changed

controllers/sandbox_controller.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,7 @@ func (r *SandboxReconciler) SetupWithManager(mgr ctrl.Manager) error {
540540
}
541541
return ctrl.NewControllerManagedBy(mgr).
542542
For(&sandboxv1alpha1.Sandbox{}).
543-
Watches(&corev1.Pod{}, &handler.EnqueueRequestForObject{}, builder.WithPredicates(labelSelectorPredicate)).
544-
Watches(&corev1.Service{}, &handler.EnqueueRequestForObject{}, builder.WithPredicates(labelSelectorPredicate)).
543+
Owns(&corev1.Pod{}, builder.WithPredicates(labelSelectorPredicate)).
544+
Owns(&corev1.Service{}, builder.WithPredicates(labelSelectorPredicate)).
545545
Complete(r)
546546
}

test/e2e/framework/client.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,35 @@ type ClusterClient struct {
4646
restConfig *rest.Config
4747
}
4848

49+
// List retrieves a list of objects matching the provided options.
50+
func (cl *ClusterClient) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
51+
cl.Helper()
52+
if err := cl.client.List(ctx, list, opts...); err != nil {
53+
return fmt.Errorf("list %T: %w", list, err)
54+
}
55+
return nil
56+
}
57+
58+
// Delete deletes the specified object from the cluster.
59+
func (cl *ClusterClient) Delete(ctx context.Context, obj client.Object) error {
60+
cl.Helper()
61+
nn := types.NamespacedName{Name: obj.GetName(), Namespace: obj.GetNamespace()}
62+
cl.Logf("Deleting object %T (%s)", obj, nn.String())
63+
if err := cl.client.Delete(ctx, obj); err != nil {
64+
return fmt.Errorf("delete %T (%s): %w", obj, nn.String(), err)
65+
}
66+
return nil
67+
}
68+
69+
// Get retrieves an object from the cluster.
70+
func (cl *ClusterClient) Get(ctx context.Context, key types.NamespacedName, obj client.Object) error {
71+
cl.Helper()
72+
if err := cl.client.Get(ctx, key, obj); err != nil {
73+
return err
74+
}
75+
return nil
76+
}
77+
4978
// Update an object that already exists on the cluster.
5079
func (cl *ClusterClient) Update(ctx context.Context, obj client.Object) error {
5180
cl.Helper()
@@ -177,6 +206,7 @@ func (cl *ClusterClient) validateAgentSandboxInstallation(ctx context.Context) e
177206
"sandboxes.agents.x-k8s.io",
178207
"sandboxclaims.extensions.agents.x-k8s.io",
179208
"sandboxtemplates.extensions.agents.x-k8s.io",
209+
"sandboxwarmpools.extensions.agents.x-k8s.io",
180210
}
181211
for _, name := range crds {
182212
crd := &apiextensionsv1.CustomResourceDefinition{}

test/e2e/framework/context.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
2424
"k8s.io/client-go/tools/clientcmd"
2525
"sigs.k8s.io/agent-sandbox/controllers"
26+
extensionsv1alpha1 "sigs.k8s.io/agent-sandbox/extensions/api/v1alpha1"
2627
"sigs.k8s.io/controller-runtime/pkg/client"
2728
)
2829

@@ -37,6 +38,7 @@ var (
3738

3839
func init() {
3940
utilruntime.Must(apiextensionsv1.AddToScheme(controllers.Scheme))
41+
utilruntime.Must(extensionsv1alpha1.AddToScheme(controllers.Scheme))
4042
}
4143

4244
// TestContext is a helper for managing e2e test scaffolding.
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
// Copyright 2025 The Kubernetes Authors.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package e2e
16+
17+
import (
18+
"testing"
19+
"time"
20+
21+
"github.com/stretchr/testify/require"
22+
corev1 "k8s.io/api/core/v1"
23+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
24+
"k8s.io/apimachinery/pkg/types"
25+
sandboxv1alpha1 "sigs.k8s.io/agent-sandbox/api/v1alpha1"
26+
extensionsv1alpha1 "sigs.k8s.io/agent-sandbox/extensions/api/v1alpha1"
27+
"sigs.k8s.io/agent-sandbox/test/e2e/framework"
28+
"sigs.k8s.io/controller-runtime/pkg/client"
29+
)
30+
31+
// TestWarmPoolSandboxWatcher verifies that Sandboxes created from WarmPools watch the right underlying pods
32+
func TestWarmPoolSandboxWatcher(t *testing.T) {
33+
tc := framework.NewTestContext(t)
34+
35+
// Set up a namespace
36+
ns := &corev1.Namespace{}
37+
ns.Name = "my-sandbox-ns"
38+
require.NoError(t, tc.CreateWithCleanup(t.Context(), ns))
39+
40+
// Create a SandboxTemplate
41+
template := &extensionsv1alpha1.SandboxTemplate{}
42+
template.Name = "test-template"
43+
template.Namespace = ns.Name
44+
template.Spec.PodTemplate = sandboxv1alpha1.PodTemplate{
45+
Spec: corev1.PodSpec{
46+
Containers: []corev1.Container{
47+
{
48+
Name: "pause",
49+
Image: "registry.k8s.io/pause:3.10",
50+
},
51+
},
52+
},
53+
}
54+
require.NoError(t, tc.CreateWithCleanup(t.Context(), template))
55+
56+
// Create a SandboxWarmPool
57+
warmPool := &extensionsv1alpha1.SandboxWarmPool{}
58+
warmPool.Name = "test-warmpool"
59+
warmPool.Namespace = ns.Name
60+
warmPool.Spec.TemplateRef.Name = template.Name
61+
warmPool.Spec.Replicas = 1
62+
require.NoError(t, tc.CreateWithCleanup(t.Context(), warmPool))
63+
64+
// Wait for warm pool to create a pod
65+
var poolPodName string
66+
require.Eventually(t, func() bool {
67+
podList := &corev1.PodList{}
68+
if err := tc.List(t.Context(), podList, client.InNamespace(ns.Name)); err != nil {
69+
return false
70+
}
71+
for _, pod := range podList.Items {
72+
if _, hasLabel := pod.Labels["agents.x-k8s.io/pool"]; hasLabel && pod.DeletionTimestamp.IsZero() {
73+
poolPodName = pod.Name
74+
return true
75+
}
76+
}
77+
return false
78+
}, 60*time.Second, 2*time.Second)
79+
80+
// Create a SandboxClaim to adopt the pod
81+
claim := &extensionsv1alpha1.SandboxClaim{}
82+
claim.Name = "test-claim"
83+
claim.Namespace = ns.Name
84+
claim.Spec.TemplateRef.Name = template.Name
85+
require.NoError(t, tc.CreateWithCleanup(t.Context(), claim))
86+
87+
// Wait for claim to create sandbox
88+
var sandbox *sandboxv1alpha1.Sandbox
89+
require.Eventually(t, func() bool {
90+
if err := tc.Get(t.Context(), types.NamespacedName{Name: claim.Name, Namespace: ns.Name}, claim); err != nil {
91+
return false
92+
}
93+
if claim.Status.SandboxStatus.Name == "" {
94+
return false
95+
}
96+
sandbox = &sandboxv1alpha1.Sandbox{}
97+
return tc.Get(t.Context(), types.NamespacedName{Name: claim.Status.SandboxStatus.Name, Namespace: ns.Name}, sandbox) == nil
98+
}, 30*time.Second, 1*time.Second)
99+
100+
// Wait for pod to be adopted by sandbox
101+
var adoptedPod *corev1.Pod
102+
require.Eventually(t, func() bool {
103+
adoptedPod = &corev1.Pod{}
104+
if err := tc.Get(t.Context(), types.NamespacedName{Name: poolPodName, Namespace: ns.Name}, adoptedPod); err != nil {
105+
return false
106+
}
107+
return metav1.IsControlledBy(adoptedPod, sandbox)
108+
}, 30*time.Second, 1*time.Second)
109+
110+
// Wait for sandbox to become ready
111+
require.Eventually(t, func() bool {
112+
if err := tc.Get(t.Context(), types.NamespacedName{Name: sandbox.Name, Namespace: ns.Name}, sandbox); err != nil {
113+
return false
114+
}
115+
for _, cond := range sandbox.Status.Conditions {
116+
if cond.Type == "Ready" && cond.Status == metav1.ConditionTrue {
117+
return true
118+
}
119+
}
120+
return false
121+
}, 30*time.Second, 1*time.Second)
122+
123+
// Delete the pod
124+
require.NoError(t, tc.Delete(t.Context(), adoptedPod))
125+
126+
// Verify sandbox status updates to reflect pod deletion
127+
// NOTE: This is the critical step that verifies that the Sandbox is watching the right pod.
128+
require.Eventually(t, func() bool {
129+
if err := tc.Get(t.Context(), types.NamespacedName{Name: sandbox.Name, Namespace: ns.Name}, sandbox); err != nil {
130+
return false
131+
}
132+
for _, cond := range sandbox.Status.Conditions {
133+
if cond.Type == "Ready" && cond.Status != metav1.ConditionTrue {
134+
return true
135+
}
136+
}
137+
return false
138+
}, 15*time.Second, 500*time.Millisecond)
139+
}

0 commit comments

Comments
 (0)