[kueue] Wait for webhooks setup before ready. #1674

trasc · 2024-01-30T12:19:58Z

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

k8s-ci-robot · 2024-01-30T12:20:00Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

k8s-ci-robot · 2024-01-30T12:20:00Z

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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/test-infra repository.

k8s-ci-robot · 2024-01-30T12:20:03Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: trasc
Once this PR has been reviewed and has the lgtm label, please assign tenzen-y for approval. For more information see the Kubernetes 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

netlify · 2024-01-30T12:20:03Z

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	`b702d0d`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/65b9f6c83834440008250110

trasc · 2024-01-30T12:21:08Z

/test pull-kueue-test-e2e-main-1-27
/test pull-kueue-test-e2e-main-1-28
/test pull-kueue-test-e2e-main-1-29

trasc · 2024-01-30T12:34:19Z

try 2.

/test pull-kueue-test-e2e-main-1-27
/test pull-kueue-test-e2e-main-1-28
/test pull-kueue-test-e2e-main-1-29

trasc · 2024-01-30T12:44:50Z

/test pull-kueue-test-e2e-main-1-27
/test pull-kueue-test-e2e-main-1-28
/test pull-kueue-test-e2e-main-1-29

trasc · 2024-01-30T12:57:09Z

/test pull-kueue-test-e2e-main-1-27
/test pull-kueue-test-e2e-main-1-28
/test pull-kueue-test-e2e-main-1-29

trasc · 2024-01-30T13:07:31Z

/test pull-kueue-test-e2e-main-1-27
/test pull-kueue-test-e2e-main-1-28
/test pull-kueue-test-e2e-main-1-29

trasc · 2024-01-30T13:23:15Z

/test pull-kueue-test-e2e-main-1-26
/test pull-kueue-test-e2e-main-1-27
/test pull-kueue-test-e2e-main-1-28
/test pull-kueue-test-e2e-main-1-29

trasc · 2024-01-30T13:37:05Z

/test pull-kueue-test-e2e-main-1-26
/test pull-kueue-test-e2e-main-1-27
/test pull-kueue-test-e2e-main-1-28
/test pull-kueue-test-e2e-main-1-29

mimowo · 2024-01-30T13:40:12Z

test/util/e2e.go

@@ -65,11 +65,19 @@ func CreateVisibilityClient(user string) visibilityv1alpha1.VisibilityV1alpha1In
 	return visibilityClient
 }

