Do not check builds/details in build by strategy admission control #6678

csrwng · 2016-01-15T16:26:02Z

Fixes BZ 1298861

This was a regression introduced by #6576.

openshift-bot · 2016-01-15T16:30:46Z

Evaluated for origin testonlyextended up to 1e9cd7f

bparees · 2016-01-15T17:07:54Z

pkg/build/admission/admission.go

@@ -49,6 +49,11 @@ func (a *buildByStrategy) Admit(attr admission.Attributes) error {
 	if resource := attr.GetResource(); resource != buildsResource && resource != buildConfigsResource {
 		return nil
 	}
+	// Explicitly exclude the builds/details subresource because it's only
+	// updating commit info and cannot change build type.
+	if attr.GetResource() == "builds" && attr.GetSubresource() == "details" {


s/"builds"/buildsResource/

bparees · 2016-01-15T17:08:54Z

one nit and lgtm. over to @smarterclayton

bparees · 2016-01-15T17:09:18Z

also fyi test+testonlyextended doesn't make much sense :)

csrwng · 2016-01-15T17:17:43Z

updated.

Should the tag be "testextended" ?

smarterclayton · 2016-01-15T17:24:21Z

Approved, merge when ready

bparees · 2016-01-15T17:24:22Z

"[test][extended:core(builds)]" is what you wanted (but it's also what you got as a result of what you did.. just saying the onlyextended does not actually limit it to only extended tests if you also put a test tag on it)

csrwng · 2016-01-15T17:25:57Z

[test]

liggitt · 2016-01-15T20:53:10Z

@smarterclayton for approval to prevent regression in builders updating build details

liggitt · 2016-01-15T20:53:17Z

nm, saw comment

bparees · 2016-01-15T21:05:26Z

@csrwng those extended test failures look scary, can you take a look?

bparees · 2016-01-15T21:09:20Z

@csrwng this one: https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/8636/consoleFull

going to run it again in the meantime after cleaning up the test tags.

[test]

csrwng · 2016-01-15T22:03:25Z

@bparees after the failure below every test fails. It looks like the server stopped responding after that point. I didn't see this when I ran extended tests earlier today, so I think something must have gone wrong with the vm.


  Expected error:
      <*url.Error | 0xc2087a6180>: {
          Op: "Get",
          URL: "https://172.18.2.235:8443/api/v1/namespaces/extended-test-build-image-source-dx2lb/endpoints?timeout=30s",
          Err: {
              Op: "dial",
              Net: "tcp",
              Addr: {
                  IP: "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xff\xff\xac\x12\x02\xeb",
                  Port: 8443,
                  Zone: "",
              },
              Err: 0x6f,
          },
      }
      Get https://172.18.2.235:8443/api/v1/namespaces/extended-test-build-image-source-dx2lb/endpoints?timeout=30s: dial tcp 172.18.2.235:8443: connection refused
  not to have occurred

bparees · 2016-01-15T22:06:24Z

ok we'll see what the current run yields.

smarterclayton · 2016-01-15T23:56:56Z

[test]

On Fri, Jan 15, 2016 at 6:35 PM, OpenShift Bot notifications@github.com
wrote:

continuous-integration/openshift-jenkins/test FAILURE (
https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/44/) (Extended
Tests: core(builds))

—
Reply to this email directly or view it on GitHub
#6678 (comment).

csrwng · 2016-01-15T23:58:47Z

@smarterclayton extended passed in one of the tests:
https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/8650/consoleFull

and failed with the same exact error above in the other test:

  Expected error:
      <*url.Error | 0xc2087a6180>: {
          Op: "Get",
          URL: "https://172.18.2.235:8443/api/v1/namespaces/extended-test-build-image-source-dx2lb/endpoints?timeout=30s",
          Err: {
              Op: "dial",
              Net: "tcp",
              Addr: {
                  IP: "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xff\xff\xac\x12\x02\xeb",
                  Port: 8443,
                  Zone: "",
              },
              Err: 0x6f,
          },
      }
      Get https://172.18.2.235:8443/api/v1/namespaces/extended-test-build-image-source-dx2lb/endpoints?timeout=30s: dial tcp 172.18.2.235:8443: connection refused
  not to have occurred

csrwng · 2016-01-15T23:59:34Z

I don't think that error is related to this pull

csrwng · 2016-01-16T03:33:24Z

[test]

openshift-bot · 2016-01-16T03:35:20Z

Evaluated for origin test up to 378876c

csrwng · 2016-01-16T03:42:03Z

I wanted to kick off regular tests without the extended tests and then extended tests separately, but it doesn't look like it's letting me do that. It kicked off both again.

openshift-bot · 2016-01-16T03:50:20Z

continuous-integration/openshift-jenkins/test ABORTED (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/53/) (Extended Tests: core(builds))

csrwng · 2016-01-16T04:08:16Z

Started 2 sets of tests manually...
Regular test: https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/54/
Extended only: https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/55/

bparees · 2016-01-16T04:24:38Z

It aggregates all the tags. If you want extended separate you can add the
testonlyextended tag again (Dan educated me this evening) and you'll get a
separate run for that.

Ben Parees | OpenShift
On Jan 15, 2016 10:42 PM, "Cesar Wong" notifications@github.com wrote:

I wanted to kick off regular tests without the extended tests and then
extended tests separately, but it doesn't look like it's letting me do
that. It kicked off both again.

—
Reply to this email directly or view it on GitHub
#6678 (comment).

bparees · 2016-01-16T15:23:51Z

1 for 2 on the test job, 2 for 2 on extended. test failure looks unrelated.
[merge]

openshift-bot · 2016-01-16T15:25:18Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/4647/) (Image: devenv-rhel7_3166)

openshift-bot · 2016-01-16T15:25:19Z

Evaluated for origin merge up to 378876c

Merged by openshift-bot

bparees reviewed Jan 15, 2016
View reviewed changes

Do not check builds/details in build by strategy admission control

378876c

csrwng force-pushed the fix_build_details branch from 1e9cd7f to 378876c Compare January 15, 2016 17:17

liggitt added this to the 1.1.1 milestone Jan 15, 2016

liggitt added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 15, 2016

openshift-bot pushed a commit that referenced this pull request Jan 16, 2016

Merge pull request #6678 from csrwng/fix_build_details

d95ec08

Merged by openshift-bot

openshift-bot merged commit d95ec08 into openshift:master Jan 16, 2016

csrwng deleted the fix_build_details branch January 27, 2016 14:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not check builds/details in build by strategy admission control #6678

Do not check builds/details in build by strategy admission control #6678

csrwng commented Jan 15, 2016

openshift-bot commented Jan 15, 2016

bparees Jan 15, 2016

bparees commented Jan 15, 2016

bparees commented Jan 15, 2016

csrwng commented Jan 15, 2016

smarterclayton commented Jan 15, 2016

bparees commented Jan 15, 2016

csrwng commented Jan 15, 2016

liggitt commented Jan 15, 2016

liggitt commented Jan 15, 2016

bparees commented Jan 15, 2016

bparees commented Jan 15, 2016

csrwng commented Jan 15, 2016

bparees commented Jan 15, 2016

smarterclayton commented Jan 15, 2016

csrwng commented Jan 15, 2016

csrwng commented Jan 15, 2016

csrwng commented Jan 16, 2016

openshift-bot commented Jan 16, 2016

csrwng commented Jan 16, 2016

openshift-bot commented Jan 16, 2016

csrwng commented Jan 16, 2016

bparees commented Jan 16, 2016

bparees commented Jan 16, 2016

openshift-bot commented Jan 16, 2016

openshift-bot commented Jan 16, 2016

Do not check builds/details in build by strategy admission control #6678

Do not check builds/details in build by strategy admission control #6678

Conversation

csrwng commented Jan 15, 2016

openshift-bot commented Jan 15, 2016

bparees Jan 15, 2016

Choose a reason for hiding this comment

bparees commented Jan 15, 2016

bparees commented Jan 15, 2016

csrwng commented Jan 15, 2016

smarterclayton commented Jan 15, 2016

bparees commented Jan 15, 2016

csrwng commented Jan 15, 2016

liggitt commented Jan 15, 2016

liggitt commented Jan 15, 2016

bparees commented Jan 15, 2016

bparees commented Jan 15, 2016

csrwng commented Jan 15, 2016

bparees commented Jan 15, 2016

smarterclayton commented Jan 15, 2016

csrwng commented Jan 15, 2016

csrwng commented Jan 15, 2016

csrwng commented Jan 16, 2016

openshift-bot commented Jan 16, 2016

csrwng commented Jan 16, 2016

openshift-bot commented Jan 16, 2016

csrwng commented Jan 16, 2016

bparees commented Jan 16, 2016

bparees commented Jan 16, 2016

openshift-bot commented Jan 16, 2016

openshift-bot commented Jan 16, 2016