-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
fix sync plugin PR tester #13577
fix sync plugin PR tester #13577
Conversation
sorry ... @openshift/devex PTAL |
[testextended][extended:core(Pipeline with env vars)] |
Evaluated for origin testextended up to 6d21312 |
@@ -200,7 +201,7 @@ var _ = g.Describe("[builds][Slow] openshift pipeline build", func() { | |||
br.AssertSuccess() | |||
|
|||
g.By("confirm all the log annotations are there") | |||
_, err = jenkins.ProcessLogURLAnnotations(oc, br) | |||
err = jenkins.ProcessLogURLAnnotations(oc, br, !useSnapshotImage) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this means the PR tester won't ever confirm we haven't broken the log annotations.... can't we make this instead discriminate on the jenkins image version being tested?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it won't verify the blue ocean annotation, but will the other 2
certainly long term there's the argument to move all the PR testing to v2; but in the past I've deferred because:
a) until the recent v1 deprecation decision, it seemed good to have some level of v1 testing being done
b) in the past when I tried switching to PR test image to extend v2, the docker build blew up (precise errors escape me but as I recall it wasn't a simple sort of fix)
My preference would be to defer on blueocean annotation verification for the PR testing for now, and then when we actually start making the changes to officially deprecate v1, circle back to switching all 3 plugin PR testers to v2, and then sort through any issues that might exist there.
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we don't have an easy way to tell which jenkins image we're testing, yeah i'm ok w/ just not doing that testing during PR testing for now and circling back to re-enable that check after we deprecate v1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool on the deferring addressing when we deprecate v1 ... that said, the useSnapshotImage
is in fact how we tell which image we are using (maybe I'm misinterpreting what you wrote?). That variable being true means this is a PR test and hence, for the time being, is based on jenkins v1. So ProcessLogURLAnnotations
will only check the raw and chrome wrappred log urls if we are PR testing. For our overnight extended test runs, we'll know via useSnapshotImage
that it is our vanilla jenkins v2 image and we'll also check the blue ocean url.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah. I assumed we were testing both versions and the usesnapshot was just telling us whether to use the local image for each, or pull them.
Seems like we might as well change the PR test to use the v2 image at this point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I figured out the v2 image hiccup and am crafting PRs to switch to v2 for the PR testers ... closed this PR.
continuous-integration/openshift-jenkins/testextended SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin_extended/106/) (Base Commit: 2130ec6) (Extended Tests: core(Pipeline with env vars)) |
continuous-integration/openshift-jenkins/merge Waiting: You are in the build queue at position: 1 |
During the conformance install suite, we appear to have hit k8s flake #12072 Those were the only failures. @bparees please repost the merge comment at your convenience - thanks |
Evaluated for origin merge up to 6d21312 |
@openshift/devexp ptal
my commit 2130ec6 from earlier today broke the jenkins v1 testing done in our plugin PR testing