🐛 Fix flakes in e2e tests for metrics

[mayuka-c]

Fixes: #5137

By looking at the logs, looks like during the curl metrics pod creation, calls to the webhook is failing due to connection refused. So added precheck to ensure the webhook is ready before beginning with the pod creation.

Validation

Ran the e2e tests multiple times here and did not see the flakes: https://github.com/mayuka-c/kubebuilder/actions/runs/18582517449

[k8s-ci-robot]

Hi @mayuka-c. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mayuka-c
Once this PR has been reviewed and has the lgtm label, please assign camilamacedo86 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[camilamacedo86]

Hi @mayuka-c 👋

That looks great! However, this change only makes sense for projects that actually have webhooks.
Not all projects do — we use markers to inject code (see Kubebuilder markers
).
For example:

+kubebuilder:scaffold:e2e-webhooks-checks

Then, if we really need that we will need to see if we can use the same; it seems not. Then we need a new marker. So, we need to be 1000% sure that is the best way. Ideally we should avoid create new markers and etc since we need keep the support for those, maker harder for end user customisartions, etc.

So, if we need to check whether the webhook is serving before the metrics, we should only add this logic when webhooks are present — meaning we’ll need to use a marker for it.

Also, please make sure the scaffolded code remains as generic as possible, so it still works even if users customise their configurations.

Lastly, since this change affects the scaffolded e2e tests in projects (which impact end users), we need to be careful with the commit type.
We should use the 🐛 emoji instead of 🌱 — the seed emoji is only used for changes that don’t affect users and can be ignored in the release notes.

[mayuka-c]

Hi @camilamacedo86
Regarding below:-

That looks great! However, this change only makes sense for projects that actually have webhooks. Not all projects do — we use markers to inject code (see Kubebuilder markers ). For example:

+kubebuilder:scaffold:e2e-webhooks-checks

Then, if we really need that we will need to see if we can use the same; it seems not. Then we need a new marker. So, we need to be 1000% sure that is the best way. Ideally we should avoid create new markers and etc since we need keep the support for those, maker harder for end user customisartions, etc.

So, if we need to check whether the webhook is serving before the metrics, we should only add this logic when webhooks are present — meaning we’ll need to use a marker for it.

Also, please make sure the scaffolded code remains as generic as possible, so it still works even if users customise their configurations.

If I'm not wrong, I could add a new fragment to do the webhook readiness check here and replace with the marker here

But the only problem that I see is, it will end up running other fragment checks too which is not required.

Shall I go ahead to define a new marker? or if you have any better solution, please do suggest :)

[mayuka-c]

Lastly, since this change affects the scaffolded e2e tests in projects (which impact end users), we need to be careful with the commit type. We should use the 🐛 emoji instead of 🌱 — the seed emoji is only used for changes that don’t affect users and can be ignored in the release notes.

Sorry my bad, I will update the title with proper emoji. Thank you :)

[camilamacedo86]

Hi @mayuka-c

Let's create a new marker
Also, if you can review the code and ensure that is generic and good enough for any project that would be great 🎉

[mayuka-c]

Hi @mayuka-c

Let's create a new marker Also, if you can review the code and ensure that is generic and good enough for any project that would be great 🎉

Sure thank you @camilamacedo86 . I will review the code and make sure it is generic.

[mayuka-c]

Hi @camilamacedo86 👋
Sorry for getting back late, as I was bust with other things.

I have added a new marker e2e-webhook-readiness and it seems to working fine.
I see the readiness check test cases are used in projects where the webhooks are present in Projectfile and for the basic test-data project, the webhook tests are not added.

Please let me know if this is fine or if I'm missing

[camilamacedo86]

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())

[camilamacedo86]

WDYT>

[mayuka-c]

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.

	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
	}

to

      const webhookServiceName = "{{.ProjectName}}-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")

[mayuka-c]

Regarding this

	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")

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!