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

Move the majority of jobs to the full pipeline #1694

Merged
merged 1 commit into from
Oct 14, 2024

Conversation

MarcelKoch
Copy link
Member

This PR moves the majority of the gitlab jobs to the full pipeline.

IMO our CI could use a more thourough restructuring, since the job properties are very random, but that should be done later.

@MarcelKoch MarcelKoch added the 1:ST:ready-for-review This PR is ready for review label Oct 14, 2024
@MarcelKoch MarcelKoch requested a review from a team October 14, 2024 07:13
@MarcelKoch MarcelKoch self-assigned this Oct 14, 2024
@ginkgo-bot ginkgo-bot added the reg:ci-cd This is related to the continuous integration system. label Oct 14, 2024
@MarcelKoch MarcelKoch force-pushed the restructure-ci-pipelines branch 2 times, most recently from e218c76 to bde4502 Compare October 14, 2024 07:43
Copy link
Member

@yhmtsai yhmtsai left a comment

Choose a reason for hiding this comment

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

I would still prefer the original grouping strategy
The configuration under same compiler version are slightly different, so grouping them together might be easier to configure or distinguish them.

@MarcelKoch
Copy link
Member Author

@yhmtsai can you explain the original grouping strategy? For me all options, except for device+compiler, seem to be choosen very randomly. I.e. wether it's debug or release, shared or static, fast test enabled or not, .... This became especially clear for the #1536 PR.

@yhmtsai
Copy link
Member

yhmtsai commented Oct 14, 2024

I mean grouping by device+compiler.
for example, we might have several configuration under cuda 12.2.
we might only adjust a bit for debug/release or some minor configuration settings.
putting them together makes it easier to see minor changes than separating them partially in quick test section and partially in full test.

@yhmtsai yhmtsai requested a review from a team October 14, 2024 10:04
@pratikvn
Copy link
Member

I think it would make sense to group jobs properly to be able to navigate and access them easily. The main groups could be cuda, hip, sycl, mpi. In each of these groups, the number of jobs should be fewer. I dont know if it is possible to be subgroups within groups, but gitlab does allow for creation of groups atleast: https://docs.gitlab.com/ee/ci/jobs/#group-jobs-in-a-pipeline

@MarcelKoch MarcelKoch added 1:ST:run-full-test 1:ST:no-changelog-entry Skip the wiki check for changelog update labels Oct 14, 2024
@MarcelKoch
Copy link
Member Author

@yhmtsai that grouping is still there, just now it's under the full pipeline. I don't think it's reasonable to have our current 'quick' pipeline run with 15+ jobs, while our 'full' pipeline has ~10 jobs. The quick pipeline is always run, even if the PR is still a bit under development. The 15+ jobs are not needed during that phase, so the CI is just clogged with meaningless tests.

@MarcelKoch
Copy link
Member Author

@pratikvn That looks like a nice feature.

Sidenote: I would not count mpi as separated from the other groups, instead mpi should be enabled in almost all of our jobs.

@yhmtsai
Copy link
Member

yhmtsai commented Oct 14, 2024

@MarcelKoch I do not mean the quick/full changes in this pr.
more precise example in changes
rocm45 has two jobs at least. one is in quick section and the rest is in full section, but they are located quite far away in the code.
you can still put them together as the original one but only change the condition to quick like now.

original:

...
rocm45/config1: # no matter it is in full or quick
rocm45/config2: # full/quick is irreverent with location 
...

the current change

# Quick
rocm45/config1:
...
...
# Full
rocm45/config2: # it is far from the config1

@MarcelKoch
Copy link
Member Author

Ah, I see. I understand your issue, perhaps the file restructuring would fit better in a later PR, e.g. by naming those test just rocm-quick, etc.

@MarcelKoch MarcelKoch force-pushed the restructure-ci-pipelines branch from bde4502 to de32575 Compare October 14, 2024 13:08
@MarcelKoch MarcelKoch requested a review from yhmtsai October 14, 2024 13:08
@MarcelKoch MarcelKoch merged commit 2bbcfb9 into develop Oct 14, 2024
8 of 11 checks passed
@MarcelKoch MarcelKoch deleted the restructure-ci-pipelines branch October 14, 2024 15:15
Copy link

MarcelKoch added a commit to MarcelKoch/ginkgo that referenced this pull request Dec 2, 2024
This merge the majority of the gitlab jobs to the full pipeline.

Related PR: ginkgo-project#1694
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1:ST:no-changelog-entry Skip the wiki check for changelog update 1:ST:ready-for-review This PR is ready for review 1:ST:run-full-test reg:ci-cd This is related to the continuous integration system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants