openshift / jenkins-plugin

Apache License 2.0
81 stars 50 forks source link

allow start of pipeline strategy builds #118

Closed gabemontero closed 7 years ago

gabemontero commented 7 years ago

fixes https://github.com/openshift/jenkins-plugin/issues/117

@csrwng @bparees FYI

(note, on the file diff, another case where ?w=1 on the file diff URL looks to be useful)

gabemontero commented 7 years ago

The test job https://ci.openshift.redhat.com/jenkins/job/test-openshift-pipeline-plugin/119/consoleFull actually passed:

Ran 11 of 573 Specs in 3663.079 seconds
SUCCESS! -- 11 Passed | 0 Failed | 0 Pending | 562 Skipped Nov 21 19:18:57.552: INFO: Error running cluster/log-dump.sh: fork/exec /data/src/github.com/openshift/origin/vendor/k8s.io

But the job is reporting a failure for a reason I am not entirely gleaming yet. I'll solicit some help on a separate thread for that piece.

gabemontero commented 7 years ago

Ok the same objects are returned on the multiple calls. We aren't getting duplicate logging. But yes i can see about prefacing both calls with if stop == null and confirm the behavior. Assuming it works ok i will push that change.

On Mon, Nov 21, 2016 at 8:40 PM Ben Parees notifications@github.com wrote:

@bparees commented on this pull request.

In src/main/java/com/openshift/jenkins/plugins/pipeline/model/IOpenShiftBuilder.java https://github.com/openshift/jenkins-plugin/pull/118:

+

  • // get internal OS Java REST Client error if access pod logs while bld is in Pending state
  • // instead of Running, Complete, or Failed +
  • IStoppable stop = null; +
  • final AtomicBoolean needToFollow = new AtomicBoolean(follow); +
  • while (System.currentTimeMillis() < (startTime + wait)) {
  • bld = client.get(ResourceKind.BUILD, bldId, getNamespace(overrides));
  • bldState = bld.getStatus();
  • if (Boolean.parseBoolean(getVerbose(overrides)))
  • listener.getLogger().println("\nOpenShiftBuilder bld state: " + bldState); +
  • if (needToFollow.compareAndSet(true, false)) {
  • stop = this.getBuildPodLogs(client, bldId, overrides, chatty, listener, needToFollow);

I'm referring to the fact that that you go through this loop multiple times as you wait for the build to finish.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/openshift/jenkins-plugin/pull/118, or mute the thread https://github.com/notifications/unsubscribe-auth/ADbadHwj9nO9tsEsEBrR8DpjJQnYJrHqks5rAkgggaJpZM4K4rhR .

gabemontero commented 7 years ago

I did push an update, adding a break in the new getBuildPodLogs method after finding the pod.

csrwng commented 7 years ago

LGTM

gabemontero commented 7 years ago

thanks gentlemen

gabemontero commented 7 years ago

@csrwng - if you like, there is a built plugin with this change at https://ci.openshift.redhat.com/jenkins/job/openshift-pipeline-plugin/33/s3/download/openshift-pipeline.hpi

@bparees - would you like to have a new version of the plugin cut for this change now, or hold (perhaps until post 3.4) ?

bparees commented 7 years ago

@bparees - would you like to have a new version of the plugin cut for this change now, or hold (perhaps until post 3.4) ?

up to you and how risky you feel it is. we have runway to get it in.

gabemontero commented 7 years ago

Feels low risk. I'll get started on cutting a release when I get back.

On Tue, Nov 22, 2016 at 10:10 AM Ben Parees notifications@github.com wrote:

@bparees https://github.com/bparees - would you like to have a new version of the plugin cut for this change now, or hold (perhaps until post 3.4) ?

up to you and how risky you feel it is. we have runway to get it in.

— You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub https://github.com/openshift/jenkins-plugin/pull/118#issuecomment-262265746, or mute the thread https://github.com/notifications/unsubscribe-auth/ADbadJ4m0BJgObyfheKrcqiOIvHAFp5Bks5rAwXHgaJpZM4K4rhR .