Skip to content

Conversation

@deads2k
Copy link
Contributor

@deads2k deads2k commented Aug 8, 2017

The last commit here stops serving the TSB from the apiserver. I'm expecting failures, so this is flushing them out for me.

I've left the types to do separately in case it breaks the patch command used elsewhere to add this to the config.

@openshift-merge-robot openshift-merge-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 8, 2017
@openshift-merge-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k

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 Aug 8, 2017
@deads2k
Copy link
Contributor Author

deads2k commented Aug 8, 2017

@jim-minter can you confirm that these are the only two tests that require it to be enabled?

Extended.[templates] templateservicebroker security test should pass security tests
Extended.[templates] templateservicebroker end-to-end test should pass an end-to-end test

@jim-minter
Copy link
Contributor

@deads2k Correct.

@jim-minter
Copy link
Contributor

don't know if there's more coming but lgtm so far

@deads2k
Copy link
Contributor Author

deads2k commented Aug 8, 2017

don't know if there's more coming but lgtm so far

I've got to start using the templates and move these tests into the normal conformance bucket. Then this will be ready.

@deads2k deads2k changed the title [WIP] stop running the TSB in the main apiserver stop running the TSB in the main apiserver Aug 9, 2017
@deads2k deads2k force-pushed the tsb-11-removeconfig branch 2 times, most recently from fd25d22 to 3afbf2d Compare August 9, 2017 16:56
@deads2k
Copy link
Contributor Author

deads2k commented Aug 9, 2017

@jim-minter please take another look. I've remove more dead code.

This is blocked on #15677 and #15707

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 10, 2017
@jim-minter
Copy link
Contributor

Will it be possible for templateServiceBrokerConfig be removed from pkg/cmd/server/api/v1/types.go? Is this the point at which it can happen? If so, doing a grep -r templateServiceBrokerConfig and fix-up would be good.

@jim-minter
Copy link
Contributor

otherwise still lgtm

@deads2k
Copy link
Contributor Author

deads2k commented Aug 10, 2017

Will it be possible for templateServiceBrokerConfig be removed from pkg/cmd/server/api/v1/types.go? Is this the point at which it can happen? If so, doing a grep -r templateServiceBrokerConfig and fix-up would be good.

It's on my list, but on my list to do separately. I'm not completely sure what will happen to the installer if I do it.

@deads2k deads2k force-pushed the tsb-11-removeconfig branch from 3afbf2d to 7c31b75 Compare August 16, 2017 11:38
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 16, 2017
@deads2k
Copy link
Contributor Author

deads2k commented Aug 16, 2017

prereqs have merged. tagging

@deads2k deads2k added lgtm Indicates that a PR is ready to be merged. and removed do-not-merge labels Aug 16, 2017
@deads2k deads2k force-pushed the tsb-11-removeconfig branch from 7c31b75 to 40878a9 Compare August 16, 2017 13:42
@openshift-merge-robot openshift-merge-robot removed the lgtm Indicates that a PR is ready to be merged. label Aug 16, 2017
@deads2k deads2k force-pushed the tsb-11-removeconfig branch from 40878a9 to b2f1bff Compare August 16, 2017 13:43
@deads2k deads2k added lgtm Indicates that a PR is ready to be merged. retest-not-required labels Aug 16, 2017
@deads2k
Copy link
Contributor Author

deads2k commented Aug 16, 2017

/retest

@openshift-merge-robot
Copy link
Contributor

Automatic merge from submit-queue

@openshift-merge-robot openshift-merge-robot merged commit c164905 into openshift:master Aug 16, 2017
openshift-merge-robot added a commit that referenced this pull request Aug 28, 2017
Automatic merge from submit-queue (batch tested with PRs 15719, 15761)

moves the TSB into its own package

This moves all TSB code into its own package.  The only references we have to this code are here:

```
./pkg/cmd/server/start/start_allinone.go
./pkg/cmd/server/origin/master.go
./test/extended/templates/templateservicebroker_security.go
./test/extended/templates/templateservicebroker_e2e.go
```

We're going to remove the `master.go` reference in #15684, leaving a dependency on building the command and in the e2e tests.

@smarterclayton @sttts Are we ready to jump off the cliff and move to staging repos?  We'd end up with cyclical imports until we sort out openshift/client-go#2, but that looks tractable for an internal solution in 3.7 so we can learn how we want to shape it....
@deads2k deads2k deleted the tsb-11-removeconfig branch January 24, 2018 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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. retest-not-required size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants