Skip to content
This repository has been archived by the owner on May 6, 2020. It is now read-only.

CI: Do not build, but do update, repos when pulling them #888

Conversation

grahamwhaley
Copy link
Contributor

Without a '-d' on the 'go get', the script ties to build the repos,
and for repos that do not work under 'go build', this leaves errors
in the CI logs. We don't need a build at this stage as we invoke the
repo specific builds later on.
Also, some CIs do not clean out their build trees between builds, and
thus have repos already present - so add an 'update -u' flag as well.

Fixes: #887

Signed-off-by: Graham Whaley [email protected]

@clearcontainersbot
Copy link

kubernetes qa-failed 👎

@@ -48,10 +48,11 @@ then
go get github.com/mattn/goveralls
fi

# Get the repository and move to the correct commit
go get ${cc_repo} || true
# Get or update the repository, but do build ie.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: should that be "but do not build it."?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed. Some odd finger/brain trouble wen't on there.... fixed.

@grahamwhaley grahamwhaley force-pushed the 20180205_jenkins_goget branch from 2aa4720 to 7f0f372 Compare February 5, 2018 13:42
@clearcontainersbot
Copy link

kubernetes qa-failed 👎

@chavafg
Copy link
Contributor

chavafg commented Feb 6, 2018

lgtm
But I see these errors on the ci-metrics Job:

05:42:26 hacking our file ownerships
05:42:26  Fetch the tests repo
05:42:26 # cd /srv/jenkins/workspace/workspace/clear-containers-tests-16.04-PR/go/src/github.com/clearcontainers/tests; git pull --ff-only
05:42:26 There is no tracking information for the current branch.
05:42:26 Please specify which branch you want to merge with.
05:42:26 See git-pull(1) for details.
05:42:26 
05:42:26     git pull <remote> <branch>
05:42:26 
05:42:26 If you wish to set tracking information for this branch you can do so with:
05:42:26 
05:42:26     git branch --set-upstream-to=origin/<branch> PR_886
05:42:26 
05:42:26 package github.com/clearcontainers/tests: exit status 1

Approved with PullApprove Approved with PullApprove

@grahamwhaley
Copy link
Contributor Author

Yeah, I've not gotten to dig into those errors yet. To note then, currently the metrics CI is a bare metal Jenkins that is not clearing out its build trees between jobs. That may thus have an effect here, particularly if we are rebuilding a PR, as that could be trying to re-use the same branch name.
It is pretty easy for me to switch the config to delete the build trees between runs - it will cost us some download time per run should be all.

@jcvenegas
Copy link
Contributor

same error happening on #895

@grahamwhaley
Copy link
Contributor Author

The Error:

10:27:16  Fetch the tests repo
10:27:16 # cd /srv/jenkins/workspace/workspace/clear-containers-tests-16.04-PR/go/src/github.com/clearcontainers/tests; git pull --ff-only
10:27:16 There is no tracking information for the current branch.
10:27:16 Please specify which branch you want to merge with.
10:27:16 See git-pull(1) for details.
10:27:16 
10:27:16     git pull <remote> <branch>
10:27:16 
10:27:16 If you wish to set tracking information for this branch you can do so with:
10:27:16 
10:27:16     git branch --set-upstream-to=origin/<branch> PR_886
10:27:16 

looks like it comes from this fragment of the Jenkins script:

# nominally, as we are about to test the tests repo, maybe we don't
# need this...
echo " Fetch the tests repo"
go get -d -u ${TESTS_REPO_NAME}

I'm thus wondering if the got get -d -u cannot handle the non-fast-forward branch. I could inject a git reset in the script before that line - what do you think @jcvenegas ?

@devimc
Copy link

devimc commented Feb 28, 2018

lgtm

Approved with PullApprove Approved with PullApprove

@grahamwhaley
Copy link
Contributor Author

A note/update on this then. This is likely:

  • related to the metrics Jenkins build bots being bare-metal machines
  • that currently do not delete their build trees between builds
  • and our git mangling in the scripts are not written to cope with potentially 'dirty' git trees right now

Something I saw locally was if I canceled a Jenkins job whilst it was doing some git work, which left me with a somewhat 'dirty' git repo. Subsequent CI runs then complained that they could not FF that git repo (which seems to be what we have above).

Quick short/mid term fix is for me to tell Jenkins to delete the build trees after each build. This has a number of up/downsides:

  • Gives us a cleaner build env (good...)
  • Will have to download everything each build (not so good)
  • Will therefore take a little longer to run each build (also not so good)

I'll go tick that box now so we can get a more stable metrics CI system, at the cost of speed and bandwidth...

And then I'll nudge a rebuild (or potentially two to get rid of the tree on the first one and do a full clean build on the second)...

Without a '-d' on the 'go get', the script ties to build the repos,
and for repos that do not work under 'go build', this leaves errors
in the CI logs. We don't need a build at this stage as we invoke the
repo specific builds later on.
Also, some CIs do not clean out their build trees between builds, and
thus have repos already present - so add an 'update -u' flag as well.

Fixes: clearcontainers#887

Signed-off-by: Graham Whaley <[email protected]>
@grahamwhaley grahamwhaley force-pushed the 20180205_jenkins_goget branch from 7f0f372 to 8fd35d7 Compare March 27, 2018 09:32
@grahamwhaley
Copy link
Contributor Author

Project is frozen. Work has moved over to: https://github.com/kata-containers
closing.

mcastelino pushed a commit to mcastelino/tests that referenced this pull request Jan 23, 2019
Enable the bare metal slave machine cleanup scripts to be
called from any arch (lifting out the good work done for the
ARM slave cleanups).

Fixes: clearcontainers#888

Signed-off-by: Graham Whaley <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants