Skip to content

Comments

Merge Service Catalog v 0.0.21 to Origin#16457

Merged
openshift-merge-robot merged 4 commits intoopenshift:masterfrom
jboyd01:master
Sep 22, 2017
Merged

Merge Service Catalog v 0.0.21 to Origin#16457
openshift-merge-robot merged 4 commits intoopenshift:masterfrom
jboyd01:master

Conversation

@jboyd01
Copy link

@jboyd01 jboyd01 commented Sep 20, 2017

Merge Service Catalog v 0.0.21 to Origin

@openshift-ci-robot openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Sep 20, 2017
@deads2k
Copy link
Contributor

deads2k commented Sep 20, 2017

/assign pmorie
/unassign

@openshift-ci-robot openshift-ci-robot assigned pmorie and unassigned deads2k Sep 20, 2017
@jboyd01 jboyd01 requested a review from pmorie September 20, 2017 19:50
@jboyd01
Copy link
Author

jboyd01 commented Sep 20, 2017

passes Service Catalog e2e locally.
/retest

@jboyd01
Copy link
Author

jboyd01 commented Sep 20, 2017

/retest

1 similar comment
@pmorie
Copy link
Contributor

pmorie commented Sep 20, 2017

/retest

@pmorie
Copy link
Contributor

pmorie commented Sep 20, 2017

@jboyd01

Looks like the example template needs to be updated with the new admission controller and bindata regenerated.

@jboyd01
Copy link
Author

jboyd01 commented Sep 21, 2017

@pmorie yes, I did miss a new Admission Controller. I've added it and regen'd bindata. Teach me to fish - - where'd you see this error that indicated the missing controller?

@spadgett
Copy link
Member

spadgett commented Sep 21, 2017

Are we going to add the feature gate flag for originating identity to the service catalog template?

@jwmatthews jwmatthews mentioned this pull request Sep 21, 2017
Merged
@pmorie
Copy link
Contributor

pmorie commented Sep 21, 2017

@jboyd01 I didn't see an error that indicated it - I knew to look for it because it was in the 0.0.21 release.

Also, we need to ensure that the feature flag for originating identity is enabled in the template used by oc cluster up

@spadgett spadgett mentioned this pull request Sep 21, 2017
Merged
Jay Boyd added 4 commits September 21, 2017 10:44
Squashed 'cmd/service-catalog/go/src/github.com/kubernetes-incubator/…
8c6f496 
…service-catalog/' changes from ae6b643caf..06b897d198

