-
Notifications
You must be signed in to change notification settings - Fork 382
Test svcat gracefully handles the ns-broker flag being disabled #2234
Test svcat gracefully handles the ns-broker flag being disabled #2234
Conversation
b4c570b
to
c1d2033
Compare
) | ||
|
||
var catalogRequestRegex = regexp.MustCompile("/apis/servicecatalog.k8s.io/v1beta1/(.*)") | ||
var coreRequestRegex = regexp.MustCompile("/api/v1/(.*)") | ||
|
||
// Verify that svcat gracefully handles when the namespaced broker feature flag is disabled | ||
// TODO: Once we take Namespaced brokers out from behind the feature flag, this test won't be necessary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably once we get it out from the feature flag + some number of versions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think you are right, I'm just not sure how long yet. Maybe until 1.0 honestly...
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: duglin 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:
Approvers can indicate their approval by writing |
svcat
will need to gracefully handle interacting with servers that have the feature flag for namespaced broker resources disabled.svcat describe broker --namespace NS NAME
, I believe that should be a real error. Since that wasn't a supported command before. When we add support for those, we'll add test cases to ensure the proper error behavior.We'll probably want to keep this around for at least until the feature flag is removed, perhaps longer so that people don't need to be careful about which client version they use with a particular version of the api server.