openshift / jenkins-plugin

Apache License 2.0
81 stars 50 forks source link

Fix NPE when no arguments supplied on exec build step #107

Closed jupierce closed 7 years ago

jupierce commented 7 years ago

Fixes NPE reported in https://bugzilla.redhat.com/show_bug.cgi?id=1390491 Will also be adding new test to origin.

ptal @gabemontero @bparees

bparees commented 7 years ago

ah yes, the classic null foreach NPE. lgtm.

gabemontero commented 7 years ago

IGTM as well ... I believe with https://github.com/openshift/origin/pull/11711 merged the PR test should be working (unless there is an issue with the dev ami perhaps) ... anyway, I'll let the test job play out before hitting the merge button.

gabemontero commented 7 years ago

The job is still running but looking at the console I'm still seeing the permission issue wrt default service account.

Exiting "OpenShift Exec" unsuccessfully: Unable to find pod: ruby-hello-world-1-2ung8
com.openshift.restclient.authorization.ResourceForbiddenException: User "system:serviceaccount:extended-test-jenkins-plugin-5z92h-e7phl-jenkins:default" cannot get pods in project "extended-test-jenkins-plugin-5z92h-e7phl" User "system:serviceaccount:extended-test-jenkins-plugin-5z92h-e7phl-jenkins:default" cannot get pods in project "extended-test-jenkins-plugin-5z92h-e7phl"
    at com.openshift.internal.restclient.okhttp.ResponseCodeInterceptor.createOpenShiftException(ResponseCodeInterceptor.java:106)
    at com.openshift.internal.restclient.okhttp.ResponseCodeInterceptor.intercept(ResponseCodeInterceptor.java:65)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:92)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:67)
    at okhttp3.RealCall.getResponseWithInterceptorChain(RealCall.java:170)
    at okhttp3.RealCall.execute(RealCall.java:60)
    at com.openshift.internal.restclient.DefaultClient.execute(DefaultClient.java:217)
    at com.openshift.internal.restclient.DefaultClient.execute(DefaultClient.java:194)
    at com.openshift.internal.restclient.DefaultClient.execute(DefaultClient.java:183)
    at com.openshift.internal.restclient.DefaultClient.get(DefaultClient.java:291)
    at com.openshift.jenkins.plugins.pipeline.model.IOpenShiftExec.coreLogic(IOpenShiftExec.java:72)
    at com.openshift.jenkins.plugins.pipeline.model.IOpenShiftPlugin.doItCore(IOpenShiftPlugin.java:303)
    at com.openshift.jenkins.plugins.pipeline.dsl.OpenShiftExecExecution.run(OpenShiftExecExecution.java:43)
    at com.openshift.jenkins.plugins.pipeline.dsl.OpenShiftExecExecution.run(OpenShiftExecExecution.java:19)
    at org.jenkinsci.plugins.workflow.steps.AbstractSynchronousNonBlockingStepExecution$1$1.call(AbstractSynchronousNonBlockingStepExecution.java:49)
    at hudson.security.ACL.impersonate(ACL.java:213)
    at org.jenkinsci.plugins.workflow.steps.AbstractSynchronousNonBlockingStepExecution$1.run(AbstractSynchronousNonBlockingStepExecution.java:47)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:745)

The devenv ami jobs have been clean for a bit, but perhaps they just having picked up the merged template change yet.

I'll merge in a few. That said - @jupierce - could you give the console log a once over and report if you see something unexpected amiss? thanks

gabemontero commented 7 years ago

Btw, I confirmed that the devenv ami had not picked up @jupierce 's latest extended test changes that leveraged the standard ephemeral template:

STEP: kick off the build for the jenkins ephermeral and application templates
tag openshift/jenkins-plugin-snapshot-test:latest hex id sha256:41697ad915a394e9c6869d28fa63b0e6bb7e48a272cb5a28f1fbf69e8cdd9932 Nov  3 12:20:37.381: INFO: Running 'oc new-app --config=/tmp/extended-test-jenkins-plugin-5z92h-awgse-user.kubeconfig --namespace=extended-test-jenkins-plugin-5z92h-awgse-jenkins /data/src/github.com/openshift/origin/test/extended/testdata/jenkins-ephemeral-template-test-new-plugin.json'

Unless we have a new fix coming, I'll try out a test PR in the next day or so to confirm that those new changes are getting picked up by the PR tester.