Skip to content

Commit 96c2e91

Browse files
camilamacedo86camilamacedo86
committed
UPSTREAM: <carry>: [OTE] simplify webhook tests, improve names and logs
1 parent 2f8f54b commit 96c2e91

File tree

4 files changed

+145
-55
lines changed

4 files changed

+145
-55
lines changed

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
package helpers
2+
3+
import (
4+
"context"
5+
"errors"
6+
"fmt"
7+
"os/exec"
8+
"strings"
9+
)
10+
11+
// findK8sTool returns "oc" if available, otherwise "kubectl".
12+
// If we are running locally we either prefer to use oc since some tests
13+
// require it, or fallback to kubectl if oc is not available.
14+
func findK8sTool() (string, error) {
15+
tools := []string{"oc", "kubectl"}
16+
for _, t := range tools {
17+
// First check if the tool is available in the PATH.
18+
if _, err := exec.LookPath(t); err != nil {
19+
continue
20+
}
21+
// Verify that the tool is working by checking its version.
22+
if err := exec.Command(t, "version", "--client").Run(); err == nil {
23+
return t, nil
24+
}
25+
}
26+
return "", fmt.Errorf("no Kubernetes CLI client found (tried %s)",
27+
strings.Join(tools, ", "))
28+
}
29+
30+
// RunK8sCommand runs a Kubernetes CLI command and returns ONLY stdout.
31+
// If the command fails, stderr is included in the returned error (not mixed with stdout).
32+
func RunK8sCommand(ctx context.Context, args ...string) ([]byte, error) {
33+
tool, err := findK8sTool()
34+
if err != nil {
35+
return nil, err
36+
}
37+
38+
cmd := exec.CommandContext(ctx, tool, args...)
39+
out, err := cmd.Output()
40+
if err != nil {
41+
var ee *exec.ExitError
42+
if errors.As(err, &ee) {
43+
stderr := strings.TrimSpace(string(ee.Stderr))
44+
if stderr != "" {
45+
return nil, fmt.Errorf("%s %s failed: %w\nstderr:\n%s",
46+
tool, strings.Join(args, " "), err, stderr)
47+
}
48+
}
49+
return nil, fmt.Errorf("%s %s failed: %w",
50+
tool, strings.Join(args, " "), err)
51+
}
52+
return out, nil
53+
}

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package helpers
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"strings"
7+
"time"
8+
9+
//nolint:staticcheck // ST1001: dot-imports for readability
10+
. "github.com/onsi/ginkgo/v2"
11+
)
12+
13+
// GetAllPodLogs prints logs for all containers in all pods in the given namespace.
14+
func GetAllPodLogs(ctx context.Context, namespace string) {
15+
fmt.Fprintf(GinkgoWriter, "\n[pod-logs] namespace=%s\n", namespace)
16+
17+
By("Getting all pods in the namespace")
18+
namesOut, err := RunK8sCommand(ctx, "get", "pods", "-n", namespace, "-o", "name")
19+
if err != nil {
20+
fmt.Fprintf(GinkgoWriter, "failed to list pods: %v\n%s\n", err, string(namesOut))
21+
return
22+
}
23+
lines := strings.Split(strings.TrimSpace(string(namesOut)), "\n")
24+
if len(lines) == 0 || (len(lines) == 1 && strings.TrimSpace(lines[0]) == "") {
25+
fmt.Fprintln(GinkgoWriter, "no pods found")
26+
return
27+
}
28+
29+
By(fmt.Sprintf("[pod-logs] namespace=%s\n", namespace))
30+
for _, res := range lines {
31+
res = strings.TrimSpace(res)
32+
if res == "" {
33+
continue
34+
}
35+
fmt.Fprintf(GinkgoWriter, "\n--- logs: %s @ %s ---\n", res, time.Now().Format(time.RFC3339))
36+
logsOut, err := RunK8sCommand(
37+
ctx,
38+
"logs",
39+
"-n", namespace,
40+
"--all-containers",
41+
"--prefix",
42+
"--timestamps",
43+
res,
44+
)
45+
if err != nil {
46+
fmt.Fprintf(GinkgoWriter, "error fetching logs: %v\n%s\n", err, string(logsOut))
47+
continue
48+
}
49+
_, _ = GinkgoWriter.Write(logsOut) // ignore write error by design
50+
}
51+
}

‎openshift/tests-extension/test/olmv1-incompatible.go‎

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,6 @@ var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLM][Skipped:Disconnected] OLMv1
6060
}
6161
By(fmt.Sprintf("testing against OCP %s", testVersion))
6262

63-
By("finding a k8s client")
64-
cmdLine, err := getK8sCommandLineClient()
65-
Expect(err).To(Succeed())
66-
6763
By("creating a new Namespace")
6864
nsCleanup := createNamespace(nsName)
6965
DeferCleanup(nsCleanup)
@@ -85,7 +81,7 @@ var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLM][Skipped:Disconnected] OLMv1
8581
DeferCleanup(fileCleanup)
8682
By(fmt.Sprintf("created operator tarball %q", fileOperator))
8783

88-
By(fmt.Sprintf("starting the operator build with %q via RAW URL", cmdLine))
84+
By("starting the operator build with %q via RAW URL")
8985
opArgs := []string{
9086
"create",
9187
"--raw",
@@ -96,7 +92,7 @@ var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLM][Skipped:Disconnected] OLMv1
9692
"-f",
9793
fileOperator,
9894
}
99-
buildOperator := startBuild(cmdLine, opArgs...)
95+
buildOperator := startBuild(opArgs...)
10096

10197
By(fmt.Sprintf("waiting for the build %q to finish", buildOperator.Name))
10298
waitForBuildToFinish(ctx, buildOperator.Name, nsName)
@@ -114,7 +110,7 @@ var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLM][Skipped:Disconnected] OLMv1
114110
DeferCleanup(fileCleanup)
115111
By(fmt.Sprintf("created catalog tarball %q", fileCatalog))
116112

117-
By(fmt.Sprintf("starting the catalog build with %q via RAW URL", cmdLine))
113+
By("starting the catalog build with %q via RAW URL")
118114
catalogArgs := []string{
119115
"create",
120116
"--raw",
@@ -125,7 +121,7 @@ var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLM][Skipped:Disconnected] OLMv1
125121
"-f",
126122
fileCatalog,
127123
}
128-
buildCatalog := startBuild(cmdLine, catalogArgs...)
124+
buildCatalog := startBuild(catalogArgs...)
129125

130126
By(fmt.Sprintf("waiting for the build %q to finish", buildCatalog.Name))
131127
waitForBuildToFinish(ctx, buildCatalog.Name, nsName)
@@ -386,21 +382,15 @@ func waitForClusterOperatorUpgradable(ctx SpecContext, name string) {
386382
}).WithTimeout(5 * time.Minute).WithPolling(1 * time.Second).Should(Succeed())
387383
}
388384

389-
func getK8sCommandLineClient() (string, error) {
390-
s, err := exec.LookPath("kubectl")
391-
if err != nil {
392-
s, err = exec.LookPath("oc")
393-
}
394-
return s, err
395-
}
396-
397-
func startBuild(cmdLine string, args ...string) *buildv1.Build {
398-
cmd := exec.Command(cmdLine, args...)
399-
output, err := cmd.Output()
385+
func startBuild(args ...string) *buildv1.Build {
386+
// Note: cmdLine is intentionally ignored because helpers.RunK8sCommand
387+
// auto-selects "oc" or "kubectl".
388+
out, err := helpers.RunK8sCommand(context.Background(), args...)
400389
Expect(err).To(Succeed(), printExitError(err))
390+
401391
/* The output is JSON of a build.build.openshift.io resource */
402392
build := &buildv1.Build{}
403-
Expect(json.Unmarshal(output, build)).To(Succeed(), "failed to unmarshal build")
393+
Expect(json.Unmarshal(out, build)).To(Succeed(), "failed to unmarshal build")
404394
return build
405395
}
406396

‎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 := newWebhookTest(name, webhookOperatorInstallNamespace, false)
9296

9397
_, err := dynamicClient.
94-
Resource(webhookTestGVRV1).
98+
Resource(webhookTestV1).
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(webhookTestV1).
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 := newWebhookTest(mutatingWebhookResourceName, webhookOperatorInstallNamespace, true)
117121
Eventually(func(g Gomega) {
118-
_, err := dynamicClient.Resource(webhookTestGVRV1).Namespace(webhookOperatorInstallNamespace).Create(ctx, resource, metav1.CreateOptions{})
122+
_, err := dynamicClient.Resource(webhookTestV1).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(webhookTestV1).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 := newWebhookTest(conversionWebhookResourceName, webhookOperatorInstallNamespace, true)
139143
Eventually(func(g Gomega) {
140-
_, err := dynamicClient.Resource(webhookTestGVRV1).Namespace(webhookOperatorInstallNamespace).Create(ctx, resourceV1, metav1.CreateOptions{})
144+
_, err := dynamicClient.Resource(webhookTestV1).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(webhookTestV2).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 := newWebhookTest(resourceName, webhookOperatorInstallNamespace, true)
188192

189-
_, err := dynamicClient.Resource(webhookTestGVRV1).Namespace(webhookOperatorInstallNamespace).Create(ctx, resource, metav1.CreateOptions{})
193+
_, err := dynamicClient.Resource(webhookTestV1).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(webhookTestV1).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 := newWebhookTest(resourceName, webhookOperatorInstallNamespace, true)
231235

232-
_, err := dynamicClient.Resource(webhookTestGVRV1).Namespace(webhookOperatorInstallNamespace).Create(ctx, resource, metav1.CreateOptions{})
236+
_, err := dynamicClient.Resource(webhookTestV1).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(webhookTestV1).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 webhookTestV1 = schema.GroupVersionResource{
242246
Group: "webhook.operators.coreos.io",
243247
Version: "v1",
244248
Resource: "webhooktests",
245249
}
246250

247-
var webhookTestGVRV2 = schema.GroupVersionResource{
251+
var webhookTestV2 = 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 newWebhookTest(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)