Port fork_ami to a Jenkins multistep job #220
Conversation
0xmichalis
changed the title
| @@ -0,0 +1,206 @@ | |||
| --- | |||
| timer: 'H H * * *' | |||
There was a problem hiding this comment.
This job should be triggered by people, not a timer.
| @@ -0,0 +1,206 @@ | |||
| --- | |||
| timer: 'H H * * *' | |||
| parameters: [] | |||
There was a problem hiding this comment.
The fork_ami job has a lot of parameters on Jenkins. I would have expected to see them here, too.
| - type: "host_script" | ||
| title: "install CI user account" | ||
| script: |- | ||
| oct prepare user |
There was a problem hiding this comment.
You'll want to pick up my hack from #219
| GOLANG_VERSION=1.4.2 | ||
| OS_BUILD_ENV_IMAGE="openshift/origin-release:golang-1.4" | ||
|
|
||
| elif [[ "${TARGET_ORIGIN_BRANCH}" =~ enterprise-3.[3-5] || "${TARGET_ORIGIN_BRANCH}" =~ release-1.[3-5] ]]; then |
There was a problem hiding this comment.
3.5 should be on 1.7
There was a problem hiding this comment.
Should we also update the old fork_ami? Master is also 1.7 I guess..
There was a problem hiding this comment.
Until this lands people will still use the old job, so yeah let's update that as well.
| OS_BUILD_ENV_PRESERVE=_output/local/bin/linux/amd64/integration.test hack/env make build-integration-test | ||
| OPENSHIFT_SKIP_BUILD='true' JUNIT_REPORT='true' make test -o check -o build -k | ||
| - type: "script" | ||
| title: "run extended conformance tests" |
There was a problem hiding this comment.
Not sure how this meshes with the old extended test selection logic above
| default_value: "false" | ||
| - name: "RUN_EXTENDED" | ||
| description: "Run the QE 'smoke' tests after build and basic verification success. These tests should be run for most amis." | ||
| default_value: "true" |
There was a problem hiding this comment.
@stevekuznetsov not sure how I make these booleans? Is there any doc somewhere about the syntax used here?
There was a problem hiding this comment.
I think we only support string variables right now -- should debug to see how "boolean" variables are done by Jenkins, shell vars are always strings anyway. That may be less effort than changing the generator stuff to honor different types of variables.
There was a problem hiding this comment.
Yeah, shell variables are strings but how does this cash out in the Jenkins UI?
There was a problem hiding this comment.
Let me pack this into a test job and see
There was a problem hiding this comment.
Jenkins UI will have a string field, into which people can type "true" and or "false" ;)
I'm OK with losing that UX here.
|
@stevekuznetsov got any idea why some steps have been skipped in https://ci.openshift.redhat.com/jenkins/view/All/job/ami_build_origin_int_rhel_fork/1/consoleFull? For example, there is no INSTALL GOLANG step... |
|
Step failed: |
|
Yeah saw that but I was wondering why we report it back as a success.. |
|
@Kargakis yeah very unclear ... would have to check return code of the script, as |
|
Wait, that's running on the Jenkins host, so it should just be the return code of the shell itself. |
|
@stevekuznetsov this should be ready for a review. I've removed the extended tests since they don't run as part of the original fork ami. |
| title: "install golang" | ||
| script: |- | ||
| if [[ "${TARGET_ORIGIN_BRANCH}" =~ enterprise-3.[1-2] || "${TARGET_ORIGIN_BRANCH}" =~ release-1.[1-2] ]]; then | ||
| oct prepare golang --version=1.4.2 |
There was a problem hiding this comment.
Not actually certain this will work. You may need to grab the ugly hack from test_pull_requests_ose with the crazy workaround -- yum install golang-1.4.2 does this thing where it notices 1.6 is available and installs that instead because you know that's great UX.
There was a problem hiding this comment.
Just stood a bare ami up and run oct prepare golang --version=1.4.2. Waiting for the results. If this is not working then we will also need to update the help text of oct prepare golang.
There was a problem hiding this comment.
Yeah, go1.6.3 is installed instead.
| export OS_BUILD_ENV_IMAGE="openshift/origin-release:golang-1.6" | ||
|
|
||
| elif [[ "${TARGET_ORIGIN_BRANCH}" =~ enterprise-3.5 || "${TARGET_ORIGIN_BRANCH}" =~ release-1.5 ]]; then | ||
| oct prepare golang --version=1.7.5 -u https://cbs.centos.org/repos/paas7-openshift-origin36-candidate/x86_64/os/ |
There was a problem hiding this comment.
These will probably work (even if they weren't the latest version) because you'll only have one repo enabled.
| if [[ -z "$EXTENDED_TESTS" ]]; then | ||
| EXTENDED_TESTS=$test | ||
| else | ||
| EXTENDED_TESTS=$EXTENDED_TESTS,$test |
There was a problem hiding this comment.
What's this block doing?
There was a problem hiding this comment.
Seems that it appends to EXTENDED_TESTS - I guess different test modes but I am not sure.
There was a problem hiding this comment.
Based on the existing script most of this seems redundant. I will boil it down to the necessary steps.
| - type: "host_script" | ||
| title: "prepare environment" | ||
| script: |- | ||
| set +o nounset |
There was a problem hiding this comment.
Fix the bash, don't turn this off
There was a problem hiding this comment.
This was breaking in TRELLO_URL. The thing is that bash shouldn't break there because there is a non-empty check. I think this may be related to the fact that TRELLO_URL is the only parameter that doesn't get a default value so it may not be exported in this script at all?
| - type: "host_script" | ||
| title: "release the AMI" | ||
| script: |- | ||
| oct package ami --mark-ready |
There was a problem hiding this comment.
We need to make sure that these fork AMIs are not used by the rest of the process -- oct provision will look for tags on the AMI to determine operating system, image stage, readiness, etc -- we need to probably create a new image stage fork for these? Not sure what the best approach is, but I did not realize this would be an issue until just now -- we'll need to change code in origin-ci-tool to support this. One of the most simple and otherwise useful approaches may be to allow oct package ami to take a variable number of --tag KEY=VALUE type flags that we stick on the AMI as-is, and amend the oct provision logic to ensure that there isn't a Trello URL in the tags or something, instead of trying to add in a new stage.
There was a problem hiding this comment.
--mark-ready is essentially --tag ready=yes so I was wondering how --tag will add the keys in the tasks? Values can be variable but I am not sure about the keys.
There was a problem hiding this comment.
| script: |- | ||
| oct package ami --mark-ready | ||
| artifacts: | ||
| - "/tmp/openshift" |
| script: |- | ||
| set +o nounset +o errexit | ||
| if [[ -n "$TRELLO_URL" ]]; then | ||
| trello comment "A fork ami has been created for this card: \`${TAG_NAME}_${BUILD_NUMBER}\`" --card-url $TRELLO_URL |
There was a problem hiding this comment.
The AMI will no longer have a tag:Name=${TAG_NAME}_${BUILD_NUMBER} -- that was only for the Vagrant provisioning. You'll need to identify it with the AMI ID or otherwise.
There was a problem hiding this comment.
How can I get the AMI ID?
| title: "prepare repositories" | ||
| script: |- | ||
| oct prepare repositories | ||
| # Replace origin repo with the specified fork |
There was a problem hiding this comment.
oct sync remote origin --new-remote fork "https://github.com/${ORIGIN_FORK_ID}/${ORIGIN_REPO}"
oct sync remote origin --remote fork --branch "${ORIGIN_BRANCH}" --merge-into masterThere was a problem hiding this comment.
Although if this is a private repo (fork of ose) there will not be a key to do it on the remote machine -- so we'd need to clone the correct state locally then use
oct sync local origin --branch "${ORIGIN_BRANCH}" --merge-into masterThere was a problem hiding this comment.
So git clone and oct sync local to cover both cases?
There was a problem hiding this comment.
Seems reasonable to me
| oct prepare repositories | ||
| # Replace origin repo with the specified fork | ||
| git clone git@github.com:"${ORIGIN_FORK_ID}"/"${ORIGIN_REPO}" | ||
| oct sync local origin --src "${ORIGIN_REPO}" --dest /data/src/github.com/openshift/origin --branch "${ORIGIN_BRANCH}" --merge-into "${TARGET_ORIGIN_BRANCH}" |
There was a problem hiding this comment.
@stevekuznetsov can you vet this change? Doesn't work for me and I can't figure out why
failed: [localhost] (item=172.18.1.100) => {
"changed": true,
"cmd": [
"/usr/bin/git",
"push",
"ssh://172.18.1.100/data/src/github.com/openshift/origin",
"master:master",
"--force",
"--tags"
],
"delta": "0:00:00.067998",
"end": "2017-05-11 06:46:37.514699",
"failed": true,
"generated_timestamp": "2017-05-11 06:46:37.534673",
"item": "172.18.1.100",
"rc": 128,
"start": "2017-05-11 06:46:37.446701",
"stderr": [
"Host key verification failed.",
"fatal: Could not read from remote repository.",
"",
"Please make sure you have the correct access rights",
"and the repository exists."
],
"stdout": [],
"warnings": []
}
There was a problem hiding this comment.
the --dest looks unnecessary here
|
yeah, removed it in the interim
…On Thu, May 11, 2017 at 5:02 PM, Steve Kuznetsov ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In sjb/config/test_cases/ami_build_origin_int_rhel_fork.yml
<#220 (comment)>:
> +
+ elif [[ "${TARGET_ORIGIN_BRANCH}" == "master" ]]; then
+ oct prepare docker --version=1.12.6 --repo=oso-rhui-rhel-server-* -u https://mirror.openshift.com/enterprise/rhel/dockerextra/x86_64/os/
+
+ else
+ echo "[FATAL] No Docker version is known for \`${TARGET_ORIGIN_BRANCH}\`.
+ Contact Steve Kuznetsov ***@***.***> for assistance."
+ exit 1
+ fi
+ - type: "host_script"
+ title: "prepare repositories"
+ script: |-
+ oct prepare repositories
+ # Replace origin repo with the specified fork
+ git clone ***@***.***:"${ORIGIN_FORK_ID}"/"${ORIGIN_REPO}"
+ oct sync local origin --src "${ORIGIN_REPO}" --dest /data/src/github.com/openshift/origin --branch "${ORIGIN_BRANCH}" --merge-into "${TARGET_ORIGIN_BRANCH}"
the --dest looks unnecessary here
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#220 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADuFf0_IaIxAmDFqeBn94ruLePTLv3viks5r4yLzgaJpZM4NI5Rj>
.
|
| fi | ||
| done | ||
|
|
||
| QE_TEST_QUEUE=~/fork_ami_qe_test_queue |
There was a problem hiding this comment.
@stevekuznetsov this is cp'd from the original job but I still am not sure of its purpose. Any ideas?
There was a problem hiding this comment.
It's not clear what this is used by. The file definitely exists, but I can't find any jobs that pick it up. @danmcp ?
There was a problem hiding this comment.
I am guessing that this particular script needs to run inside the AMI (so "script" instead of "host_script") and when a QE stands the image up, he will look at ~/fork_ami_qe_test_queue and run the tests found in the file.
There was a problem hiding this comment.
What beats me is that it runs outside the AMI in the old job. Do we know who wrote that?
There was a problem hiding this comment.
It was used by the QE jenkins system. When you kick off an forkami, there are options for what set of QE tests you want to run against it. This option is read by the QE jenkins systems and the requested jobs are kicked off there.
@danmcp should ~/fork_ami_qe_test_queue be inside the AMI or in the host (ie. our ci.openshift Jenkins workspace)?
There was a problem hiding this comment.
It was previously used on the host. But it doesn't look like QE is currently using it. I would ask the QE leads if they still have a use case for it going forward and if not then remove it.
There was a problem hiding this comment.
AFAIK QE didn't use this queue, @akostadinov @xltian @jianlinliu Do you have any thoughts here?
There was a problem hiding this comment.
It's used in V2, not used in V3, it could be removed now, we will discuss with CD team later about using CI message bus to trigger QE's testing automatically for different build.
There was a problem hiding this comment.
Thanks guys, will remove for now
|
@stevekuznetsov is this a straight port of |
|
No, we'll probably need to update the pruning logic -- but it should neatly fit into the tag-based approach, I think @Kargakis is adding a new tag to show it's a fork. |
| - type: "host_script" | ||
| title: "release the AMI" | ||
| script: |- | ||
| oct package ami --mark-ready --tag qe=ready |
There was a problem hiding this comment.
@stevekuznetsov what tag do you want here?
There was a problem hiding this comment.
@Kargakis sync with QE -- this specific pairing looks fine to me but if they have opinions I defer to them
There was a problem hiding this comment.
@wanghaoran1988 need your input here
There was a problem hiding this comment.
@wanghaoran1988 the context here is we are updating the job that builds AMIs for QE. In the Trello post and/or the e-mail we will have the AMI ID but we were wondering what tags you would like to see on the AMI itself -- @Kargakis maybe we can put QE metadata here like BZ ID?
There was a problem hiding this comment.
@stevekuznetsov I think it depends on what this PR is addressed, if this is used to generate a AMI for a feature that tracked on trello card, the card id is ok, if its for a bug, then a bug id is ok, so id like we define two tags here:
trello=< card id>
bugzila=< bug id >
qe=ready
The first two can be optional based on what this AMI addressd, and the qe=ready is alwasy required, which means it's ok for QE to have test.
There was a problem hiding this comment.
You can already set a Trello URL in Jenkins, I will also update this job to set an additional tag in the AMI. Bugzilla ID SGTM but I would prefer to do that in a follow-up since it's an extra feature not supported by the old fork_ami job.
There was a problem hiding this comment.
Ack, whatever we have available from the current interface is fine
There was a problem hiding this comment.
Opened #289 to track
|
We got our first fork AMI yesterday, this should be ready for a final pass @stevekuznetsov |
| description: "The resulting AMI will be tagged with the specified Trello card and a comment will be added to the card." | ||
| default_value: "<none>" | ||
| - name: "ORIGIN_FORK_ID" | ||
| description: "The fork id to use for the origin repo." |
There was a problem hiding this comment.
This is the GitHub organization or user whose fork of Origin to use, right?
There was a problem hiding this comment.
Right, do you want better wording?
There was a problem hiding this comment.
Yeah, that would be nice
| if [[ "${TARGET_ORIGIN_BRANCH}" =~ enterprise-3.[1-2] || "${TARGET_ORIGIN_BRANCH}" =~ release-1.[1-2] ]]; then | ||
| chmod +x install-go-14.sh | ||
| scp -F ./.config/origin-ci-tool/inventory/.ssh_config install-go-14.sh openshiftdevel:install-go-14.sh | ||
| ssh -F ./.config/origin-ci-tool/inventory/.ssh_config openshiftdevel sudo chown origin:origin install-go-14.sh |
There was a problem hiding this comment.
You are origin when you use the .ssh_config so this is not necessary
| chmod +x install-go-14.sh | ||
| scp -F ./.config/origin-ci-tool/inventory/.ssh_config install-go-14.sh openshiftdevel:install-go-14.sh | ||
| ssh -F ./.config/origin-ci-tool/inventory/.ssh_config openshiftdevel sudo chown origin:origin install-go-14.sh | ||
| ssh -F ./.config/origin-ci-tool/inventory/.ssh_config openshiftdevel sudo su origin install-go-14.sh |
There was a problem hiding this comment.
In order to get the right $PATH you should use:
ssh -F ./.config/origin-ci-tool/inventory/.ssh_config -t openshiftdevel "bash -l -c install-go-14.sh"
| oct prepare repositories | ||
| # Replace origin repo with the specified fork | ||
| oct sync remote origin --new-remote fork "https://github.com/${ORIGIN_FORK_ID}/${ORIGIN_REPO}" | ||
| oct sync remote origin --remote fork --branch "${ORIGIN_BRANCH}" --merge-into "${TARGET_ORIGIN_BRANCH}" |
There was a problem hiding this comment.
Did we determine what happens when someone uses a fork of ose for this?
There was a problem hiding this comment.
I couldn't figure it out when I looked but I opened #281 to track it.
| title: "use a ramdisk for etcd" | ||
| script: |- | ||
| sudo mkdir -p /tmp | ||
| sudo mount -t tmpfs -o size=2048m tmpfs /tmp |
There was a problem hiding this comment.
Do you want larger TMPFS here?
There was a problem hiding this comment.
Also in general this block is out of sync with the others
| chmod +x run_extended.sh | ||
| scp -F ./.config/origin-ci-tool/inventory/.ssh_config run_extended.sh openshiftdevel:run_extended.sh | ||
| ssh -F ./.config/origin-ci-tool/inventory/.ssh_config openshiftdevel sudo chown origin:origin run_extended.sh | ||
| ssh -F ./.config/origin-ci-tool/inventory/.ssh_config openshiftdevel sudo su origin run_extended.sh |
There was a problem hiding this comment.
Same comments here as above
|
@stevekuznetsov all comments addressed, new build running in https://ci.openshift.redhat.com/jenkins/view/All/job/ami_build_origin_int_rhel_fork/36/ |
Signed-off-by: Michail Kargakis <mkargaki@redhat.com>
Signed-off-by: Michail Kargakis mkargaki@redhat.com