Skip to content

Commit 158497e

Browse files
camilamacedo86camilamacedo86
committed
UPSTREAM: <carry>: [OTE] simplify webhook tests, improve names and logs
1 parent 5ad2b55 commit 158497e

File tree

3 files changed

+90
-35
lines changed

3 files changed

+90
-35
lines changed

‎openshift/tests-extension/pkg/helpers/commands.go‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package helpers
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"os/exec"
7+
)
8+
9+
// findK8sClient returns "oc" if available, otherwise "kubectl".
10+
func findK8sClient() (string, error) {
11+
clients := []string{"oc", "kubectl"}
12+
for _, c := range clients {
13+
if err := exec.Command(c, "version", "--client").Run(); err == nil {
14+
return c, nil
15+
}
16+
}
17+
return "", fmt.Errorf("no Kubernetes CLI client found (tried oc, kubectl)")
18+
}
19+
20+
// RunK8sCommand runs a Kubernetes CLI command with the given args and returns combined output.
21+
func RunK8sCommand(ctx context.Context, args ...string) ([]byte, error) {
22+
client, err := findK8sClient()
23+
if err != nil {
24+
return nil, err
25+
}
26+
27+
cmd := exec.CommandContext(ctx, client, args...)
28+
data, err := cmd.CombinedOutput()
29+
if err != nil {
30+
return data, fmt.Errorf("command failed: %v\n%s", err, data)
31+
}
32+
return data, nil
33+
}

‎openshift/tests-extension/pkg/helpers/pod_logs.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package helpers
2+
3+
import (
4+
"context"
5+
"fmt"
6+
7+
//nolint:staticcheck // ST1001: dot-imports for readability
8+
. "github.com/onsi/ginkgo/v2"
9+
)
10+
11+
// GetAllPodLogs runs logs command with context and writes the result to GinkgoWriter.
12+
func GetAllPodLogs(ctx context.Context, namespace string) {
13+
data, err := RunK8sCommand(ctx,
14+
"logs",
15+
"-n", namespace,
16+
"--all-containers",
17+
"--prefix",
18+
"--timestamps",
19+
)
20+
fmt.Fprintf(GinkgoWriter, "\n[pod-logs] namespace=%s\n", namespace)
21+
if err != nil {
22+
fmt.Fprintf(GinkgoWriter, "Error fetching logs: %v\nOutput:\n%s\n", err, string(data))
23+
return
24+
}
25+
_, _ = GinkgoWriter.Write(data)
26+
}

‎openshift/tests-extension/test/webhooks.go‎

Lines changed: 31 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1818
"k8s.io/apimachinery/pkg/runtime/schema"
1919
"k8s.io/apimachinery/pkg/util/rand"
20-
"k8s.io/apimachinery/pkg/util/wait"
2120
"k8s.io/client-go/dynamic"
2221
"sigs.k8s.io/controller-runtime/pkg/client"
2322

@@ -46,7 +45,7 @@ var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLMWebhookProviderOpenshiftServi
4645
)
4746

4847
BeforeEach(func(ctx SpecContext) {
49-
By("initializing Kubernetes client and dynamic client")
48+
By("initializing Kubernetes client")
5049
k8sClient = env.Get().K8sClient
5150
restCfg := env.Get().RestCfg
5251
var err error
@@ -78,6 +77,11 @@ var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLMWebhookProviderOpenshiftServi
7877
})
7978

