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

jenkins: Redirect pkg diff to file #210

Merged
merged 1 commit into from
Aug 16, 2018

Conversation

Rahuls0720
Copy link
Contributor

@Rahuls0720 Rahuls0720 commented Aug 7, 2018

Archives pkg_diff.txt and syncs a copy of the file to the artifact server.

Jira card: https://projects.engineering.redhat.com/browse/COREOS-275

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 7, 2018
@Rahuls0720
Copy link
Contributor Author

@cgwalters How would you test that the pkgdiff.txt file appears at http://aos-ostree.rhev-ci-vms.eng.rdu2.redhat.com/rhcos/images/cloud/latest/?

@cgwalters
Copy link
Member

Yeah, the treecompose job doesn't currently sync there. We do sync out to http://aos-ostree.rhev-ci-vms.eng.rdu2.redhat.com/rhcos/repo/ - what's ${repo} in this job, and so we could just dump the text file in there?

But that would mean we'd only get diffs of the latest.

This quickly gets into #201 as well as #212

@Rahuls0720
Copy link
Contributor Author

Rahuls0720 commented Aug 8, 2018

${repo} refers to /srv/rhcos/treecompose/repo (I didn't update repo's definition in the commit).

I noticed treecompose isn't synced to http://aos-ostree.rhev-ci-vms.eng.rdu2.redhat.com/rhcos/images/cloud/latest/ so I added dirpath (not sure if it will work, I was hoping to test it using the PR). Another method could be passing the pkgdiff.txt file as a parameter to jenkinsfile.cloud but I'm not sure how wait: false might affect this (https://github.com/openshift/os/blob/master/Jenkinsfile.treecompose#L154).

Yes, this is only posting diffs of the latests. When starting work on this card, I was confused why this takes priority over #201. But, if I can test that this works, updating it to pkg diff tags should be straightforward.

@Rahuls0720
Copy link
Contributor Author

@cgwalters Do you know the login/password for the jenkins account or how to set one up? I'm not able to get in using my normal RH login.

I guess I could sync the files to http://aos-ostree.rhev-ci-vms.eng.rdu2.redhat.com/rhcos/repo/. I feel that it would be most useful to have the pkg diff live in the same directory as the tagged image. Something like having the latest directory contain a pkg_diff.txt between the previous commit and the current. In the alpha directory, having a pkg_diff.txt between latest and alpha, and so on.

@cgwalters
Copy link
Member

Do you know the login/password for the jenkins account or how to set one up?

It's still a huge pain to test jobs, particularly in a way that doesn't end up overwriting prod data. That said I granted you access (I think).

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 10, 2018
@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 13, 2018
@openshift-ci-robot openshift-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 13, 2018
@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 14, 2018
@@ -92,7 +94,9 @@ node(NODE) {
version = utils.get_rev_version("${repo}", commit)
currentBuild.description = "🆕 ${version} (commit ${commit})";
if (previous_commit.length() > 0) {
sh "rpm-ostree --repo=${repo} db diff ${previous_commit} ${commit}"
sh "mkdir -p ${images}"
utils.rsync_dir_in(ARTIFACT_SERVER, KEY_FILE, images)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If images is the best place for this file to live, should I create a sync in stage?

Copy link
Member

Choose a reason for hiding this comment

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

You don't need to sync anything in for this operation. Just making the directory and writing out the pkg diff should be sufficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed the cloud pipeline sync it here: https://github.com/openshift/os/blob/master/Jenkinsfile.cloud#L36. If it isn't sync, wouldn't files/directories get overwritten during treecompose's rsync-out stage?

Copy link
Member

@miabbott miabbott Aug 16, 2018

Choose a reason for hiding this comment

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

As I understand it, in the cloud pipeline, that sync in is used so that the previous commit.txt artifact can be referenced in the Check for changes stage:

https://github.com/openshift/os/blob/master/Jenkinsfile.cloud#L60

Since writing out the pkg diff doesn't require the knowledge of any previous artifacts in that images directory, I believe this rsync_dir_in can be omitted.

@Rahuls0720
Copy link
Contributor Author

@miabbott When you get a chance, can you look over this.

sh "rpm-ostree --repo=${repo} db diff ${previous_commit} ${commit}"
sh "mkdir -p ${images}"
utils.rsync_dir_in(ARTIFACT_SERVER, KEY_FILE, images)
sh "rpm-ostree --repo=${repo} db diff ${previous_commit} ${commit} > /${images}/pkg_diff.txt"
Copy link
Member

@miabbott miabbott Aug 15, 2018

Choose a reason for hiding this comment

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

I think we should archive the pkg_diff.txt each time it is created, as it will be overwritten whenever there is a new commit to diff.

Perhaps you can write out the diff to ${WORKSPACE}/pkg_diff.txt and then copy it to ${images}. Having the file in the workspace would allow us to use the archiveAritifacts function like we do in the cloud pipeline:

https://github.com/openshift/os/blob/master/Jenkinsfile.cloud#L94-L96

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. If we archive pkg_diff.txt, doesn't it still get overwritten?

Copy link
Member

Choose a reason for hiding this comment

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

My understanding of archiveArtirfacts is that it keeps the files around on the Jenkins host until they are pruned. For example, in the cloud pipeline, we archive the rhcos-qcow2-install.txt:

https://continuous-infra-jenkins.rhev-ci-vms.eng.rdu2.redhat.com/view/rhcos/job/coreos-rhcos-cloud/1561/artifact/rhcos-qcow2-install.txt

https://continuous-infra-jenkins.rhev-ci-vms.eng.rdu2.redhat.com/view/rhcos/job/coreos-rhcos-cloud/1558/artifact/rhcos-qcow2-install.txt

So we are able to keep files per build, but it is overwritten on the artifact server in the sync out stage.

@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 15, 2018
@Rahuls0720 Rahuls0720 changed the title [WIP] jenkins: Redirect pkg diff to file jenkins: Redirect pkg diff to file Aug 15, 2018
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 15, 2018
@Rahuls0720
Copy link
Contributor Author

Rahuls0720 commented Aug 16, 2018

@miabbott Can you look over this PR once again before you leave for the conference (and my internship ends)?

It passed the tests on rahul-treecompose-test-pipeline (https://continuous-infra-jenkins.rhev-ci-vms.eng.rdu2.redhat.com/blue/organizations/jenkins/rhcos/activity/) which runs Jenkinsfile from https://github.com/Rahuls0720/rhcos/tree/pkg_diff-jenkinsTESTING. That jenkinsfile is identical to the Jenkinsfile.treecompose in this PR.

currentBuild.description = "🆕 ${version} (commit ${commit})";
if (previous_commit.length() > 0) {
sh "rpm-ostree --repo=${repo} db diff ${previous_commit} ${commit}"
try {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's necessary to wrap this whole stage in a try{}, but it shouldn't cause any problems leaving it in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could be totally wrong since jenkins is (seems to be) unpredictable, but it failed saying it doesn't understand the command archiveArtifacts when I had the finally statement here-ish: https://github.com/Rahuls0720/rhcos/blob/pkg_diff-jenkinsTESTING/Jenkinsfile#L98

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll try testing it without the try again

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LOL nvm ^...already merged :)

@miabbott
Copy link
Member

@Rahuls0720 I had one minor comment that doesn't need to be resolved.

I think this is ready to get merged. 🎉

First we'll test it, though

@miabbott
Copy link
Member

/ok-to-test

@openshift-ci-robot openshift-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 16, 2018
@miabbott
Copy link
Member

Let's roll with it!

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 16, 2018
@openshift-merge-robot openshift-merge-robot merged commit 163986a into openshift:master Aug 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants