Include namespaced brokers in svcat get brokers #2227

carolynvs · 2018-07-23T17:30:37Z

$ svcat get brokers
     NAME      NAMESPACE                              URL                              STATUS
+------------+-----------+-----------------------------------------------------------+--------+
  minibroker               http://minibroker-minibroker.minibroker.svc.cluster.local   Ready

$ svcat get brokers -n minibroker
      NAME       NAMESPACE                               URL                              STATUS
+--------------+------------+-----------------------------------------------------------+--------+
  minibroker                  http://minibroker-minibroker.minibroker.svc.cluster.local   Ready
  myminibroker   minibroker   http://minibroker-minibroker.minibroker.svc.cluster.local   Ready

$ svcat get brokers --scope=cluster
     NAME      NAMESPACE                              URL                              STATUS
+------------+-----------+-----------------------------------------------------------+--------+
  minibroker               http://minibroker-minibroker.minibroker.svc.cluster.local   Ready

$ svcat get brokers -n minibroker --scope=namespace
      NAME       NAMESPACE                               URL                              STATUS
+--------------+------------+-----------------------------------------------------------+--------+
  myminibroker   minibroker   http://minibroker-minibroker.minibroker.svc.cluster.local   Ready

$ svcat get brokers --all-namespaces
         NAME          NAMESPACE                               URL                              STATUS
+--------------------+------------+-----------------------------------------------------------+--------+
  minibroker                        http://minibroker-minibroker.minibroker.svc.cluster.local   Ready
  another-minibroker   default      http://minibroker-minibroker.minibroker.svc.cluster.local
  myminibroker         minibroker   http://minibroker-minibroker.minibroker.svc.cluster.local   Ready

Preview of the docs available at: https://deploy-preview-2227--svc-cat.netlify.com/docs/cli/#find-brokers-installed-on-the-cluster

TODO:

Add unit tests for RetrieveBrokers call
Add unit tests for brokers svcat command
Update docs with new behavior and flags

Closes #2218.

jboyd01 · 2018-07-23T18:16:13Z

cmd/svcat/command/scoped.go

+	"github.com/spf13/pflag"
+)
+
+// HasScopedFlags represents a command that can be scoped to can be scoped


duplicate words "can be scoped to"

carolynvs · 2018-07-23T19:45:15Z

/hold

MHBauer · 2018-07-26T18:20:29Z

/retest

MHBauer · 2018-07-26T18:39:52Z

@carolynvs I dug the details out of gcs that was pointed out to me


I0726 17:16:25.927] === RUN   TestUpdateServiceInstanceNewDashboardResponse
I0726 17:16:25.928] === RUN   TestUpdateServiceInstanceNewDashboardResponse/alpha_features_enabled
W0726 17:16:37.472] 2018-07-26 17:16:37.471528 W | wal: sync duration of 1.359038397s, expected less than 1s
W0726 17:16:38.140] 2018-07-26 17:16:38.139553 W | etcdserver: apply entries took too long [368.242626ms for 1 entries]
W0726 17:16:38.140] 2018-07-26 17:16:38.139640 W | etcdserver: avoid queries with large range/delete range!
W0726 17:17:31.257] E0726 17:17:31.256098       1 controller_instance.go:1685] ServiceInstance "test-namespace/test-instance" v842: Failed to update references: Timeout: request did not complete within allowed duration
I0726 17:17:34.354] === RUN   TestUpdateServiceInstanceNewDashboardResponse/alpha_feature_disabled
W0726 17:17:37.052] E0726 17:17:37.051306       1 controller_instance.go:1685] ServiceInstance "test-namespace/test-instance" v842: Failed to update references: Operation cannot be fulfilled on serviceinstances.servicecatalog.k8s.io "test-instance": the object has been modified; please apply your changes to the latest version and try again
W0726 17:17:41.405] 2018-07-26 17:17:41.405036 W | wal: sync duration of 1.599916754s, expected less than 1s
W0726 17:17:53.538] 2018-07-26 17:17:53.537639 W | etcdserver: apply entries took too long [276.964261ms for 1 entries]
W0726 17:17:53.538] 2018-07-26 17:17:53.537740 W | etcdserver: avoid queries with large range/delete range!
W0726 17:17:56.325] 2018-07-26 17:17:56.324981 W | wal: sync duration of 1.114856277s, expected less than 1s
W0726 17:18:00.381] 2018-07-26 17:18:00.381358 W | wal: sync duration of 1.122258947s, expected less than 1s
W0726 17:18:03.780] 2018-07-26 17:18:03.779714 W | wal: sync duration of 1.416115309s, expected less than 1s
I0726 17:18:08.411] --- FAIL: TestUpdateServiceInstanceNewDashboardResponse (102.58s)
I0726 17:18:08.411]     --- FAIL: TestUpdateServiceInstanceNewDashboardResponse/alpha_features_enabled (68.53s)
I0726 17:18:08.411]     	framework.go:111: Server started on port 36265
I0726 17:18:08.411]     	framework.go:142: Test client will use API Server URL of https://localhost:36265
I0726 17:18:08.412]     	controller_test.go:790: controller start
I0726 17:18:08.412]     	controller_test.go:1200: error waiting for instance to become ready: timed out waiting for the condition
I0726 17:18:08.412]     --- PASS: TestUpdateServiceInstanceNewDashboardResponse/alpha_feature_disabled (34.06s)
I0726 17:18:08.412]     	framework.go:111: Server started on port 43229
I0726 17:18:08.412]     	framework.go:142: Test client will use API Server URL of https://localhost:43229
I0726 17:18:08.412]     	controller_test.go:790: controller start
I0726 17:18:08.412]     	framework.go:116: Shutting down server on port: 43229

