leverage new source-to-image API around git clone spec validation/correction #5863
Conversation
| args: []string{".", testDir, "git://server/repo.git"}, | ||
| repos: util.StringList([]string{".", testDir, "git://server/repo.git"}), | ||
| args: []string{".", testDir, "git://github.com/openshift/origin.git"}, | ||
| repos: util.StringList([]string{".", testDir, "git://github.com/openshift/origin.git"}), |
There was a problem hiding this comment.
if this test is going to end up cloning this repo, we should pick a smaller one. origin is huge.
gabemontero
force-pushed
the
bugzilla1268478-2
branch
from
3645267
to
5e1cd15
Compare
|
responses to comments pushed |
|
[test] |
|
fyi - the test failure above was e2e-docker ... failure to connect to a container ... seems unrelated to this change. |
gabemontero
force-pushed
the
bugzilla1268478-2
branch
from
5e1cd15
to
54c7c32
Compare
|
update for forcePull to pullPolicy pushed |
|
continuous-integration/openshift-jenkins/test Waiting: Determining build queue position |
| } else { | ||
| config.PreviousImagePullPolicy = s2iapi.PullIfNotPresent | ||
| } | ||
|
|
There was a problem hiding this comment.
shouldn't we also stop setting ForcePull above?
There was a problem hiding this comment.
(and make this if be based on s.build.Spec.Strategy.SourceStrategy.ForcePull)
|
one nit and lgtm. |
gabemontero
force-pushed
the
bugzilla1268478-2
branch
from
54c7c32
to
326eb43
Compare
|
update for force pull / pull policy nit pushed |
|
lgtm, but this would probably be a good time to run your extended tests that exercise the forcepull flag :) |
|
continuous-integration/openshift-jenkins/test Running (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/7125/) (Extended Tests: core(forcepull), forcepull) |
|
hold on merge, will review this tomorrow
|
gabemontero
force-pushed
the
bugzilla1268478-2
branch
from
326eb43
to
32db2f0
Compare
|
continuous-integration/openshift-jenkins/test Running (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/7132/) (Extended Tests: core(forcepull), forcepull) |
|
@bparees @mfojtik - the forcepull extended test actually passed. I'll paste a copule of snippets from the jenkins console:
The failure stems from some hiccups with rsync in copying the logs files. Are we still holding on merge based on @mfojtik 's request ? |
| config.PreviousImagePullPolicy = s2iapi.PullAlways | ||
| config.BuilderPullPolicy = s2iapi.PullAlways | ||
| } else { | ||
| glog.V(4).Infof("With force pull false, setting policies to %s", s2iapi.PullAlways) |
There was a problem hiding this comment.
ahh ... ok, just pushed
gabemontero
force-pushed
the
bugzilla1268478-2
branch
from
32db2f0
to
891fc21
Compare
|
Evaluated for origin testonlyextended up to 891fc21 |
|
@mfojtik if you have comments we'll handle them in a follow up. |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/4012/) (Image: devenv-rhel7_2709) |
|
Evaluated for origin merge up to 891fc21 |
|
continuous-integration/openshift-jenkins/test Running (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/7138/) (Extended Tests: core(forcepull)) |
|
hmmm - same scenario ... tests passed, rsync of logs failed ... also, the test infrastructure is trying to call forcepull.sh (and it appears as if the extended tests are still running with a forcepull focus ... perhaps the prior forcepull only run has left a bit set somewhere). The message after only running the forcepull test:
forcepull.sh used to exist, but was deleted as part of one of the extended test refactors; i would have to dig some more to see if the missing forcepull.sh is related to the rsync failures, and if it makes sense to add a forcepull.sh back in. I'm thinking of trying a full extended test run manually first (perhaps unset the run only forcepull bit if my theory is correct). @bparees thoughts when you get the chance |
|
@gabemontero it's trying to call forcepull.sh because of your comment above where you named it as the extended test to run. the bot aggregrates all the test tags. delete that comment and it'll stop trying to call forcepull.sh. |
|
ok ... so my theory was correct - didn't think about the delete comment though - thx |
|
[test] |
|
also the rsync of the logs didn't fail in general, it just didn't find certain directories because you didn't run those tests. that isn't causing the build failure, the build failure is because forcepull.sh wasn't found, so that's a failure code. So i think once you remove your extended:forcepull comment, and then add another test tag so it reruns, you'll get a clean run. |
|
@gabemontero i'm not sure how test and testonlyextended interact since they are basically opposites. you probably want: (with brackets instead of pipes of course. just trying to avoid messing up your PR) |
|
or you can just watch it and see what happens and then we'll learn something. |
|
[test] |
|
Evaluated for origin test up to 891fc21 |
|
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/7141/) |
Merged by openshift-bot
@bparees @csrwng revision 2 of the origin side changes for leveraging the improved git clone spec validation.
PTAL - thx