06b897d198 origin build: add origin tooling
092d7f8 Fix typos and resource names in walkthrough e2e logs (openshift#1237)
d25bd11 Archive the old agenda doc, link to new one (openshift#1243)
6192d14 fix lint errors (openshift#1242)
d103dad Fix lint errors and regenerate openapi (openshift#1238)
e9328d3 Broker Relist (openshift#1183)
b0f3222 Correct the reasons and messages set on the ready condition during async polling (openshift#1235)
d2bb82f Re-enable the href checker (openshift#1232)
2c29654 Use feature gates in controller-manager (openshift#1231)
699eab9 switch build to go1.9 (openshift#1155)
7529ed8 broker resource secret authorization checking (openshift#1186)
50d9bdf v0.0.20 chart updates (openshift#1228)
REVERT: ae6b643caf Use oc adm instead of oadm which might not exist in various installations.
REVERT: 66a4eb2a2c Update instructions... will remove once documented elsewhere
REVERT: 1b704d1530 replace build context setup with init containers
REVERT: ee4df18c7f hack/lib: dedup os::util::host_platform and os::build::host_platform
REVERT: 1cd6dfa998 origin: Switch out owners to Red Hatters
REVERT: 664f4d318f Add instructions for syncing repos
REVERT: 2f2cdd546b origin-build: delete files with colon in them
REVERT: cdf8b12848 origin-build: don't build user-broker
REVERT: ebfede9056 origin build: add _output to .gitignore
REVERT: 55412c7e3d origin build: make build-go and build-cross work
REVERT: 68c74ff4ae origin build: modify hard coded path
REVERT: 3d41a217f6 origin build: add origin tooling

git-subtree-dir: cmd/service-catalog/go/src/github.com/kubernetes-incubator/service-catalog
git-subtree-split: 06b897d1988a5a3c035c5a971c15b97cbc732918
Merge version v0.0.21 of Service Catalog from https://github.com/open…
5a87f76 
…shift/service-catalog:v0.0.21+origin
catalog: added new admission controller BrokerAuthSarCheck and update…
4ccfe1c 
…d bindata
catalog: enable OriginatingIdentity
2570865
@jboyd01
Copy link
Author

jboyd01 commented Sep 21, 2017

ok, should be ironed out this time. The BrokerAuthSarCheck admission controller is enabled as is OriginatingIdentity.

@pmorie
Copy link
Contributor

pmorie commented Sep 21, 2017

/test extended_conformance_install_update

@jboyd01
Copy link
Author

jboyd01 commented Sep 21, 2017

extended_networking_minimal failing with no route to host....
/test extended_networking_minimal

@jboyd01
Copy link
Author

jboyd01 commented Sep 21, 2017

/test extended_networking_minimal

1 similar comment
@pmorie
Copy link
Contributor

pmorie commented Sep 21, 2017

/test extended_networking_minimal

@pmorie
Copy link
Contributor

pmorie commented Sep 22, 2017

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 22, 2017
@pmorie
Copy link
Contributor

pmorie commented Sep 22, 2017 •
edited
Loading

@bparees can you approve this one?

@bparees
Copy link
Contributor

bparees commented Sep 22, 2017

i assume i'm only reviewing the template yaml changes, so that part lgtm.

@gabemontero
Copy link
Contributor

gabemontero commented Sep 22, 2017

need an api review as well, right? ... at least the label has been added ... bump @openshift/api-review

@pmorie
Copy link
Contributor

pmorie commented Sep 22, 2017

@gabemontero I don't understand why the api-review label is on there - on our other rebase PRs for catalog it's also been added but it isn't clear why. I imagine that it might be because the rebase contains API files - which are not up for review since they're coming from upstream.

@deads2k
Copy link
Contributor

deads2k commented Sep 22, 2017

need an api review as well, right? ... at least the label has been added ... bump @openshift/api-review

It's trigger on types.go changes. This one is a false positive because it's a fake repo inside of our real repo.

@gabemontero
Copy link
Contributor

gabemontero commented Sep 22, 2017

thanks @deads2k

with passing tests, let's remove the label and merge this puppy then :-) (assuming that is what is blocking its presence on https://origin-sq-status-ci.svc.ci.openshift.org/#/queue)

@deads2k
Copy link
Contributor

deads2k commented Sep 22, 2017

thanks @deads2k

with passing tests, let's remove the label and merge this puppy then :-) (assuming that is what is blocking its presence on https://origin-sq-status-ci.svc.ci.openshift.org/#/queue)

Different flag. This pull touches samples and something in oc cluster up, so its waiting for approval on changing those things.

@deads2k
Copy link
Contributor

deads2k commented Sep 22, 2017

Different flag. This pull touches samples and something in oc cluster up, so its waiting for approval on changing those things.

@gabemontero this commit in particular catalog: enable OriginatingIdentity. @bparees could do it if he knows that its doing.

@bparees
Copy link
Contributor

bparees commented Sep 22, 2017

i verbally approved that already: #16457 (comment)

but sure, labels updated. they don't actually block merging anyway as far as i know.

@deads2k
Copy link
Contributor

deads2k commented Sep 22, 2017

i verbally approved that already: #16457 (comment)

but sure, labels updated. they don't actually block merging anyway as far as i know.

oh, you need to say

/approve

to give the right label

@openshift-merge-robot
Copy link
Contributor

openshift-merge-robot commented Sep 22, 2017

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, jboyd01, pmorie

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 22, 2017
@deads2k
Copy link
Contributor

deads2k commented Sep 22, 2017

bparees added api-approved and

api-approval doesn't block merge.

@bparees
Copy link
Contributor

bparees commented Sep 22, 2017

I was only approving the template change so i did not want to use /approve.

i haven't reviewed any of the other commits, nor do i intend to.

@openshift-merge-robot
Copy link
Contributor

openshift-merge-robot commented Sep 22, 2017

Automatic merge from submit-queue (batch tested with PRs 16452, 16478, 16457)

1 similar comment
@openshift-merge-robot
Copy link
Contributor

openshift-merge-robot commented Sep 22, 2017

Automatic merge from submit-queue (batch tested with PRs 16452, 16478, 16457)

@openshift-merge-robot openshift-merge-robot merged commit b3912c0 into openshift:master Sep 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pmorie pmorie Awaiting requested review from pmorie

Assignees

@pmorie pmorie

@smarterclayton smarterclayton

@bparees bparees

Labels

api-approved approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. needs-api-review size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@jboyd01 @deads2k @pmorie @spadgett @bparees @gabemontero @openshift-merge-robot @smarterclayton @openshift-ci-robot