Skip to content

E2E Test #56

Merged
k8s-ci-robot merged 6 commits intokubernetes-sigs:mainfrom
kannon92:setup-e2e-test-github-action
Sep 5, 2024
Merged

E2E Test #56
k8s-ci-robot merged 6 commits intokubernetes-sigs:mainfrom
kannon92:setup-e2e-test-github-action

Conversation

@kannon92
Copy link
Contributor

@kannon92 kannon92 commented Aug 30, 2024

I am hoping to at least build the driver, create kind and maybe run a few of those pods as a e2e test.

Surprising this works. I don’t really understand where mine came from in the GitHub runner but it is found.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 30, 2024
@k8s-ci-robot k8s-ci-robot requested review from bart0sh and pohly August 30, 2024 19:02
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 30, 2024
@kannon92 kannon92 mentioned this pull request Aug 30, 2024
@kannon92 kannon92 force-pushed the setup-e2e-test-github-action branch from 7430725 to 5a8343b Compare August 30, 2024 20:14
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 30, 2024
@kannon92 kannon92 force-pushed the setup-e2e-test-github-action branch 4 times, most recently from 800711c to 81c8577 Compare August 30, 2024 21:11
@kannon92 kannon92 force-pushed the setup-e2e-test-github-action branch from 81c8577 to 5b73e06 Compare August 30, 2024 21:25
@kannon92 kannon92 changed the title WIP: E2E Test E2E Test Aug 30, 2024
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 30, 2024
Copy link
Contributor

@byako byako left a comment

Choose a reason for hiding this comment

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

Fail fast, no need to proceed the whole flow after a failure.

@kannon92 kannon92 force-pushed the setup-e2e-test-github-action branch from a3562d4 to d86fdbd Compare September 2, 2024 13:43
run: go mod vendor
- name: Build
run: make PREFIX=artifacts cmds
- name: install helm and kubelet
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: install helm and kubelet
- name: install helm and kubectl

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we not need to also install kind?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Surprisingly not.

Comment on lines +26 to +30
gpu_test_1=$(kubectl get pods -n gpu-test1 | grep -c 'Running')
if [ $gpu_test_1 != 2 ]; then
echo "gpu_test_1 $gpu_test_1 failed to match against 2 expected pods"
exit 1
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also check for the existence of GPU_DEVICE_* envvars in each test. Its possible for these to enter the running state even if CDI didn't do its thing behind the scenes. This can happen if CDI is not enabled in the underlying container runtime, or the DynamicResourceAllocation feature gate was not enabled on the Kubelet somehow.

@kannon92
Copy link
Contributor Author

kannon92 commented Sep 3, 2024

/hold

Test seems flaky.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 3, 2024
@kannon92 kannon92 force-pushed the setup-e2e-test-github-action branch from c3f20b4 to 34fe3d2 Compare September 3, 2024 19:49
@kannon92
Copy link
Contributor Author

kannon92 commented Sep 3, 2024

/hold cancel

added kubectl waits on the pods which may help with flakiness.

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 3, 2024
@bart0sh
Copy link
Contributor

bart0sh commented Sep 4, 2024

/lgtm

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

bart0sh commented Sep 4, 2024

/assign @klueska
for approval

@kannon92
Copy link
Contributor Author

kannon92 commented Sep 5, 2024

Last outstanding item is #56 (comment). We can resolve this or we can do it as a follow up. I don't care either way. I'll maybe find some time next week to finish that up.

@klueska
Copy link
Contributor

klueska commented Sep 5, 2024

Let's merge as-is for now since running something is better than running nothing. But let's definitely follow-up on that.

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kannon92, klueska

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

The pull request process is described here

Details 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 Sep 5, 2024
@k8s-ci-robot k8s-ci-robot merged commit abc52cf into kubernetes-sigs:main Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants