-
Notifications
You must be signed in to change notification settings - Fork 382
Make svcat provision support namespaced resources #2618
Make svcat provision support namespaced resources #2618
Conversation
f34532d
to
1c22c8a
Compare
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.
/lgtm
for code, but
/hold
for some comment about the design to
"always resolve to kube name before acting" or some such
PlanReference: v1beta1.PlanReference{ | ||
ClusterServiceClassExternalName: className, | ||
ClusterServicePlanExternalName: planName, | ||
func (sdk *SDK) Provision(instanceName, classKubeName, planKubeName string, provisionClusterInstance bool, opts *ProvisionOptions) (*v1beta1.ServiceInstance, error) { |
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.
this function changed because of the mentioned comment I'd like added.
It's always kube names that come in now.
1c22c8a
to
1d7b79e
Compare
@@ -26,24 +27,29 @@ import ( | |||
"github.com/spf13/cobra" | |||
) | |||
|
|||
type provisonCmd struct { | |||
// ProvisionCmd contains the info needed to provision a new service instance | |||
type ProvisionCmd struct { |
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.
Why do we need to make ProvisionCmd
public?
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.
I completely refactored the testing here. As part of that, I moved the tests into their own package, as per Ginkgo guidelines. So, I had to export the struct type and it's fields.
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jboyd01 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 |
1d7b79e
to
22f2416
Compare
- add unit tests for provision_cmd.go - refactor provision_cmd.go to support new tests - fix typo ProvisonCmd -> ProvisionCmd
22f2416
to
4259f5c
Compare
/test pull-service-catalog-xbuild |
1 similar comment
/test pull-service-catalog-xbuild |
/lgtm |
…#2618) - add unit tests for provision_cmd.go - refactor provision_cmd.go to support new tests - fix typo ProvisonCmd -> ProvisionCmd
Note that the deleted goldenfile test was removed because the first call to the backend isn't a namespaced as it tries to locate exactly which cluster/namespaced service/plane the user is referring to. Thus, it no longer made sense in the current test frameworks. The args to the pkg lib (including the namespace) are tested in the newly added unit test for provision.go
Fixes #2371