carolynvs · 2018-07-26T21:42:16Z

@MHBauer I didn't make any changes that would have affected those tests. The only non-svcat changes were adding methods like GetName() to SerivceBroker and ClusterServiceBroker.

I'm not quite sure if this is something that is a problem with just my PR that I need to try to figure out how to see what gcp is doing, or if other PRs are also rando hitting the the same problem? I may just wait and see if others hit this before deciding that I caused this, and need to change the PR to fix it. 😀

MHBauer · 2018-07-26T22:32:19Z

could be a flake then. oh noes.

carolynvs · 2018-07-27T15:04:46Z

@MHBauer I'll open an issue for that, seems to be a flake as I just pushed up a doc commit and now the test is passing. 😝

jichenjc

LGTM

jichenjc

LGTM

jichenjc

LGTM

jeremyrickard · 2018-07-30T16:55:17Z

/lgtm

jberkhahn · 2018-07-31T00:00:27Z

/lgtm
/approve

k8s-ci-robot · 2018-07-31T00:00:40Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jberkhahn

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [jberkhahn]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

carolynvs added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 23, 2018

carolynvs requested review from jboyd01 and jberkhahn July 23, 2018 17:30

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 23, 2018

k8s-ci-robot requested a review from jeremyrickard July 23, 2018 17:30

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 23, 2018

carolynvs force-pushed the list-ns-brokers branch 2 times, most recently from e5fe813 to 3e81cee Compare July 23, 2018 18:09

jboyd01 reviewed Jul 23, 2018

View reviewed changes

carolynvs force-pushed the list-ns-brokers branch from 7839be5 to 897c7cb Compare July 23, 2018 18:39

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 23, 2018

carolynvs force-pushed the list-ns-brokers branch from 8f321b3 to d7b2e07 Compare July 26, 2018 17:03

carolynvs force-pushed the list-ns-brokers branch from 835d7ea to 114e194 Compare July 27, 2018 14:38

carolynvs removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 27, 2018

Retrieve brokers by scope

def9575

carolynvs force-pushed the list-ns-brokers branch from 114e194 to 0354775 Compare July 27, 2018 15:26

carolynvs-msft added 7 commits July 27, 2018 10:31

Add Broker interface

306cfb9

Apply Namespaced interface to svcat get broker command

b53735a

List namespaced brokers with svcat

b8e3b9d

Add ns-scoped broker testdata

2109283

Sync generated files with new broker flags

f6144ff

Sync golden files

ebe020a

Update generated fake

4ad2d93

carolynvs-msft added 3 commits July 27, 2018 10:33

Update unit tests for RetrieveBrokers to validate ns-scoped brokers too

5811fed

Add unit tests for get brokers command

30815ee

Document listing namespaced brokers with svcat

dbc065a

carolynvs force-pushed the list-ns-brokers branch from 0354775 to dbc065a Compare July 27, 2018 15:34

carolynvs mentioned this pull request Jul 27, 2018

Add unit test for cmd/svcat/broker/get_cmd.go #2178

Closed

jichenjc reviewed Jul 30, 2018

View reviewed changes

k8s-ci-robot assigned jeremyrickard Jul 30, 2018

k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 30, 2018

jeremyrickard added the LGTM1 label Jul 30, 2018

k8s-ci-robot assigned jberkhahn Jul 31, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 31, 2018

jboyd01 added the LGTM2 label Jul 31, 2018

k8s-ci-robot merged commit 46a8fb0 into kubernetes-retired:master Jul 31, 2018

MHBauer mentioned this pull request Aug 2, 2018

v0.1.28 Release #2256

Merged

carolynvs deleted the list-ns-brokers branch October 4, 2018 21:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Include namespaced brokers in svcat get brokers #2227

Include namespaced brokers in svcat get brokers #2227

carolynvs commented Jul 23, 2018 •

edited

Loading

jboyd01 Jul 23, 2018

carolynvs commented Jul 23, 2018

MHBauer commented Jul 26, 2018

MHBauer commented Jul 26, 2018

carolynvs commented Jul 26, 2018

MHBauer commented Jul 26, 2018

carolynvs commented Jul 27, 2018

jichenjc left a comment

jichenjc left a comment

jichenjc left a comment

jeremyrickard commented Jul 30, 2018

jberkhahn commented Jul 31, 2018

k8s-ci-robot commented Jul 31, 2018

Include namespaced brokers in svcat get brokers #2227

Include namespaced brokers in svcat get brokers #2227

Conversation

carolynvs commented Jul 23, 2018 • edited Loading

jboyd01 Jul 23, 2018

Choose a reason for hiding this comment

carolynvs commented Jul 23, 2018

MHBauer commented Jul 26, 2018

MHBauer commented Jul 26, 2018

carolynvs commented Jul 26, 2018

MHBauer commented Jul 26, 2018

carolynvs commented Jul 27, 2018

jichenjc left a comment

Choose a reason for hiding this comment

jichenjc left a comment

Choose a reason for hiding this comment

jichenjc left a comment

Choose a reason for hiding this comment

jeremyrickard commented Jul 30, 2018

jberkhahn commented Jul 31, 2018

k8s-ci-robot commented Jul 31, 2018

carolynvs commented Jul 23, 2018 •

edited

Loading