-
Notifications
You must be signed in to change notification settings - Fork 1.6k
🐛 Fix flakes in e2e tests for metrics #5138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -216,6 +216,41 @@ var _ = Describe("Manager", Ordered, func() { | |
| } | ||
| Eventually(verifyMetricsServerStarted).Should(Succeed()) | ||
|
|
||
| By("waiting for webhook service to be ready") | ||
| verifyWebhookServiceReady := func(g Gomega) { | ||
| const webhookServiceName = "project-webhook-service" | ||
|
|
||
| // Webhook service should exist since webhooks are configured | ||
| cmd := exec.Command("kubectl", "get", "service", webhookServiceName, "-n", namespace) | ||
| _, err := utils.Run(cmd) | ||
| g.Expect(err).NotTo(HaveOccurred(), "Webhook service should exist but was not found") | ||
|
|
||
| // Check if webhook server is ready by verifying pod readiness | ||
| cmd = exec.Command("kubectl", "get", "pods", "-l", "control-plane=controller-manager", | ||
| "-n", namespace, "-o", "jsonpath={.items[0].status.conditions[?(@.type=='Ready')].status}") | ||
| output, err := utils.Run(cmd) | ||
| g.Expect(err).NotTo(HaveOccurred()) | ||
| g.Expect(output).To(Equal("True"), | ||
| "Controller manager pod not ready (webhook server may not be accepting connections)") | ||
|
|
||
| // Check if webhook service endpoints are available | ||
| cmd = exec.Command("kubectl", "get", "endpoints", webhookServiceName, | ||
| "-n", namespace, "-o", "jsonpath={.subsets[*].addresses[*].ip}") | ||
| output, err = utils.Run(cmd) | ||
| g.Expect(err).NotTo(HaveOccurred()) | ||
| g.Expect(output).NotTo(BeEmpty(), "Webhook service endpoints are not ready") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should avoid use endpoints. |
||
|
|
||
| // Test webhook connectivity by checking if webhook server port is responding | ||
| cmd = exec.Command("kubectl", "run", "webhook-test", "--rm", "-i", "--restart=Never", | ||
| "--image=curlimages/curl:latest", "--", | ||
| "curl", "-k", "--connect-timeout", "5", | ||
| "https://"+webhookServiceName+"."+namespace+".svc:443/readyz") | ||
| _, err = utils.Run(cmd) | ||
| g.Expect(err).NotTo(HaveOccurred(), "Webhook server not responding on port 443") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
| Eventually(verifyWebhookServiceReady, 2*time.Minute).Should(Succeed()) | ||
| // +kubebuilder:scaffold:e2e-webhooks-readiness | ||
|
|
||
| By("creating the curl-metrics pod to access the metrics endpoint") | ||
| cmd = exec.Command("kubectl", "run", "curl-metrics", "--restart=Never", | ||
| "--namespace", namespace, | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -22,6 +22,7 @@ import ( | |||||||||
| log "log/slog" | ||||||||||
| "os" | ||||||||||
| "path/filepath" | ||||||||||
| "strings" | ||||||||||
|
|
||||||||||
| "sigs.k8s.io/kubebuilder/v4/pkg/machinery" | ||||||||||
| ) | ||||||||||
|
|
@@ -31,7 +32,10 @@ var ( | |||||||||
| _ machinery.Inserter = &WebhookTestUpdater{} | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| const webhookChecksMarker = "e2e-webhooks-checks" | ||||||||||
| const ( | ||||||||||
| webhookChecksMarker = "e2e-webhooks-checks" | ||||||||||
| webhookReadinessMarker = "e2e-webhooks-readiness" | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| // Test defines the basic setup for the e2e test | ||||||||||
| type Test struct { | ||||||||||
|
|
@@ -74,6 +78,7 @@ func (*WebhookTestUpdater) GetIfExistsAction() machinery.IfExistsAction { | |||||||||
| // GetMarkers implements file.Inserter | ||||||||||
| func (f *WebhookTestUpdater) GetMarkers() []machinery.Marker { | ||||||||||
| return []machinery.Marker{ | ||||||||||
| machinery.NewMarkerFor(f.GetPath(), webhookReadinessMarker), | ||||||||||
| machinery.NewMarkerFor(f.GetPath(), webhookChecksMarker), | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
@@ -107,25 +112,35 @@ func (f *WebhookTestUpdater) GetCodeFragments() machinery.CodeFragmentsMap { | |||||||||
| } | ||||||||||
|
|
||||||||||
| var fragments []string | ||||||||||
| fragments = append(fragments, webhookChecksFragment) | ||||||||||
|
|
||||||||||
| if f.Resource != nil && f.Resource.HasDefaultingWebhook() { | ||||||||||
| mutatingWebhookCode := fmt.Sprintf(mutatingWebhookChecksFragment, f.ProjectName) | ||||||||||
| fragments = append(fragments, mutatingWebhookCode) | ||||||||||
| } | ||||||||||
| // Handle different markers differently | ||||||||||
| markerString := marker.String() | ||||||||||
| if strings.Contains(markerString, webhookReadinessMarker) { | ||||||||||
| // Only inject readiness check for this marker | ||||||||||
| webhookReadinessCode := fmt.Sprintf(webhookReadinessFragment, f.ProjectName) | ||||||||||
| fragments = append(fragments, webhookReadinessCode) | ||||||||||
| } else if strings.Contains(markerString, webhookChecksMarker) { | ||||||||||
| // Inject all other webhook tests for this marker | ||||||||||
| fragments = append(fragments, webhookChecksFragment) | ||||||||||
|
|
||||||||||
| if f.Resource != nil && f.Resource.HasDefaultingWebhook() { | ||||||||||
| mutatingWebhookCode := fmt.Sprintf(mutatingWebhookChecksFragment, f.ProjectName) | ||||||||||
| fragments = append(fragments, mutatingWebhookCode) | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if f.Resource != nil && f.Resource.HasValidationWebhook() { | ||||||||||
| validatingWebhookCode := fmt.Sprintf(validatingWebhookChecksFragment, f.ProjectName) | ||||||||||
| fragments = append(fragments, validatingWebhookCode) | ||||||||||
| } | ||||||||||
| if f.Resource != nil && f.Resource.HasValidationWebhook() { | ||||||||||
| validatingWebhookCode := fmt.Sprintf(validatingWebhookChecksFragment, f.ProjectName) | ||||||||||
| fragments = append(fragments, validatingWebhookCode) | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if f.Resource != nil && f.Resource.HasConversionWebhook() { | ||||||||||
| conversionWebhookCode := fmt.Sprintf( | ||||||||||
| conversionWebhookChecksFragment, | ||||||||||
| f.Resource.Kind, | ||||||||||
| f.Resource.Plural+"."+f.Resource.Group+"."+f.Resource.Domain, | ||||||||||
| ) | ||||||||||
| fragments = append(fragments, conversionWebhookCode) | ||||||||||
| if f.Resource != nil && f.Resource.HasConversionWebhook() { | ||||||||||
| conversionWebhookCode := fmt.Sprintf( | ||||||||||
| conversionWebhookChecksFragment, | ||||||||||
| f.Resource.Kind, | ||||||||||
| f.Resource.Plural+"."+f.Resource.Group+"."+f.Resource.Domain, | ||||||||||
| ) | ||||||||||
| fragments = append(fragments, conversionWebhookCode) | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| codeFragments[marker] = fragments | ||||||||||
|
|
@@ -198,6 +213,41 @@ const conversionWebhookChecksFragment = `It("should have CA injection for %[1]s | |||||||||
|
|
||||||||||
| ` | ||||||||||
|
|
||||||||||
| const webhookReadinessFragment = `By("waiting for webhook service to be ready") | ||||||||||
| verifyWebhookServiceReady := func(g Gomega) { | ||||||||||
| const webhookServiceName = "%s-webhook-service" | ||||||||||
|
|
||||||||||
| // Webhook service should exist since webhooks are configured | ||||||||||
| cmd := exec.Command("kubectl", "get", "service", webhookServiceName, "-n", namespace) | ||||||||||
| _, err := utils.Run(cmd) | ||||||||||
| g.Expect(err).NotTo(HaveOccurred(), "Webhook service should exist but was not found") | ||||||||||
|
|
||||||||||
| // Check if webhook server is ready by verifying pod readiness | ||||||||||
| cmd = exec.Command("kubectl", "get", "pods", "-l", "control-plane=controller-manager", | ||||||||||
| "-n", namespace, "-o", "jsonpath={.items[0].status.conditions[?(@.type=='Ready')].status}") | ||||||||||
| output, err := utils.Run(cmd) | ||||||||||
| g.Expect(err).NotTo(HaveOccurred()) | ||||||||||
| g.Expect(output).To(Equal("True"), | ||||||||||
|
||||||||||
| g.Expect(output).To(Equal("True"), | |
| g.Expect(output).To(Equal("True"), |
Copilot
AI
Nov 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing whitespace detected at the end of this line. This should be removed for consistency with code style standards.
| cmd = exec.Command("kubectl", "get", "endpoints", webhookServiceName, | |
| cmd = exec.Command("kubectl", "get", "endpoints", webhookServiceName, |
Copilot
AI
Nov 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardcoded pod name "webhook-test" could cause conflicts when Eventually retries this function. If a previous attempt fails after the pod is created but before it's deleted (e.g., due to curl failure), subsequent attempts will fail with "pod already exists" errors. Consider using a unique name with a timestamp suffix or using --generate-name flag instead.
| cmd = exec.Command("kubectl", "run", "webhook-test", "--rm", "-i", "--restart=Never", | |
| "--image=curlimages/curl:latest", "--", | |
| cmd = exec.Command("kubectl", "run", "--rm", "-i", "--restart=Never", | |
| "--generate-name=webhook-test-", "--image=curlimages/curl:latest", "--", |
Copilot
AI
Nov 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing whitespace detected at the end of this line. This should be removed for consistency with code style standards.
| "curl", "-k", "--connect-timeout", "5", | |
| "curl", "-k", "--connect-timeout", "5", |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -211,6 +211,41 @@ var _ = Describe("Manager", Ordered, func() { | |
| } | ||
| Eventually(verifyMetricsServerStarted).Should(Succeed()) | ||
|
|
||
| By("waiting for webhook service to be ready") | ||
| verifyWebhookServiceReady := func(g Gomega) { | ||
| const webhookServiceName = "project-v4-multigroup-webhook-service" | ||
|
|
||
| // Webhook service should exist since webhooks are configured | ||
| cmd := exec.Command("kubectl", "get", "service", webhookServiceName, "-n", namespace) | ||
| _, err := utils.Run(cmd) | ||
| g.Expect(err).NotTo(HaveOccurred(), "Webhook service should exist but was not found") | ||
|
|
||
| // Check if webhook server is ready by verifying pod readiness | ||
| cmd = exec.Command("kubectl", "get", "pods", "-l", "control-plane=controller-manager", | ||
| "-n", namespace, "-o", "jsonpath={.items[0].status.conditions[?(@.type=='Ready')].status}") | ||
| output, err := utils.Run(cmd) | ||
| g.Expect(err).NotTo(HaveOccurred()) | ||
| g.Expect(output).To(Equal("True"), | ||
| "Controller manager pod not ready (webhook server may not be accepting connections)") | ||
|
|
||
| // Check if webhook service endpoints are available | ||
| cmd = exec.Command("kubectl", "get", "endpoints", webhookServiceName, | ||
| "-n", namespace, "-o", "jsonpath={.subsets[*].addresses[*].ip}") | ||
| output, err = utils.Run(cmd) | ||
| g.Expect(err).NotTo(HaveOccurred()) | ||
| g.Expect(output).NotTo(BeEmpty(), "Webhook service endpoints are not ready") | ||
|
|
||
| // Test webhook connectivity by checking if webhook server port is responding | ||
| cmd = exec.Command("kubectl", "run", "webhook-test", "--rm", "-i", "--restart=Never", | ||
| "--image=curlimages/curl:latest", "--", | ||
| "curl", "-k", "--connect-timeout", "5", | ||
| "https://"+webhookServiceName+"."+namespace+".svc:443/readyz") | ||
| _, err = utils.Run(cmd) | ||
| g.Expect(err).NotTo(HaveOccurred(), "Webhook server not responding on port 443") | ||
| } | ||
| Eventually(verifyWebhookServiceReady, 2*time.Minute).Should(Succeed()) | ||
| // +kubebuilder:scaffold:e2e-webhooks-readiness | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we may can simplify with By("waiting for webhook service to be ready if webhooks are configured")
verifyWebhookServiceReady := func(g Gomega) {
const webhookServiceName = "{{ProjectName}}-webhook-service"
cmd := exec.Command("kubectl", "get", "service", webhookServiceName, "-n", namespace)
_, err := utils.Run(cmd)
if err != nil {
// Project has no webhook service; nothing to wait on.
return
}
cmd = exec.Command("kubectl", "get", "pods", "-l", "control-plane=controller-manager",
"-n", namespace, "-o", "jsonpath={.items[0].status.conditions[?(@.type=='Ready')].status}")
output, err := utils.Run(cmd)
g.Expect(err).NotTo(HaveOccurred())
g.Expect(output).To(Equal("True"),
"Controller manager pod not ready (webhook server may not be accepting connections)")
cmd = exec.Command("kubectl", "get", "endpoints", webhookServiceName,
"-n", namespace, "-o", "jsonpath={.subsets[*].addresses[*].ip}")
output, err = utils.Run(cmd)
g.Expect(err).NotTo(HaveOccurred())
g.Expect(output).NotTo(BeEmpty(), "Webhook service endpoints are not ready")
cmd = exec.Command("kubectl", "get", "--raw",
fmt.Sprintf("/api/v1/namespaces/%s/services/https:%[2]s:https/proxy/readyz", namespace, webhookServiceName))
_, err = utils.Run(cmd)
g.Expect(err).NotTo(HaveOccurred(), "Webhook server not responding on port 443")
}
Eventually(verifyWebhookServiceReady, 2*time.Minute).Should(Succeed())
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WDYT>
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks you, this looks Perfect :) Regarding this I have slightly modified. Since now we have the marker which runs exclusively for webhooks, instead of ignoring the error, will be failing if service is not found. to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Regarding this Looks like it is failing after using the service proxy to check for readiness here: https://github.com/mayuka-c/kubebuilder/actions/runs/18962257859/job/54151953303. I will see on what could be the reason for this. Please do let me know if the endpoint used is wrong. Thanks!
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we use a pod with curl to do so is fine but we need to cleanup
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately I reverted to use curl, it does remove the pod once it finishes with
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to ensure that all that we create is cleaned. Well done for sure. |
||
|
|
||
| By("creating the curl-metrics pod to access the metrics endpoint") | ||
| cmd = exec.Command("kubectl", "run", "curl-metrics", "--restart=Never", | ||
| "--namespace", namespace, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could use
--for=condition=Ready