-func KueueReadyForTesting(ctx context.Context, client client.Client) {
+func KueueReadyForTesting(ctx context.Context, c client.Client) {


Can we delete this function if we are waiting for the "webooks setup before ready"?

no, this is running in the test binary

but should not the test binary wait until Kueue is healthy / ready, rather than trying to periodically create the items? If we still periodically need to create items I don't seem much gain of delaying the healthy status.

It's just another way of checking the same thing.

I theory you can check if all the kueue-controller-manager pods are ready instead.

I see, but this involves creating / deleting objects, something which users may not want to do in prod. I think it is better if e2e tests correspond better to production use. Also, the creation / deletion takes more time some may cover some user issues. If we go with this approach I would suggest to wait as for ready deployment, this can be done as here:

kubectl wait --for=condition=available --timeout=3m deployment/kueue-controller-manager -n kueue-system

trasc · 2024-01-30T13:54:47Z

/test pull-kueue-test-e2e-main-1-26
/test pull-kueue-test-e2e-main-1-27
/test pull-kueue-test-e2e-main-1-28
/test pull-kueue-test-e2e-main-1-29

trasc · 2024-01-30T14:21:40Z

/test pull-kueue-test-e2e-main-1-26
/test pull-kueue-test-e2e-main-1-27
/test pull-kueue-test-e2e-main-1-28
/test pull-kueue-test-e2e-main-1-29

trasc · 2024-01-30T14:52:58Z

/test pull-kueue-test-e2e-main-1-26
/test pull-kueue-test-e2e-main-1-27
/test pull-kueue-test-e2e-main-1-28
/test pull-kueue-test-e2e-main-1-29

trasc · 2024-01-30T15:13:04Z

/test pull-kueue-test-e2e-main-1-26
/test pull-kueue-test-e2e-main-1-27
/test pull-kueue-test-e2e-main-1-28
/test pull-kueue-test-e2e-main-1-29

trasc · 2024-01-30T15:29:30Z

/test pull-kueue-test-e2e-main-1-26
/test pull-kueue-test-e2e-main-1-27
/test pull-kueue-test-e2e-main-1-28
/test pull-kueue-test-e2e-main-1-29

trasc · 2024-01-30T15:49:12Z

/test pull-kueue-test-e2e-main-1-26
/test pull-kueue-test-e2e-main-1-27
/test pull-kueue-test-e2e-main-1-28
/test pull-kueue-test-e2e-main-1-29

trasc · 2024-01-30T16:07:49Z

/test pull-kueue-test-e2e-main-1-26
/test pull-kueue-test-e2e-main-1-27
/test pull-kueue-test-e2e-main-1-28
/test pull-kueue-test-e2e-main-1-29

trasc · 2024-01-30T16:20:39Z

/test pull-kueue-test-e2e-main-1-26
/test pull-kueue-test-e2e-main-1-27
/test pull-kueue-test-e2e-main-1-28
/test pull-kueue-test-e2e-main-1-29

trasc · 2024-01-30T16:33:22Z

/test pull-kueue-test-e2e-main-1-26
/test pull-kueue-test-e2e-main-1-27
/test pull-kueue-test-e2e-main-1-28
/test pull-kueue-test-e2e-main-1-29

alculquicondor · 2024-01-30T22:04:36Z

/assign @astefanutti

k8s-ci-robot · 2024-01-30T22:04:38Z

@alculquicondor: GitHub didn't allow me to assign the following users: astefanutti.

Note that only kubernetes-sigs members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @astefanutti

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/test-infra repository.

alculquicondor · 2024-01-30T22:08:44Z

A failure happened https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_kueue/1674/pull-kueue-test-e2e-main-1-28/1752369431784198144 :(

The webhook never became available.

alculquicondor · 2024-01-30T22:07:07Z

test/util/e2e.go

 	}, StartUpTimeout, Interval).Should(gomega.Succeed())
-	ExpectResourceFlavorToBeDeleted(ctx, client, resourceKueue, true)


why change this?

the reporting is hard to follow with that, in case of multikueue you don't know which of the clusters is not ready.

trasc · 2024-01-31T05:03:37Z

/test pull-kueue-test-e2e-main-1-26
/test pull-kueue-test-e2e-main-1-27
/test pull-kueue-test-e2e-main-1-28
/test pull-kueue-test-e2e-main-1-29

trasc · 2024-01-31T06:46:04Z

/test pull-kueue-test-e2e-main-1-26
/test pull-kueue-test-e2e-main-1-27
/test pull-kueue-test-e2e-main-1-28
/test pull-kueue-test-e2e-main-1-29

trasc · 2024-01-31T06:57:16Z

/test pull-kueue-test-e2e-main-1-26
/test pull-kueue-test-e2e-main-1-27
/test pull-kueue-test-e2e-main-1-28
/test pull-kueue-test-e2e-main-1-29

trasc · 2024-01-31T07:29:27Z

/test pull-kueue-test-e2e-main-1-26
/test pull-kueue-test-e2e-main-1-27
/test pull-kueue-test-e2e-main-1-28
/test pull-kueue-test-e2e-main-1-29

trasc · 2024-01-31T08:56:51Z

/test pull-kueue-test-e2e-main-1-26
/test pull-kueue-test-e2e-main-1-27
/test pull-kueue-test-e2e-main-1-28
/test pull-kueue-test-e2e-main-1-29

k8s-ci-robot · 2024-01-31T09:02:26Z

@trasc: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-kueue-test-e2e-main-1-27	`b702d0d`	link	true	`/test pull-kueue-test-e2e-main-1-27`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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/test-infra repository. I understand the commands that are listed here.

trasc · 2024-01-31T12:50:26Z

/close
in favor of #1676

k8s-ci-robot · 2024-01-31T12:50:31Z

@trasc: Closed this PR.

In response to this:

/close
in favor of #1676

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/test-infra repository.

[kueue] Wait for webhooks setup before ready.

25dd4bf

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jan 30, 2024

k8s-ci-robot requested review from denkensk and mimowo January 30, 2024 12:20

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 30, 2024

trasc mentioned this pull request Jan 30, 2024

[e2e/multukueue] Retry creating test setup objects #1659

Closed

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 30, 2024

trasc force-pushed the ready-after-weebhooks-start branch from a0454ba to 387a10b Compare January 30, 2024 13:22

mimowo reviewed Jan 30, 2024

View reviewed changes

mimowo mentioned this pull request Jan 30, 2024

Flaky Multikueue E2E tests #1658

Closed

trasc force-pushed the ready-after-weebhooks-start branch from 387a10b to 1031a49 Compare January 30, 2024 14:48

alculquicondor reviewed Jan 30, 2024

View reviewed changes

trasc force-pushed the ready-after-weebhooks-start branch from 1031a49 to b10f549 Compare January 31, 2024 05:01

trasc added 2 commits January 31, 2024 08:51

[e2e/multikueue]Increase logging level on k8s componenets

23e5736

[util/e2e] Rework KueueReadyForTesting.

7f66d18

trasc force-pushed the ready-after-weebhooks-start branch from 974b886 to 7f66d18 Compare January 31, 2024 06:57

[debug] Increase api_server logging

b702d0d

k8s-ci-robot closed this Jan 31, 2024

trasc mentioned this pull request Feb 5, 2024

Fleky multikueue E2E tests #1663

Closed

trasc deleted the ready-after-weebhooks-start branch February 6, 2024 04:51

		}, StartUpTimeout, Interval).Should(gomega.Succeed())
		ExpectResourceFlavorToBeDeleted(ctx, client, resourceKueue, true)

