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.

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

Add Kubernetes jobs to GH Actions workflow #1174

Merged
merged 1 commit into from
Jul 2, 2024
Merged

Conversation

gtroitsk
Copy link
Member

@gtroitsk gtroitsk commented Jun 20, 2024

Summary

Closes #433
Adds jobs to run Kubernetes tests in JVM and Native modes for both PR and daily CI runs

Please check the relevant options

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Dependency update
  • Refactoring
  • Breaking change (fix or feature that would cause existing functionality to change)
  • This change requires a documentation update
  • This change requires execution against OCP (use run tests phrase in comment)

Checklist:

  • Example scenarios has been updated / added
  • Methods and classes used in PR scenarios are meaningful
  • Commits are well encapsulated and follow the best practices

@gtroitsk gtroitsk force-pushed the fix-kubernetes-ci branch 2 times, most recently from 35462e2 to 6d25399 Compare June 23, 2024 18:19
@gtroitsk gtroitsk marked this pull request as ready for review June 23, 2024 18:24
@gtroitsk gtroitsk marked this pull request as draft June 24, 2024 06:56
Copy link

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

@rsvoboda
Copy link
Member

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
Copy link
Member

mjurc commented Jun 28, 2024

Agreed, it should be standalone

@michalvavrik
Copy link
Member

Agreed, it should be standalone

+1

@gtroitsk gtroitsk force-pushed the fix-kubernetes-ci branch 3 times, most recently from 935e106 to a1d48b6 Compare June 30, 2024 18:51
@gtroitsk gtroitsk marked this pull request as ready for review June 30, 2024 18:52
@michalvavrik
Copy link
Member

run tests

@gtroitsk
Copy link
Member Author

gtroitsk commented Jul 1, 2024

run tests

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

@michalvavrik
Copy link
Member

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
Copy link
Member

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

  • Daily - Linux - Native build - Released Version (current, 17, examples/pingpong,examples/restclie.
  • Daily - Linux - Native build - Released Version (current, 17, !examples/pingpong,!examples/restcl...
  • Linux - JVM build - Latest Version (999-SNAPSHOT, 17)

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
Copy link
Member

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:

  • Daily - Linux - Native build - Released Version (current, 17, !examples/pingpong,!examples/restcl...
    • Build uses 30m 16s
    • Build for Kubernetes uses 22m 8s
  • Daily - Linux - Native build - Released Version (current, 17, examples/pingpong,examples/restclie...
    • Build uses 48m 31s
    • Build for Kubernetes uses 40m 34s

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
Copy link
Member

@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
Copy link
Member

mjurc commented Jul 1, 2024

@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
Copy link
Member Author

gtroitsk commented Jul 1, 2024

@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
Copy link
Member

@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 #1187.

@mjurc
Copy link
Member

mjurc commented Jul 2, 2024

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

Copy link

github-actions bot commented Jul 2, 2024

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

@michalvavrik
Copy link
Member

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

  • Linux - Native build - Released Version (current, 17, !examples/pingpong,!examples/restclient,!ex... took 24 m 7s
  • Linux - Native build - Released Version (current, 17, examples/pingpong,examples/restclient,examp... took 42m 40s

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

Copy link
Member

@michalvavrik michalvavrik left a comment

Choose a reason for hiding this comment

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

LGTM; I added very unimportant comments. Please address them and merge this.

.github/workflows/daily.yaml Outdated Show resolved Hide resolved
.github/workflows/daily.yaml Show resolved Hide resolved
.github/workflows/pr.yaml Outdated Show resolved Hide resolved
@gtroitsk gtroitsk merged commit 893f022 into main Jul 2, 2024
8 checks passed
@gtroitsk gtroitsk deleted the fix-kubernetes-ci branch July 2, 2024 10:40
@michalvavrik
Copy link
Member

We need to revert this one as explained here #1191 (comment). I didn't mention this PR was from main repo and not fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kubernetes use-cases are not working
4 participants