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

[WIP] Introduce Jenkins Job Builder for CI #344

Closed
wants to merge 1 commit into from

Conversation

wainersm
Copy link
Contributor

@wainersm wainersm commented Oct 5, 2020

This change introduces the files and scripts to use with Jenkins
Job Builder to manage our CI jobs.

Fixes #343

Signed-off-by: Wainer dos Santos Moschetta wainersm@redhat.com

@wainersm
Copy link
Contributor Author

wainersm commented Oct 5, 2020

Hi @GabyCT @chavafg , this PR is just a draft for the idea I proposed in #343 . If you think it is worth to adopt the tool, then I can manage to finish the implementation.

With this initial yaml files, the following jobs are generated:

# jenkins-jobs --conf jjb.conf list -p jobs/
INFO:root:Matching jobs: 8
kata-containers-agent-centos8-pr
kata-containers-agent-ubuntu1804-pr
kata-containers-osbuilder-centos8-pr
kata-containers-osbuilder-ubuntu1804-pr
kata-containers-runtime-centos8-pr
kata-containers-runtime-ubuntu1804-pr
kata-containers-tests-centos8-pr
kata-containers-tests-ubuntu1804-pr

Copy link
Contributor

@jodh-intel jodh-intel left a comment

Choose a reason for hiding this comment

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

Hi @wainersm,

I like this idea a lot. Thanks very much for raising this!!

XML is terrible and YAML is quite the opposite imho :)

A few comments but this is a major improvement over the morass of XML spew which is frankly unreviewable.

{% elif os == "centos8" -%}
centos8_azure
{% else %}
unknown
Copy link
Contributor

Choose a reason for hiding this comment

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

ooi, what happens if we hit this condition?


export GOPATH=$WORKSPACE/go
export GOROOT="/usr/local/go"
export PATH=${{GOPATH}}/bin:/usr/local/go/bin:/usr/sbin:/usr/local/bin:${{PATH}}
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Could you use $GOROOT/bin here rather than hard-coding /usr/local/go/bin?
  • Should/usr/local/bin be "higher up" than /usr/sbin/?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be honest I just copied the builder scripts from some of our current jobs, so I didn't pay attention to that kind of detail. I will fix it as we go.

Now that you asked, I noted that some jobs (maybe the majority?) simply clone the tests repo and call tests/.ci/jenkins_job_build.sh; others curl and execute tests/.ci/ci_entry_point.sh. So what's the ultimate entry point script for CI?

export GOROOT="/usr/local/go"
export PATH=${{GOPATH}}/bin:/usr/local/go/bin:/usr/sbin:/usr/local/bin:${{PATH}}

cd $GOPATH/src/github.com/kata-containers/tests
Copy link
Contributor

Choose a reason for hiding this comment

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

cd "${{tests_repo_dir}}"

jjb/jobs/template.yaml Outdated Show resolved Hide resolved
#
# Use this script to publish the jobs on Jenkins.
#

Copy link
Contributor

Choose a reason for hiding this comment

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

Script isn't set -e. How about also adding set -o nounset?

builders:
- shell: |
#!/bin/bash
set -e
Copy link
Contributor

Choose a reason for hiding this comment

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

How about also adding set -o nounset? Also, is there a way to set a variable to enable tracing to work with out standard idiom of:

[ -n "${DEBUG:-}" ] && set -o xtrace

jjb/publish_jobs.sh Outdated Show resolved Hide resolved
[ -n "$config_file" ] || usage

readonly cmd="jenkins-jobs"
command -V $cmd || echo "Needs $cmd command"
Copy link
Contributor

Choose a reason for hiding this comment

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

command -V "$cmd" >/dev/null ?

.gitignore Outdated Show resolved Hide resolved
@jodh-intel jodh-intel added the release-gating Release must wait for this to be resolved before release label Oct 6, 2020
@wainersm
Copy link
Contributor Author

wainersm commented Oct 9, 2020

Hi @wainersm,

I like this idea a lot. Thanks very much for raising this!!

XML is terrible and YAML is quite the opposite imho :)

A few comments but this is a major improvement over the morass of XML spew which is frankly unreviewable.

Ditto.

@jodh-intel I'm glad that you like the idea, it will really improve the management of jobs. I will be working on addressing your initial review but there are many decisions to take. We keep talking in this PR, is it okay for you?

This change introduces the files and scripts to use with Jenkins
Job Builder to manage our CI jobs.

Fixes kata-containers#343

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Copy link
Contributor

@jodh-intel jodh-intel left a comment

Choose a reason for hiding this comment

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

Thanks @wainersm - a couple of comments.

btw, it helps if you add a comment to the PR once you've re-pushed so we get notified that it needs are view again ;)

I'm very keen that we switch to YAML config, particularly for Kata 2.x jobs due to the fact it's so much clearer and easier to review and understand the jobs. Can't wait to see this land!

/cc @amshinde.

@@ -0,0 +1,68 @@
---
- job-template:
name: '{configuration}-nightly-x86_64'
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you try to find a way to uses variables to avoid hard-coding architectures? Ideally, we'd be able to use this set of files for all architectures Kata runs on.

[ -d "${{tests_repo_dir}}" ] || git clone "https://${{tests_repo}}.git" "${{tests_repo_dir}}"
${{GOPATH}}/src/${{tests_repo}}/.ci/install_go.sh -p -f
pushd ${{tests_repo_dir}}
./cmd/container-manager/manage_ctr_mgr.sh docker install -f
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it makes sense to leave the existing Jenkins XML config for Kata 1.x and just make the switch to YAML config for the 2.0-dev branch?

wdyt @GabyCT, @amshinde ?

@jodh-intel
Copy link
Contributor

Ping @kata-containers/ci for more reviews.

@wainersm
Copy link
Contributor Author

@jodh-intel Hi James! I'd like to let you know I didn't forget this! I am managing to get some time to resume this work, and I believe next week I will be able to.

@jodh-intel
Copy link
Contributor

Thanks @wainersm ! 😄

@jodh-intel jodh-intel added the do-not-merge PR has problems or depends on another label Jan 4, 2021
@jodh-intel
Copy link
Contributor

Added dnm label as we now have #359.

@jodh-intel jodh-intel mentioned this pull request Jul 9, 2021
@jodh-intel
Copy link
Contributor

@wainersm - can we close this as no longer required?

@wainersm
Copy link
Contributor Author

Closed in favor of #359

@wainersm wainersm closed this Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge PR has problems or depends on another release-gating Release must wait for this to be resolved before release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use the Jenkins Job Builder to manage the jobs
2 participants