Skip to content

Commit 25ca8fc

Browse files
gflarityrenormalize
gflarity
and
renormalize
authored
test coverage for internal/webhooks (#230)
* test coverage for internal/webhooks * network packing was removed Signed-off-by: Geoff Flarity <gflarity@nvidia.com> * fix lint issues Signed-off-by: Geoff Flarity <gflarity@nvidia.com> * more lint fixes Signed-off-by: Geoff Flarity <gflarity@nvidia.com> * Update operator/internal/webhook/register_test.go Co-authored-by: Saketh Kalaga <51327242+renormalize@users.noreply.github.com> Signed-off-by: Geoff Flarity <geoff.flarity@gmail.com> * Update operator/internal/webhook/register_test.go Co-authored-by: Saketh Kalaga <51327242+renormalize@users.noreply.github.com> Signed-off-by: Geoff Flarity <geoff.flarity@gmail.com> * Update operator/internal/webhook/register_test.go Co-authored-by: Saketh Kalaga <51327242+renormalize@users.noreply.github.com> Signed-off-by: Geoff Flarity <geoff.flarity@gmail.com> * PR feedback Signed-off-by: Geoff Flarity <gflarity@nvidia.com> --------- Signed-off-by: Geoff Flarity <gflarity@nvidia.com> Signed-off-by: Geoff Flarity <geoff.flarity@gmail.com> Co-authored-by: Saketh Kalaga <51327242+renormalize@users.noreply.github.com>
1 parent e91e7b5 commit 25ca8fc

File tree

10 files changed

+1817
-3
lines changed

10 files changed

+1817
-3
lines changed

‎operator/internal/webhook/admission/pcs/authorization/register_test.go‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// /*
2+
// Copyright 2025 The Grove Authors.
3+
//
4+
// Licensed under the Apache License, Version 2.0 (the "License");
5+
// you may not use this file except in compliance with the License.
6+
// You may obtain a copy of the License at
7+
//
8+
// http://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
// */
16+
17+
package authorization
18+
19+
import (
20+
"testing"
21+
22+
groveconfigv1alpha1 "github.com/ai-dynamo/grove/operator/api/config/v1alpha1"
23+
testutils "github.com/ai-dynamo/grove/operator/test/utils"
24+
25+
"github.com/go-logr/logr"
26+
"github.com/stretchr/testify/require"
27+
"sigs.k8s.io/controller-runtime/pkg/webhook"
28+
)
29+
30+
// TestRegisterWithManager tests webhook registration with the controller manager.
31+
func TestRegisterWithManager(t *testing.T) {
32+
cl := testutils.NewTestClientBuilder().Build()
33+
mgr := &testutils.FakeManager{
34+
Client: cl,
35+
Scheme: cl.Scheme(),
36+
Logger: logr.Discard(),
37+
}
38+
39+
// Create a real webhook server
40+
server := webhook.NewServer(webhook.Options{
41+
Port: 9443,
42+
})
43+
mgr.WebhookServer = server
44+
45+
handler := NewHandler(mgr, groveconfigv1alpha1.AuthorizerConfig{
46+
Enabled: true,
47+
}, "system:serviceaccount:default:test-sa")
48+
err := handler.RegisterWithManager(mgr)
49+
require.NoError(t, err)
50+
}

‎operator/internal/webhook/admission/pcs/defaulting/handler_test.go‎

Lines changed: 222 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
1+
// /*
2+
// Copyright 2024 The Grove Authors.
3+
//
4+
// Licensed under the Apache License, Version 2.0 (the "License");
5+
// you may not use this file except in compliance with the License.
6+
// You may obtain a copy of the License at
7+
//
8+
// http://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
// */
16+
17+
package defaulting
18+
19+
import (
20+
"context"
21+
"testing"
22+
"time"
23+
24+
grovecorev1alpha1 "github.com/ai-dynamo/grove/operator/api/core/v1alpha1"
25+
testutils "github.com/ai-dynamo/grove/operator/test/utils"
26+
27+
"github.com/go-logr/logr"
28+
"github.com/stretchr/testify/assert"
29+
"github.com/stretchr/testify/require"
30+
admissionv1 "k8s.io/api/admission/v1"
31+
authenticationv1 "k8s.io/api/authentication/v1"
32+
corev1 "k8s.io/api/core/v1"
33+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
34+
"k8s.io/apimachinery/pkg/runtime"
35+
"k8s.io/apimachinery/pkg/util/uuid"
36+
"k8s.io/utils/ptr"
37+
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
38+
)
39+
40+
// TestNewHandler tests the creation of a new defaulting handler.
41+
func TestNewHandler(t *testing.T) {
42+
cl := testutils.NewTestClientBuilder().Build()
43+
mgr := &testutils.FakeManager{
44+
Client: cl,
45+
Scheme: cl.Scheme(),
46+
Logger: logr.Discard(),
47+
}
48+
49+
handler := NewHandler(mgr)
50+
require.NotNil(t, handler)
51+
assert.NotNil(t, handler.logger)
52+
}
53+
54+
// TestDefault tests the Default method which applies default values to PodCliqueSet.
55+
func TestDefault(t *testing.T) {
56+
tests := []struct {
57+
// name identifies this test case
58+
name string
59+
// obj is the runtime object to apply defaults to
60+
obj runtime.Object
61+
// setupContext sets up the admission context if needed
62+
setupContext func(context.Context) context.Context
63+
// expectError indicates whether defaulting should fail
64+
expectError bool
65+
// errorContains is a substring expected in the error message
66+
errorContains string
67+
// verify is a function to verify the defaulting was applied correctly
68+
verify func(*testing.T, runtime.Object)
69+
}{
70+
{
71+
name: "valid PodCliqueSet has defaults applied",
72+
obj: testutils.NewPodCliqueSetBuilder("test-pcs", "default", uuid.NewUUID()).
73+
WithReplicas(1).
74+
WithTerminationDelay(4 * time.Hour).
75+
WithCliqueStartupType(ptr.To(grovecorev1alpha1.CliqueStartupTypeAnyOrder)).
76+
WithPodCliqueTemplateSpec(
77+
testutils.NewPodCliqueTemplateSpecBuilder("test").
78+
WithReplicas(1).
79+
WithRoleName("test-role").
80+
WithMinAvailable(1).
81+
Build()).
82+
Build(),
83+
setupContext: func(ctx context.Context) context.Context {
84+
return admission.NewContextWithRequest(ctx, admission.Request{
85+
AdmissionRequest: admissionv1.AdmissionRequest{
86+
Name: "test-pcs",
87+
Namespace: "default",
88+
Operation: admissionv1.Create,
89+
UserInfo: authenticationv1.UserInfo{
90+
Username: "test-user",
91+
},
92+
},
93+
})
94+
},
95+
expectError: false,
96+
verify: func(t *testing.T, obj runtime.Object) {
97+
pcs, ok := obj.(*grovecorev1alpha1.PodCliqueSet)
98+
require.True(t, ok)
99+
// Verify that some defaults are set (e.g., startup type should remain as set)
100+
assert.NotNil(t, pcs.Spec.Template.StartupType)
101+
},
102+
},
103+
{
104+
name: "PodCliqueSet without startup type gets default",
105+
obj: &grovecorev1alpha1.PodCliqueSet{
106+
ObjectMeta: metav1.ObjectMeta{
107+
Name: "test-pcs",
108+
Namespace: "default",
109+
},
110+
Spec: grovecorev1alpha1.PodCliqueSetSpec{
111+
Replicas: 1,
112+
Template: grovecorev1alpha1.PodCliqueSetTemplateSpec{
113+
StartupType: nil,
114+
Cliques: []*grovecorev1alpha1.PodCliqueTemplateSpec{
115+
{
116+
Name: "test",
117+
Spec: grovecorev1alpha1.PodCliqueSpec{
118+
Replicas: 1,
119+
RoleName: "test-role",
120+
PodSpec: corev1.PodSpec{
121+
Containers: []corev1.Container{
122+
{
123+
Name: "test",
124+
Image: "test:latest",
125+
},
126+
},
127+
},
128+
},
129+
},
130+
},
131+
},
132+
},
133+
},
134+
setupContext: func(ctx context.Context) context.Context {
135+
return admission.NewContextWithRequest(ctx, admission.Request{
136+
AdmissionRequest: admissionv1.AdmissionRequest{
137+
Name: "test-pcs",
138+
Namespace: "default",
139+
Operation: admissionv1.Create,
140+
UserInfo: authenticationv1.UserInfo{
141+
Username: "test-user",
142+
},
143+
},
144+
})
145+
},
146+
expectError: false,
147+
verify: func(t *testing.T, obj runtime.Object) {
148+
pcs, ok := obj.(*grovecorev1alpha1.PodCliqueSet)
149+
require.True(t, ok)
150+
// Verify that the termination delay is set (this is one of the defaults applied)
151+
assert.NotNil(t, pcs.Spec.Template.TerminationDelay)
152+
},
153+
},
154+
{
155+
name: "wrong object type returns error",
156+
obj: &corev1.Pod{},
157+
setupContext: func(ctx context.Context) context.Context {
158+
return admission.NewContextWithRequest(ctx, admission.Request{
159+
AdmissionRequest: admissionv1.AdmissionRequest{
160+
Name: "test-pod",
161+
Namespace: "default",
162+
Operation: admissionv1.Create,
163+
UserInfo: authenticationv1.UserInfo{
164+
Username: "test-user",
165+
},
166+
},
167+
})
168+
},
169+
expectError: true,
170+
errorContains: "expected an PodCliqueSet object",
171+
},
172+
{
173+
name: "context without admission request returns error",
174+
obj: testutils.NewPodCliqueSetBuilder("test-pcs", "default", uuid.NewUUID()).
175+
WithReplicas(1).
176+
WithTerminationDelay(4 * time.Hour).
177+
WithCliqueStartupType(ptr.To(grovecorev1alpha1.CliqueStartupTypeAnyOrder)).
178+
WithPodCliqueTemplateSpec(
179+
testutils.NewPodCliqueTemplateSpecBuilder("test").
180+
WithReplicas(1).
181+
WithRoleName("test-role").
182+
WithMinAvailable(1).
183+
Build()).
184+
Build(),
185+
setupContext: func(ctx context.Context) context.Context { return ctx },
186+
expectError: true,
187+
errorContains: "not found in context",
188+
},
189+
}
190+
191+
for _, tt := range tests {
192+
t.Run(tt.name, func(t *testing.T) {
193+
cl := testutils.NewTestClientBuilder().Build()
194+
mgr := &testutils.FakeManager{
195+
Client: cl,
196+
Scheme: cl.Scheme(),
197+
Logger: logr.Discard(),
198+
}
199+
200+
handler := NewHandler(mgr)
201+
202+
ctx := context.Background()
203+
if tt.setupContext != nil {
204+
ctx = tt.setupContext(ctx)
205+
}
206+
207+
err := handler.Default(ctx, tt.obj)
208+
209+
if tt.expectError {
210+
require.Error(t, err)
211+
if tt.errorContains != "" {
212+
assert.Contains(t, err.Error(), tt.errorContains)
213+
}
214+
} else {
215+
assert.NoError(t, err)
216+
if tt.verify != nil {
217+
tt.verify(t, tt.obj)
218+
}
219+
}
220+
})
221+
}
222+
}

0 commit comments

Comments
 (0)