8079
AfterEach(func(ctx SpecContext) {
80+
if CurrentSpecReport().Failed() {
81+
By("dumping pod logs for debugging")
82+
helpers.GetAllPodLogs(ctx, webhookOperatorInstallNamespace)
83+
}
84+
8185
By("performing webhook operator cleanup")
8286
if cleanup != nil {
8387
cleanup(ctx)
@@ -88,17 +92,17 @@ var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLMWebhookProviderOpenshiftServi
8892
By("creating a webhook test resource that will be rejected by the validating webhook")
8993
Eventually(func() error {
9094
name := fmt.Sprintf("validating-webhook-test-%s", rand.String(5))
91-
obj := newWebhookTestV1(name, webhookOperatorInstallNamespace, false)
95+
obj := newTestWebhook(name, webhookOperatorInstallNamespace, false)
9296

9397
_, err := dynamicClient.
94-
Resource(webhookTestGVRV1).
98+
Resource(testWebhookV1).
9599
Namespace(webhookOperatorInstallNamespace).
96100
Create(ctx, obj, metav1.CreateOptions{})
97101

98102
switch {
99103
case err == nil:
100104
// Webhook not ready yet; clean up and keep polling.
101-
_ = dynamicClient.Resource(webhookTestGVRV1).
105+
_ = dynamicClient.Resource(testWebhookV1).
102106
Namespace(webhookOperatorInstallNamespace).
103107
Delete(ctx, name, metav1.DeleteOptions{})
104108
return fmt.Errorf("webhook not rejecting yet")
@@ -111,16 +115,16 @@ var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLMWebhookProviderOpenshiftServi
111115
})
112116

113117
It("should have a working mutating webhook", func(ctx SpecContext) {
114-
By("creating a valid webhook test resource")
118+
By("creating a valid webhook")
115119
mutatingWebhookResourceName := "mutating-webhook-test"
116-
resource := newWebhookTestV1(mutatingWebhookResourceName, webhookOperatorInstallNamespace, true)
120+
resource := newTestWebhook(mutatingWebhookResourceName, webhookOperatorInstallNamespace, true)
117121
Eventually(func(g Gomega) {
118-
_, err := dynamicClient.Resource(webhookTestGVRV1).Namespace(webhookOperatorInstallNamespace).Create(ctx, resource, metav1.CreateOptions{})
122+
_, err := dynamicClient.Resource(testWebhookV1).Namespace(webhookOperatorInstallNamespace).Create(ctx, resource, metav1.CreateOptions{})
119123
g.Expect(err).ToNot(HaveOccurred())
120124
}).WithTimeout(1 * time.Minute).WithPolling(5 * time.Second).Should(Succeed())
121125

122126
By("getting the created resource in v1 schema")
123-
obj, err := dynamicClient.Resource(webhookTestGVRV1).Namespace(webhookOperatorInstallNamespace).Get(ctx, mutatingWebhookResourceName, metav1.GetOptions{})
127+
obj, err := dynamicClient.Resource(testWebhookV1).Namespace(webhookOperatorInstallNamespace).Get(ctx, mutatingWebhookResourceName, metav1.GetOptions{})
124128
Expect(err).ToNot(HaveOccurred())
125129
Expect(obj).ToNot(BeNil())
126130

@@ -135,14 +139,14 @@ var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLMWebhookProviderOpenshiftServi
135139
It("should have a working conversion webhook", func(ctx SpecContext) {
136140
By("creating a conversion webhook test resource")
137141
conversionWebhookResourceName := "conversion-webhook-test"
138-
resourceV1 := newWebhookTestV1(conversionWebhookResourceName, webhookOperatorInstallNamespace, true)
142+
resourceV1 := newTestWebhook(conversionWebhookResourceName, webhookOperatorInstallNamespace, true)
139143
Eventually(func(g Gomega) {
140-
_, err := dynamicClient.Resource(webhookTestGVRV1).Namespace(webhookOperatorInstallNamespace).Create(ctx, resourceV1, metav1.CreateOptions{})
144+
_, err := dynamicClient.Resource(testWebhookV1).Namespace(webhookOperatorInstallNamespace).Create(ctx, resourceV1, metav1.CreateOptions{})
141145
g.Expect(err).ToNot(HaveOccurred())
142146
}).WithTimeout(1 * time.Minute).WithPolling(5 * time.Second).Should(Succeed())
143147

144148
By("getting the created resource in v2 schema")
145-
obj, err := dynamicClient.Resource(webhookTestGVRV2).Namespace(webhookOperatorInstallNamespace).Get(ctx, conversionWebhookResourceName, metav1.GetOptions{})
149+
obj, err := dynamicClient.Resource(testWebhookV2).Namespace(webhookOperatorInstallNamespace).Get(ctx, conversionWebhookResourceName, metav1.GetOptions{})
146150
Expect(err).ToNot(HaveOccurred())
147151
Expect(obj).ToNot(BeNil())
148152

@@ -184,12 +188,12 @@ var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLMWebhookProviderOpenshiftServi
184188
By("checking webhook is responsive through cert rotation")
185189
Eventually(func(g Gomega) {
186190
resourceName := fmt.Sprintf("cert-rotation-test-%s", rand.String(5))
187-
resource := newWebhookTestV1(resourceName, webhookOperatorInstallNamespace, true)
191+
resource := newTestWebhook(resourceName, webhookOperatorInstallNamespace, true)
188192

189-
_, err := dynamicClient.Resource(webhookTestGVRV1).Namespace(webhookOperatorInstallNamespace).Create(ctx, resource, metav1.CreateOptions{})
193+
_, err := dynamicClient.Resource(testWebhookV1).Namespace(webhookOperatorInstallNamespace).Create(ctx, resource, metav1.CreateOptions{})
190194
g.Expect(err).ToNot(HaveOccurred(), fmt.Sprintf("failed to create test resource %s: %v", resourceName, err))
191195

192-
err = dynamicClient.Resource(webhookTestGVRV1).Namespace(webhookOperatorInstallNamespace).Delete(ctx, resource.GetName(), metav1.DeleteOptions{})
196+
err = dynamicClient.Resource(testWebhookV1).Namespace(webhookOperatorInstallNamespace).Delete(ctx, resource.GetName(), metav1.DeleteOptions{})
193197
g.Expect(client.IgnoreNotFound(err)).ToNot(HaveOccurred(), fmt.Sprintf("failed to delete test resource %s: %v", resourceName, err))
194198
}).WithTimeout(2 * time.Minute).WithPolling(10 * time.Second).Should(Succeed())
195199
})
@@ -227,30 +231,30 @@ var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLMWebhookProviderOpenshiftServi
227231

228232
Eventually(func(g Gomega) {
229233
resourceName := fmt.Sprintf("tls-deletion-test-%s", rand.String(5))
230-
resource := newWebhookTestV1(resourceName, webhookOperatorInstallNamespace, true)
234+
resource := newTestWebhook(resourceName, webhookOperatorInstallNamespace, true)
231235

232-
_, err := dynamicClient.Resource(webhookTestGVRV1).Namespace(webhookOperatorInstallNamespace).Create(ctx, resource, metav1.CreateOptions{})
236+
_, err := dynamicClient.Resource(testWebhookV1).Namespace(webhookOperatorInstallNamespace).Create(ctx, resource, metav1.CreateOptions{})
233237
g.Expect(err).ToNot(HaveOccurred(), fmt.Sprintf("failed to create test resource %s: %v", resourceName, err))
234238

235-
err = dynamicClient.Resource(webhookTestGVRV1).Namespace(webhookOperatorInstallNamespace).Delete(ctx, resource.GetName(), metav1.DeleteOptions{})
239+
err = dynamicClient.Resource(testWebhookV1).Namespace(webhookOperatorInstallNamespace).Delete(ctx, resource.GetName(), metav1.DeleteOptions{})
236240
g.Expect(client.IgnoreNotFound(err)).ToNot(HaveOccurred(), fmt.Sprintf("failed to delete test resource %s: %v", resourceName, err))
237241
}).WithTimeout(2 * time.Minute).WithPolling(10 * time.Second).Should(Succeed())
238242
})
239243
})
240244

241-
var webhookTestGVRV1 = schema.GroupVersionResource{
245+
var testWebhookV1 = schema.GroupVersionResource{
242246
Group: "webhook.operators.coreos.io",
243247
Version: "v1",
244248
Resource: "webhooktests",
245249
}
246250

247-
var webhookTestGVRV2 = schema.GroupVersionResource{
251+
var testWebhookV2 = schema.GroupVersionResource{
248252
Group: "webhook.operators.coreos.io",
249253
Version: "v2",
250254
Resource: "webhooktests",
251255
}
252256

253-
func newWebhookTestV1(name, namespace string, valid bool) *unstructured.Unstructured {
257+
func newTestWebhook(name, namespace string, valid bool) *unstructured.Unstructured {
254258
mutateValue := valid
255259
obj := &unstructured.Unstructured{
256260
Object: map[string]interface{}{
@@ -344,19 +348,11 @@ func setupWebhookOperator(ctx SpecContext, k8sClient client.Client, webhookOpera
344348
_ = k8sClient.Delete(ctx, ns, client.PropagationPolicy(metav1.DeletePropagationForeground))
345349

346350
By(fmt.Sprintf("waiting for namespace %s to be fully deleted", webhookOperatorInstallNamespace))
347-
pollErr := wait.PollUntilContextTimeout(ctx, 5*time.Second, 2*time.Minute, true, func(pollCtx context.Context) (bool, error) {
348-
var currentNS corev1.Namespace
349-
err := k8sClient.Get(pollCtx, client.ObjectKey{Name: webhookOperatorInstallNamespace}, &currentNS)
350-
if err != nil {
351-
if apierrors.IsNotFound(err) {
352-
return true, nil
353-
}
354-
return false, err
355-
}
356-
return false, nil
357-
})
358-
if pollErr != nil {
359-
GinkgoLogr.Info(fmt.Sprintf("Warning: namespace %s deletion wait failed: %v", webhookOperatorInstallNamespace, pollErr))
360-
}
351+
Eventually(func(g Gomega) {
352+
ns := &corev1.Namespace{}
353+
err := k8sClient.Get(ctx, client.ObjectKey{Name: webhookOperatorInstallNamespace}, ns)
354+
g.Expect(client.IgnoreNotFound(err)).To(Succeed())
355+
g.Expect(apierrors.IsNotFound(err)).To(BeTrue(), "namespace still exists")
356+
}).WithTimeout(3 * time.Minute).WithPolling(5 * time.Second).Should(Succeed())
361357
}
362358
}

0 commit comments

Comments
 (0)