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

Add github workflow running jenkins job #255

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Add github workflow running jenkins job #255

wants to merge 2 commits into from

Conversation

sylwiaszunejko
Copy link
Collaborator

@sylwiaszunejko sylwiaszunejko commented Sep 4, 2024 •
edited
Loading

This workflow pushes scylla-bench docker image using gocql version from the PR and triggers jenkins job that uses this image of scylla-bench.

@sylwiaszunejko sylwiaszunejko force-pushed the advanced_ci branch 21 times, most recently from 57efde1 to a9870c9 Compare September 11, 2024 12:18
@sylwiaszunejko sylwiaszunejko force-pushed the advanced_ci branch 7 times, most recently from 89801e4 to fa8ad66 Compare September 12, 2024 12:37
@sylwiaszunejko sylwiaszunejko mentioned this pull request Sep 13, 2024
Closed
@sylwiaszunejko sylwiaszunejko self-assigned this Sep 13, 2024
@dkropachev
Copy link
Collaborator

@roydahan What should be the structure of this official location? Drivers -> Gocql -> longevity-large-partition-asymmetric-cluster-3h-test?

And do we need to run asymmetric-cluster test ? or it is better to have regular 3h longevity for sake of stability ?

@sylwiaszunejko
Copy link
Collaborator Author

@roydahan Should the next steps be decided on SCT side e.g. which test to run and what should be the official location for those jenkins jobs?

@dkropachev
Copy link
Collaborator

dkropachev commented Sep 26, 2024 •
edited
Loading

@sylwiaszunejko , please take a look at job/scylla-drivers/job/gocql/job/advanced-ci-cd/job/longevity-large-partition-asymmetric-cluster-3h-test/

@sylwiaszunejko
Copy link
Collaborator Author

@sylwiaszunejko , please take a look at job/scylla-drivers/job/gocql/job/advanced-ci-cd/job/longevity-large-partition-asymmetric-cluster-3h-test/

shouldn't it be named extended-ci instead of advanced-ci-cd for consistency with the label and dockerhub repo? https://hub.docker.com/r/scylladb/gocql-extended-ci/tags

@dkropachev
Copy link
Collaborator

@sylwiaszunejko , please take a look at job/scylla-drivers/job/gocql/job/advanced-ci-cd/job/longevity-large-partition-asymmetric-cluster-3h-test/

shouldn't it be named extended-ci instead of advanced-ci-cd for consistency with the label and dockerhub repo? https://hub.docker.com/r/scylladb/gocql-extended-ci/tags

Done

@sylwiaszunejko
Copy link
Collaborator Author

So it looks like we have an official location for a job, next thing would be to have a designated user for it, is there some generic one we can use or who could create such?

roydahan
roydahan reviewed Sep 30, 2024
View reviewed changes
.github/workflows/extended-ci-longevity-large-partition-asymmetric-cluster-3h-test.yml Outdated
uses: sylwiaszunejko/jenkins_client@main
with:
jenkins_job_name: 'scylla-drivers/gocql/extended-ci/longevity-large-partition-asymmetric-cluster-3h-test'
jenkins_job_parameters: '{"email_recipients": "[email protected]", "scylla_version": "6.1.1", "extra_environment_variables": "SCT_STRESS_IMAGE.scylla-bench=scylladb/gocql-extended-ci:scylla-bench-${{ github.event.pull_request.head.sha }}"}'
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm unsure if we should fix the scylla_version to something specific.
On one hand, it can make the CI more predictable (not sure if stable, but quite repeatable).
On the other hand, at some point we need to go back and update it and possibly face new issues.

BTW, there is an option to set here "scylla_version: 6.1" so it will take all scylla 6.1 patches.
Another option is to use an enterprise release like 2024.1 who is LTS and won't require us to update it very often (will automatically pick the latest patch release).

Regarding email_recipent, can we set it to the author of the PR (as long as it internal author?) - not a must.

Lastly, I noticed that the Jenkins job is set to master, which again brings back the question of stability vs being updated automatically.

@sylwiaszunejko / @dkropachev / @fruch - your thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm unsure if we should fix the scylla_version to something specific. On one hand, it can make the CI more predictable (not sure if stable, but quite repeatable). On the other hand, at some point we need to go back and update it and possibly face new issues.

having master there is not an option.
We can do same thing we do for cpp-rust-driver - get last release and run on it.

BTW, there is an option to set here "scylla_version: 6.1" so it will take all scylla 6.1 patches. Another option is to use an enterprise release like 2024.1 who is LTS and won't require us to update it very often (will automatically pick the latest patch release).

From one side using OSS will bring top feature to the test as fast as possible.
On other side using enterprise LTS will bring more stability.
It looks like we need both ))

Regarding email_recipent, can we set it to the author of the PR (as long as it internal author?) - not a must.

It is doable, but it will expose ip addresses, urls of our test infrastructure, if we are ok with it then sure, why not.
But we also should have [email protected] as CC.

