-
Notifications
You must be signed in to change notification settings - Fork 35
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
main | Introduce Jenkins Job Builder for CI #406
Conversation
@jodh-intel @GabyCT just now I noticed the #359 was opened against the unused 2.0-dev branch :) So this is just a forward-port.... |
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.
@wainersm. the idea looks reasonable to me, but I've left a bunch of comments, mostly about the distros / branches we don't support anymore.
@fidencio thanks for the review! Mind if I update this PR with another commit on top of the original? |
That's fine, let me know when you update and I can check the other commit. |
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.
Thanks @wainersm. A few more comments. Let's see if we can get this landed early January!
jobs-builder/jobs/dependencies.yaml
Outdated
export branch="{{branch}}" | ||
export target_branch="$branch" | ||
{% if flavor == "experimental" -%} | ||
export experimental_kernel=true |
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.
This was changed to build_type
on kata-containers/kata-containers#2519.
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.
@jodh-intel I will change it to build_type
but I have some doubts... is there any CI job using that?
I see tests/.ci/install_kata_kernel.sh
has not references to build_type
or experimental_kernel
. I am almost sure that script used to deal with cached experimental kernel. In the end, do we need a job for caching the experimental kernel?
Cc @jcvenegas
jobs-builder/jobs/dependencies.yaml
Outdated
name: kata-containers-{branch}-qemu-{arch} | ||
<<: *common_job_properties | ||
maintainers: | ||
- Wainer dos Santos Moschetta <wainersm@redhat.com> |
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.
You're brave! I'd be templted to either remove this section or put in a generic email address to avoid getting pinged for the rest of your life! 😄
This introduces the files and scripts to use with Jenkins Job Builder to manage our CI jobs. As a proof of concept, it is generated only four jobs: kata-containers-main-kernel-experimental-x86_64 kata-containers-main-kernel-vanilla-x86_64 kata-containers-main-qemu-x86_64 kata-containers-stable-2.2-qemu-x86_64 Fixes kata-containers#343 Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com> (cherry-picked from commit a22049c)
Updated the OS to node mapping file: - removed rhel8, debian and sles as they haven't CI jobs anymore - ARM jobs use Ubuntu 20.04 - jobs with label ubuntu1804_azure or ubuntu1804-azure should get a Ubuntu 18.04 VM Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Instead we should be building QEMU for stable-2.3 Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Added errexit and friends to publish_jobs.sh Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Updated the QEMU and Kernel templates so that: - golang package isn't installed - go installed with install_go.sh is searcheable - add more shell debug flags - kernel experimental job uses the new `build_type` variable Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Our Jenkins is configured to render the job description as HTML, so this changed the description of all jobs properly. Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
It is added a minus sign (-) in the control flow statements of os2node.yaml.inc so that newlines are striped off. Otherwise the output will contain newline characters which can end up messing with the generated XML representation of the jobs. Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
2890478
to
7b19b72
Compare
Make the spell checker happy. Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Updated this PR to address your comments, @fidencio , @jodh-intel , @jongwu. Also I could make the static check pass. |
The list of generated jobs with the job builder can be found at http://jenkins.katacontainers.io/view/JJB%20managed/ |
@wainersm, amazing work on this one. Please, just update the fedora32 usage to a newer one (matching with what Snir added) and we're good to go from my side. |
export target_branch="$branch" | ||
{% if flavor == "experimental" -%} |
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.
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.
source ci/lib.sh | ||
export branch="{{branch}}" | ||
export target_branch="$branch" | ||
{% if flavor == "experimental" -%} | ||
export experimental_kernel=true | ||
export build_type="experimental" |
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.
Hello @wainersm . I want to add a new arch-specific build_type, maybe it can be called "arch-experimental", to distinguish with this comment one. So, what about exporting this new build_type here?
I'm trying to do this here, but have not settled down.
@jodh-intel
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.
it's been a moving target :) What do you prefer:
a) merge as is; send a follow up when things are settle down
b) keep changing and merge after a settlement
I'm in favor of a)
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.
@jodh-intel @wainersm -, I'm fine for a). let's do the following after it settles down.
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.
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.
great! done!
The Fedora 32 jobs will be replaced by 35, so update the os2node.yaml.inc properly. Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Added the contact to the Kata CI team on the QEMU and Kernel job templates. Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Updated this PR to address the latest comments of @fidencio and @jodh-intel . I didn't change the experimental job, as agreed with @jongwu and @jodh-intel I will send a follow up when things are settle down. |
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.
lgtm, thanks @wainersm!
@fidencio @jodh-intel could you please merge this? ps: Fabiano added me on this repository but for some obscure reasons I cannot merge nor do administrative tasks on it... |
This introduces the files and scripts to use with Jenkins
Job Builder to manage our CI jobs.
As a proof of concept, it is generated only four jobs:
kata-containers-main-kernel-experimental-x86_64
kata-containers-main-kernel-vanilla-x86_64
kata-containers-main-qemu-x86_64
kata-containers-stable-2.2-qemu-x86_64
Fixes #343
Signed-off-by: Wainer dos Santos Moschetta wainersm@redhat.com
(cherry-picked from commit a22049c)