-
Notifications
You must be signed in to change notification settings - Fork 51
CI: Refactor Jenkinsfile helpers into implicit Shared Library #2092
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
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the Jenkinsfile by extracting reusable functions into separate utility modules in the vars/ directory. This improves code organization, maintainability, and reusability across Jenkins pipelines.
Key changes:
- Extracted utility functions from the monolithic Jenkinsfile into five modular files
- Removed the
AgentOfflineExceptionimport from the main Jenkinsfile (now in nodeUtils.groovy) - Organized functions by domain: build, CI logic, node management, reporting, SCM operations, and testing
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vars/buildUtils.groovy | Build-related functions for compiling projects (buildProject, buildCK, buildMIGraphX) |
| vars/ciLogic.groovy | CI workflow logic including label resolution, codepath filtering, and configuration splitting |
| vars/nodeUtils.groovy | Node health checks, GPU management, Docker configuration, and node selection logic |
| vars/reportUtils.groovy | Performance report post-processing and artifact archiving |
| vars/scmUtils.groovy | Git health checks and robust checkout with retry logic |
| vars/testUtils.groovy | Test execution functions including E2E tests, parameter sweeps, and coverage collection |
| mlir/utils/jenkins/Jenkinsfile | Removed function definitions (moved to utility files) and AgentOfflineException import |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
…ttps://github.com/ROCm/rocMLIR into leo/move-to-sharedlib
dorde-antic
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please include me in the review process once this is completed, so I’m aware of the changes for future work related to CI
Motivation
The primary
mlir/utils/jenkins/Jenkinsfilehas grown to over 1500 lines, with the vast majority being helper functions defined at the top. This makes the file difficult to read, maintain, and navigate.This PR refactors all this helper logic into an implicit Shared Library stored in the
vars/directory at the repository root. This greatly simplifies the Jenkinsfile, organizes helper code by purpose, and makes the logic reusable and easier to manage.This approach also ensures that any PRs modifying the shared library code will use the new code from that same PR for testing, rather than the one from the
developbranch.Technical Details
Created
vars/directory: Added a newvars/directory at the repository root. This is automatically loaded by Jenkins as an implicit library.Grouped Functions into Objects: All Groovy helper functions were moved from the Jenkinsfile and grouped into new "singleton" objects in the
vars/directory:scmUtils.groovy: ForgitHealthCheck()androbustScmCheckout().nodeUtils.groovy: ForresetGPUs(),checkNodeHealth(),dockerArgs(),get_gpu_architecture(), withHealthyNode(), etc.buildUtils.groovy: ForbuildProject(),buildCK(),getAndBuildMIGraphX(), etc.testUtils.groovy: ForpreMergeCheck(),setLitWorkerCount(),parameterSweep(), etc.reportUtils.groovy: ForpostProcessPerfRes()and archivePerfDB().ciLogic.groovy: For getLabelFromCodepath(),shouldRunFromChip(),resetBuild(), etc.generalUtils.groovy: ForsplitConfigFile().Updated
Jenkinsfile:def nodeUtils = nodeUtils(this)).pipelineblock to use their new object namespace (e.g.,checkNodeHealth()is now nodeUtils.checkNodeHealth()).Handled Global State: The global
@Field DOCKER_ARGS_BY_NODEvariable remains in theJenkinsfile. The Jenkinsfile'sthis(script) context is passed tonodeUtilsso it can read/write to this variable usingscript.DOCKER_ARGS_BY_NODE.