Lastly, I noticed that the Jenkins job is set to master, which again brings back the question of stability vs being updated automatically.

Agree, it is better to target either branch-6.1 or branch-2023 or particular commit from master

Copy link

Choose a reason for hiding this comment

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

I'm unsure if we should fix the scylla_version to something specific. On one hand, it can make the CI more predictable (not sure if stable, but quite repeatable). On the other hand, at some point we need to go back and update it and possibly face new issues.

having master there is not an option.
We can do same thing we do for cpp-rust-driver - get last release and run on it.

BTW, there is an option to set here "scylla_version: 6.1" so it will take all scylla 6.1 patches. Another option is to use an enterprise release like 2024.1 who is LTS and won't require us to update it very often (will automatically pick the latest patch release).

From one side using OSS will bring top feature to the test as fast as possible.
On other side using enterprise LTS will bring more stability.
It looks like we need both ))

Regarding email_recipent, can we set it to the author of the PR (as long as it internal author?) - not a must.

It is doable, but it will expose ip addresses, urls of our test infrastructure, if we are ok with it then sure, why not.
But we also should have [email protected] as CC.

Nothing secret in the emails
Also those should run only by trusted users (with some definition of trust), isn't it ?

Lastly, I noticed that the Jenkins job is set to master, which again brings back the question of stability vs being updated automatically.

Agree, it is better to target either branch-6.1 or branch-2023 or particular commit from master

release branch do get backports, on the other hand they might not have all of the fixes features for those test (not to mention we'll need to backport specific things unrelated to a release need)

A specific master commit, is also problematic, when one will update it ? what happens when it breaks and stops working ?

@sylwiaszunejko sylwiaszunejko force-pushed the advanced_ci branch 9 times, most recently from c021aaa to cdb6f0f Compare October 2, 2024 06:40
@sylwiaszunejko
Copy link
Collaborator Author

v2:

  • user changed to org level JENKINS_USERNAME
  • added the logic to get latest scylla oss and enterprise release (as in cpp-rust-driver)
  • added extra parameter to jenkins-client to be able to configure timeout for waiting in the queue (needed when we want to trigger two jobs for oss and enterprise release and one have to wait for the other to finish)
  • changed email to [email protected]

Things that still need to be addressed:

  • creating scylladb fork of jenkins-client, now this PR uses my fork
  • what branch should Jenkins job target
  • is running asymmetric-cluster test the right choice or should we choose different one?

@dkropachev @roydahan @fruch

@sylwiaszunejko
Add github workflow running jenkins job
a3d9e25 
This workflow pushes scylla-bench docker image using gocql
version from the PR and triggers jenkins job that uses this
image of scylla-bench.
@sylwiaszunejko
Copy link
Collaborator Author

sylwiaszunejko commented Oct 3, 2024 •
edited
Loading

https://github.com/scylladb/gocql/actions/runs/11157393428/job/31011588519?pr=263 I am not sure why the second workflow gets interrupted even though the timeout has not passed yet and the job was still running when it happened

@sylwiaszunejko
Copy link
Collaborator Author

@dkropachev is https://github.com/scylladb-actions/jenkins-client an official location for this action?

@sylwiaszunejko
Copy link
Collaborator Author

what branch should Jenkins job target
is running asymmetric-cluster test the right choice or should we choose different one?

@roydahan @fruch could you help with this questions?

@dkropachev
Copy link
Collaborator

@dkropachev is https://github.com/scylladb-actions/jenkins-client an official location for this action?

It is not, but it shouldn't matter since it is an GitHub action.

@roydahan
Copy link
Collaborator

roydahan commented Oct 8, 2024

what branch should Jenkins job target

is running asymmetric-cluster test the right choice or should we choose different one?

@roydahan @fruch could you help with this questions?

I'm working on stabilizing another shorter test for CI (1h test that is based on this test).
+
My comment from a different related issue:

I'm not sure what you mean by "need to manually create new job".

I also don't think this mechanism is necessary.

IMO the CI should use 2024.1 as Scylla_version and the SCT branch is branch-2024.1.
This promises to test on latest 2024.1 which LTS and make sure there are no regressions.

In addition, we can have another CI job if we think is necessary that points to "scylla_version: master:latest".
This job can be used with SCT branch master and should be used in order to test our driver with the latest and greatest Scylla features.
It won't be as stable as the first one, but it will be a good addition to get an additional data point on client stability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fruch fruch fruch left review comments

@Lorak-mmk Lorak-mmk Lorak-mmk left review comments

@yaronkaikov yaronkaikov yaronkaikov left review comments

@roydahan roydahan roydahan left review comments

@dkropachev dkropachev dkropachev left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@sylwiaszunejko sylwiaszunejko

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sylwiaszunejko @dkropachev @roydahan @fruch @Lorak-mmk @yaronkaikov