Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Feature] Introduce TAS e2e tests with dedicated infra #3489

Merged
merged 5 commits into from
Nov 14, 2024
Merged

[Feature] Introduce TAS e2e tests with dedicated infra #3489

merged 5 commits into from
Nov 14, 2024
+433 −6

Conversation

mszadkow
Copy link
Contributor

@mszadkow mszadkow commented Nov 8, 2024 •
edited
Loading

What type of PR is this?

/kind feature

What this PR does / why we need it:

Add e2e test Job which will have larger structure (multiple blocks and racks) to test TAS more throughly.

Which issue(s) this PR fixes:

Relates to #3480

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

@k8s-ci-robot
Copy link
Contributor

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 k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 8, 2024
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Nov 8, 2024
@netlify Netlify
Copy link

netlify bot commented Nov 8, 2024 •
edited
Loading

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name Link
🔨 Latest commit db1f957
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/6735f092815bf30008d67212

@mszadkow mszadkow changed the title [Feature] Introduce TAS e2e tests with dedicated infra WIP!!!!! [Feature] Introduce TAS e2e tests with dedicated infra Nov 8, 2024
@mszadkow mszadkow force-pushed the feature/tas-extend-e2e-tests branch 2 times, most recently from cace5c2 to 90f6299 Compare November 12, 2024 09:13
@k8s-ci-robot k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 12, 2024
@mszadkow mszadkow marked this pull request as ready for review November 12, 2024 09:23
@mszadkow mszadkow changed the title WIP!!!!! [Feature] Introduce TAS e2e tests with dedicated infra [Feature] Introduce TAS e2e tests with dedicated infra Nov 12, 2024
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Nov 12, 2024
mbobrovskyi
mbobrovskyi suggested changes Nov 12, 2024
View reviewed changes
test/e2e/tas/util.go Outdated Show resolved Hide resolved
test/e2e/tas/suite_test.go Outdated Show resolved Hide resolved
Makefile-test.mk Show resolved Hide resolved
test/e2e/tas/suite_test.go Outdated Show resolved Hide resolved
test/e2e/tas/suite_test.go Outdated Show resolved Hide resolved
Makefile-test.mk Show resolved Hide resolved
mbobrovskyi
mbobrovskyi reviewed Nov 12, 2024
View reviewed changes
Copy link
Contributor

@mbobrovskyi mbobrovskyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also please run test-tas-e2e on test-e2e for now.

mbobrovskyi
mbobrovskyi reviewed Nov 12, 2024
View reviewed changes
test/e2e/tas/tas_test.go Outdated Show resolved Hide resolved
test/e2e/tas/tas_test.go Outdated Show resolved Hide resolved
@mszadkow
Copy link
Contributor Author

Also please run test-tas-e2e on test-e2e for now.

So when should it become a separate target?

@mbobrovskyi
Copy link
Contributor

Also please run test-tas-e2e on test-e2e for now.

So when should it become a separate target?

After update test-infra. But I think we should merge it to main first, as other PRs will fail without the make command.

@k8s-ci-robot k8s-ci-robot removed the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 13, 2024
mimowo
mimowo reviewed Nov 14, 2024
View reviewed changes
test/e2e/tas/tas_test.go Outdated Show resolved Hide resolved
mimowo
mimowo reviewed Nov 14, 2024
View reviewed changes
test/e2e/tas/tas_test.go Outdated Show resolved Hide resolved
@mszadkow
Copy link
Contributor Author

Also please run test-tas-e2e on test-e2e for now.

So when should it become a separate target?

After update test-infra. But I think we should merge it to main first, as other PRs will fail without the make command.

Then I should probably have the test-tas-e2e as separate target too?
The moment new test-infra target runs test-tas-e2e, we will be able to remove run-test-tas-e2e- from test-e2e.
wdyt?

@mimowo
Copy link
Contributor

mimowo commented Nov 14, 2024

Having a new e2e target sgtm, I think this is what we did for multikueue during the transition period to the dedicated job.

@mbobrovskyi mbobrovskyi mentioned this pull request Nov 14, 2024
Merged
mbobrovskyi
mbobrovskyi reviewed Nov 14, 2024
View reviewed changes
Copy link
Contributor

@mbobrovskyi mbobrovskyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

Thanks!

test/e2e/tas/suite_test.go Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 14, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 4e150fc78fa239b2f1c5f9f34a6e99c12f91b7fc

mimowo
mimowo reviewed Nov 14, 2024
View reviewed changes
Copy link
Contributor

@mimowo mimowo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just nits

test/e2e/tas/tas_test.go Outdated Show resolved Hide resolved
test/e2e/tas/tas_test.go Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 14, 2024
@mbobrovskyi
Copy link
Contributor

mbobrovskyi commented Nov 14, 2024 •
edited
Loading

/lgtm
Thanks!

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 14, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 3727f7b3f146938bf525f540c5814cf92374a231

@mimowo
Copy link
Contributor

mimowo commented Nov 14, 2024

/approve
Awesome, hope we can get it used more over time to test trickier scenarios. But for now this is a great starting point
cc @PBundyra

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mimowo, mszadkow

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 14, 2024
@k8s-ci-robot k8s-ci-robot merged commit 9624ed8 into kubernetes-sigs:main Nov 14, 2024
16 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v0.10 milestone Nov 14, 2024
@mbobrovskyi mbobrovskyi deleted the feature/tas-extend-e2e-tests branch November 14, 2024 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mimowo mimowo mimowo left review comments

@tenzen-y tenzen-y Awaiting requested review from tenzen-y

@denkensk denkensk Awaiting requested review from denkensk

@mbobrovskyi mbobrovskyi Awaiting requested review from mbobrovskyi

Assignees

@mbobrovskyi mbobrovskyi

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v0.10
Development

Successfully merging this pull request may close these issues.
4 participants
@mszadkow @k8s-ci-robot @mbobrovskyi @mimowo