quarkus-qe / quarkus-test-framework

Write your test once and run it everywhere!
Apache License 2.0
14 stars 26 forks source link

Add Kubernetes jobs to GH Actions workflow #1174

Closed gtroitsk closed 1 week ago

gtroitsk commented 2 weeks ago

Summary

Closes https://github.com/quarkus-qe/quarkus-test-framework/issues/433 Adds jobs to run Kubernetes tests in JVM and Native modes for both PR and daily CI runs

Please check the relevant options

Checklist:

github-actions[bot] commented 2 weeks ago

Following jobs contain at least one flaky test: 'Linux - JVM build - Latest Version'

rsvoboda commented 1 week ago

Imho Kubernetes runs should be independent on the existing ones. It's more convenient to have the runs isolated - better for investigation and status overview. kubernetes-build-native-latest from .github/workflows/daily.yaml could be updated.

@michalvavrik @mjurc what's your take on this?

mjurc commented 1 week ago

Agreed, it should be standalone

michalvavrik commented 1 week ago

Agreed, it should be standalone

+1

michalvavrik commented 1 week ago

run tests

gtroitsk commented 1 week ago

run tests

If you want to compare results with Jenkins k8s runs, it will not work. I deleted all k8s clusters

michalvavrik commented 1 week ago

run tests

If you want to compare results with Jenkins k8s runs, it will not work. I delete all k8s clusters

The quarkus-test-kubernetes module is part of several examples test deps and I want to see that presence of the kubernetes-httpclient-vertx does not affect OCP tests.

michalvavrik commented 1 week ago

@rsvoboda @mjurc in this PR CI workflow, K8 tests are part of the:

It took me while to figure it's there, but I suppose based on the failed test name and part of the workflow you know it's there.. Is this ok with you?

Personally I'd prefer it separately.

P.S.: I'll open PR for "daily", it doesn't make sense.

michalvavrik commented 1 week ago

On the other hand, I wonder if we shouldn't run it as part of the native tests instead of separately. Because we are loosing time in the native, let see:

I am thinking - if our FW is written correctly, it can reuse native executable most of the time (if not, fix FW) so that we don't need additional 40 m + 22 m. In my eyes, it should be something about 10 additional minutes as JVM requires additional 8m 28s.

Thoughts?

michalvavrik commented 1 week ago

@gtroitsk let's give one to two days to have a say, otherwise I suggest to run K8 native and baremetal native together to figure out if we can safe considerable time.

mjurc commented 1 week ago

@michalvavrik @gtroitsk I don't mind losing those minutes in daily build at all actually, since we're not in a hurry for daily build. So I still vote for keeping the Kuberenetes runs separate from the other natives.

gtroitsk commented 1 week ago

@michalvavrik @gtroitsk I don't mind losing those minutes in daily build at all actually, since we're not in a hurry for daily build. So I still vote for keeping the Kuberenetes runs separate from the other natives.

I agree with this solution for daily runs. In case of PRs CI separate native build extend running time almost twice

michalvavrik commented 1 week ago

@michalvavrik @gtroitsk I don't mind losing those minutes in daily build at all actually, since we're not in a hurry for daily build. So I still vote for keeping the Kuberenetes runs separate from the other natives.

I am concerned about CI workflow. The Daily was removed from CI workflow name in the https://github.com/quarkus-qe/quarkus-test-framework/pull/1187.

mjurc commented 1 week ago

Alright, let's put it in together with CI native and see how stable it is.

github-actions[bot] commented 1 week ago

Following jobs contain at least one flaky test: 'Linux - JVM build - Latest Version'

michalvavrik commented 1 week ago

I can see K8 tests running in native CI, new numbers are:

Which is big difference to previous times. IMO we should keep it.

michalvavrik commented 6 days ago

We need to revert this one as explained here https://github.com/quarkus-qe/quarkus-test-framework/pull/1191#issuecomment-2206606288. I didn't mention this PR was from main repo and not fork.