Skip to content
This repository has been archived by the owner on May 6, 2022. It is now read-only.

Update instance loop support for namespaced classes and plans #2133

Merged
merged 24 commits into from
Jun 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
291ebed
Initial ns instance loop updates
eriknelson May 8, 2018
5458df7
Disambiguate getServiceInstanceWithRefs
eriknelson Jun 14, 2018
8b4ce7d
Update .gitignore to ignore debug.test
eriknelson Jun 18, 2018
0bc3801
Disambiguate getTestServiceInstanceWithClusterRefs
eriknelson Jun 18, 2018
ff61185
Add namespaces to the test type producers
eriknelson Jun 18, 2018
faca699
Add TestReconcileServiceInstanceWithNamespacedRefs
eriknelson Jun 18, 2018
0bd698b
Passing cp
eriknelson Jun 18, 2018
614841a
Add sync provision test for namespaced class/plan
eriknelson Jun 18, 2018
e03feea
Add reconcile async with namespaced refs
eriknelson Jun 18, 2018
5887ad5
Give instance tests their own file
eriknelson Jun 18, 2018
221efbf
Add ResolveNamespacedReferences test
eriknelson Jun 18, 2018
e317ba1
Add instance DeleteWithNamespacedRefs sync test
eriknelson Jun 18, 2018
5b327cd
Add instance DeleteAsyncWithNamespaced refs test
eriknelson Jun 18, 2018
6754adf
Buildable prepareUpdateInstanceRequest
jmrodri Jun 18, 2018
334c3b5
Fix in progress polling for instances
eriknelson Jun 19, 2018
732ae37
Add inprogress provision polling test
eriknelson Jun 19, 2018
3916378
Cover the nil plan case for last op reqs
eriknelson Jun 19, 2018
35cc9ed
Add success async provision ns type test
eriknelson Jun 19, 2018
d0c11dd
check properties for nil early
jmrodri Jun 19, 2018
c05ab18
Add inprogress provision async failure ns test
eriknelson Jun 19, 2018
1e9139f
Add inprogress depro poll ns test
eriknelson Jun 19, 2018
4af7d59
Add polling success deprovision ns test
eriknelson Jun 19, 2018
ad1f04b
Add failed polling deprovision ns test
eriknelson Jun 19, 2018
d4faebe
Update new files with 2018 copyright header
eriknelson Jun 21, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,4 @@ contrib/examples/consumer/Gopkg.lock
contrib/examples/consumer
contrib/examples/vendor/*
integration.test*
debug.test*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is this coming from?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was some kind of binary that showed up after running a failed make command, I can't remember which. I'm assuming we would prefer not to have that checked in.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I've seen those pop up too after running the tests.

40 changes: 20 additions & 20 deletions pkg/apis/servicecatalog/plan_reference.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,26 +90,6 @@ func (pr PlanReference) GetSpecifiedServiceClass() string {
return ""
}

// GetSpecifiedServicePlan returns the user-specified class value from either:
// * ServicePlanExternalName
// * ServicePlanExternalID
// * ServicePlanName
func (pr PlanReference) GetSpecifiedServicePlan() string {
if pr.ServicePlanExternalName != "" {
return pr.ServicePlanExternalName
}

if pr.ServicePlanExternalID != "" {
return pr.ServicePlanExternalID
}

if pr.ServicePlanName != "" {
return pr.ServicePlanName
}

return ""
}

// GetSpecifiedClusterServicePlan returns the user-specified plan value from one of:
// * ClusterServicePlanExternalName
// * ClusterServicePlanExternalID
Expand All @@ -131,6 +111,26 @@ func (pr PlanReference) GetSpecifiedClusterServicePlan() string {
return ""
}

// GetSpecifiedServicePlan returns the user-specified plan value from either:
// * ServicePlanExternalName
// * ServicePlanExternalID
// * ServicePlanName
func (pr PlanReference) GetSpecifiedServicePlan() string {
if pr.ServicePlanExternalName != "" {
return pr.ServicePlanExternalName
}

if pr.ServicePlanExternalID != "" {
return pr.ServicePlanExternalID
}

if pr.ServicePlanName != "" {
return pr.ServicePlanName
}

return ""
}

// GetClusterServiceClassFilterFieldName returns the appropriate field name for filtering
// a list of service catalog classes by the PlanReference.
func (pr PlanReference) GetClusterServiceClassFilterFieldName() string {
Expand Down
30 changes: 29 additions & 1 deletion pkg/apis/servicecatalog/v1beta1/plan_reference.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (pr PlanReference) GetSpecifiedClusterServicePlan() string {
return ""
}

// GetSpecifiedServicePlan returns the user-specified class value from either:
// GetSpecifiedServicePlan returns the user-specified plan value from either:
// * ServicePlanExternalName
// * ServicePlanExternalID
// * ServicePlanName
Expand Down Expand Up @@ -159,6 +159,34 @@ func (pr PlanReference) GetClusterServicePlanFilterFieldName() string {
return ""
}

// GetServiceClassFilterFieldName returns the appropriate field name for filtering
// a list of service catalog classes by the PlanReference.
func (pr PlanReference) GetServiceClassFilterFieldName() string {
if pr.ServiceClassExternalName != "" {
return "spec.externalName"
}

if pr.ServiceClassExternalID != "" {
return "spec.externalID"
}

return ""
}

// GetServicePlanFilterFieldName returns the appropriate field name for filtering
// a list of service catalog plans by the PlanReference.
func (pr PlanReference) GetServicePlanFilterFieldName() string {
if pr.ServicePlanExternalName != "" {
return "spec.externalName"
}

if pr.ServicePlanExternalID != "" {
return "spec.externalID"
}

return ""
}

// String representation of a PlanReference
// Example: class_name/plan_name, class_id/plan_id
func (pr PlanReference) String() string {
Expand Down
74 changes: 72 additions & 2 deletions pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,29 @@ func (c *controller) getClusterServiceClassPlanAndClusterServiceBroker(instance
return serviceClass, servicePlan, brokerName, brokerClient, nil
}

func (c *controller) getServiceClassPlanAndServiceBroker(instance *v1beta1.ServiceInstance) (*v1beta1.ServiceClass, *v1beta1.ServicePlan, string, osb.Client, error) {
serviceClass, brokerName, brokerClient, err := c.getServiceClassAndServiceBroker(instance)
if err != nil {
return nil, nil, "", nil, err
}

var servicePlan *v1beta1.ServicePlan
if instance.Spec.ServicePlanRef != nil {
var err error
servicePlan, err = c.servicePlanLister.ServicePlans(instance.Namespace).Get(instance.Spec.ServicePlanRef.Name)
if nil != err {
return nil, nil, "", nil, &operationError{
reason: errorNonexistentServicePlanReason,
message: fmt.Sprintf(
"The instance references a non-existent ServicePlan %q - %v",
instance.Spec.ServicePlanRef.Name, instance.Spec.PlanReference,
),
}
}
}
return serviceClass, servicePlan, brokerName, brokerClient, nil
}

// getClusterServiceClassAndClusterServiceBroker is a sequence of operations that's done in couple of
// places so this method fetches the Service Class and creates
// a brokerClient to use for that method given an ServiceInstance.
Expand Down Expand Up @@ -464,6 +487,55 @@ func (c *controller) getClusterServiceClassAndClusterServiceBroker(instance *v1b
return serviceClass, broker.Name, brokerClient, nil
}

// getServiceClassAndServiceBroker is a sequence of operations that's done in couple of
// places so this method fetches the Service Class and creates
// a brokerClient to use for that method given a ServiceInstance.
func (c *controller) getServiceClassAndServiceBroker(instance *v1beta1.ServiceInstance) (*v1beta1.ServiceClass, string, osb.Client, error) {
pcb := pretty.NewContextBuilder(pretty.ServiceInstance, instance.Namespace, instance.Name, "")
serviceClass, err := c.serviceClassLister.ServiceClasses(instance.Namespace).Get(instance.Spec.ServiceClassRef.Name)
if err != nil {
return nil, "", nil, &operationError{
reason: errorNonexistentServiceClassReason,
message: fmt.Sprintf(
"The instance references a non-existent ServiceClass (K8S: %q ExternalName: %q)",
instance.Spec.ServiceClassRef.Name, instance.Spec.ServiceClassExternalName,
),
}
}

broker, err := c.serviceBrokerLister.ServiceBrokers(instance.Namespace).Get(serviceClass.Spec.ServiceBrokerName)
if err != nil {
return nil, "", nil, &operationError{
reason: errorNonexistentServiceBrokerReason,
message: fmt.Sprintf(
"The instance references a non-existent broker %q",
serviceClass.Spec.ServiceBrokerName,
),
}

}

authConfig, err := getAuthCredentialsFromServiceBroker(c.kubeClient, broker)
if err != nil {
return nil, "", nil, &operationError{
reason: errorAuthCredentialsReason,
message: fmt.Sprintf(
"Error getting broker auth credentials for broker %q: %s",
broker.Name, err,
),
}
}

clientConfig := NewClientConfigurationForBroker(broker.ObjectMeta, &broker.Spec.CommonServiceBrokerSpec, authConfig)
glog.V(4).Info(pcb.Messagef("Creating client for ServiceBroker %v, URL: %v", broker.Name, broker.Spec.URL))
brokerClient, err := c.brokerClientCreateFunc(clientConfig)
if err != nil {
return nil, "", nil, err
}

return serviceClass, broker.Name, brokerClient, nil
}

// getClusterServiceClassPlanAndClusterServiceBrokerForServiceBinding is a sequence of operations that's
// done to validate service plan, service class exist, and handles creating
// a brokerclient to use for a given ServiceInstance.
Expand Down Expand Up @@ -637,8 +709,6 @@ func getAuthCredentialsFromClusterServiceBroker(client kubernetes.Interface, bro
// returns an error. If the AuthInfo field is nil, empty values are
// returned.
func getAuthCredentialsFromServiceBroker(client kubernetes.Interface, broker *v1beta1.ServiceBroker) (*osb.AuthConfig, error) {
// ERIK TODO: This method is mostly error handling boilerplate, is it worth consolidating with common elements?
// Main difference are just using the broker's namespace instead of the same namespace as the broker.
if broker.Spec.AuthInfo == nil {
return nil, nil
}
Expand Down
Loading