[kueue] Wait for webhooks setup before ready. #1674

[kueue] Wait for webhooks setup before ready. #1674

Conversation

trasc commented Jan 30, 2024

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

k8s-ci-robot commented Jan 30, 2024

k8s-ci-robot commented Jan 30, 2024

k8s-ci-robot commented Jan 30, 2024

netlify bot commented Jan 30, 2024 • edited Loading

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

trasc commented Jan 30, 2024

trasc commented Jan 30, 2024

trasc commented Jan 30, 2024

trasc commented Jan 30, 2024

trasc commented Jan 30, 2024

trasc commented Jan 30, 2024

trasc commented Jan 30, 2024

mimowo Jan 30, 2024

Choose a reason for hiding this comment

trasc Jan 30, 2024

Choose a reason for hiding this comment

mimowo Jan 30, 2024

Choose a reason for hiding this comment

trasc Jan 30, 2024

Choose a reason for hiding this comment

mimowo Jan 31, 2024

Choose a reason for hiding this comment

trasc commented Jan 30, 2024

trasc commented Jan 30, 2024

trasc commented Jan 30, 2024

trasc commented Jan 30, 2024

trasc commented Jan 30, 2024

trasc commented Jan 30, 2024

trasc commented Jan 30, 2024

trasc commented Jan 30, 2024

trasc commented Jan 30, 2024

alculquicondor commented Jan 30, 2024

k8s-ci-robot commented Jan 30, 2024

alculquicondor commented Jan 30, 2024

alculquicondor Jan 30, 2024

Choose a reason for hiding this comment

trasc Jan 31, 2024

Choose a reason for hiding this comment

trasc commented Jan 31, 2024

trasc commented Jan 31, 2024

trasc commented Jan 31, 2024

trasc commented Jan 31, 2024

trasc commented Jan 31, 2024

k8s-ci-robot commented Jan 31, 2024

trasc commented Jan 31, 2024

k8s-ci-robot commented Jan 31, 2024

netlify bot commented Jan 30, 2024 •

edited

Loading