Skip to content

Commit

Permalink
Adopting Orphan Instances via Kubernetes Annotation (#50)
Browse files Browse the repository at this point in the history 
* Initial commit for adopting orphan instances in cf

* Adoption for the existing service bindings

* update for adopting instance annotation

* Change GetInstance by name or by owner. Update CF instance with label and annotation.

* new branch for orphan instances adoption

* revert to original defer function

* implement for orphan service binding

* Update markdown documentation with the new annotation, adopt-instances.

* Modify GetInstance and GetBinding functions based on PR comments

* Add changes based or reviewer comments

* changes for comments on listoptions

* Update the documentation

* Changes to getinstance call  in clinet_test.go file

* Update generated deepcopy go

* Update adopt.md file following comment from reviewer

---------

Co-authored-by: shilparamasamyreddy <[email protected]>
  • Loading branch information
@santiago-ventura @shilparamasamyreddy
santiago-ventura and shilparamasamyreddy authored Jun 25, 2024
1 parent ccfa83d commit 3d152c6
Show file tree
Hide file tree
Showing 9 changed files with 272 additions and 76 deletions.
3 changes: 3 additions & 0 deletions api/v1alpha1/constants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,7 @@ const (
AnnotationMaxRetries = "service-operator.cf.cs.sap.com/max-retries"
// annotation to hold the reconciliation timeout value
AnnotationReconcileTimeout = "service-operator.cf.cs.sap.com/timeout-on-reconcile"
// annotation to adopt orphan CF resources. If set to 'adopt', the operator will adopt orphan CF resource.
// Ex. "service-operator.cf.cs.sap.com/adopt-cf-resources"="adopt"
AnnotationAdoptCFResources = "service-operator.cf.cs.sap.com/adopt-cf-resources"
)
61 changes: 55 additions & 6 deletions internal/cf/binding.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,64 @@ import (
"github.com/sap/cf-service-operator/internal/facade"
)

func (c *spaceClient) GetBinding(ctx context.Context, owner string) (*facade.Binding, error) {
type bindingFilter interface {
getListOptions() *cfclient.ServiceCredentialBindingListOptions
}

type bindingFilterName struct {
name string
}
type bindingFilterOwner struct {
owner string
}

func (bn *bindingFilterName) getListOptions() *cfclient.ServiceCredentialBindingListOptions {
listOpts := cfclient.NewServiceCredentialBindingListOptions()
listOpts.LabelSelector.EqualTo(labelPrefix + "/" + labelKeyOwner + "=" + owner)
listOpts.Names.EqualTo(bn.name)
return listOpts
}

func (bo *bindingFilterOwner) getListOptions() *cfclient.ServiceCredentialBindingListOptions {
listOpts := cfclient.NewServiceCredentialBindingListOptions()
listOpts.LabelSelector.EqualTo(fmt.Sprintf("%s/%s=%s", labelPrefix, labelKeyOwner, bo.owner))
return listOpts
}

// GetBinding returns the binding with the given bindingOpts["owner"] or bindingOpts["name"].
// If bindingOpts["name"] is empty, the binding with the given bindingOpts["owner"] is returned.
// If bindingOpts["name"] is not empty, the binding with the given Name is returned for orphan bindings.
// If no binding is found, nil is returned.
// If multiple bindings are found, an error is returned.
// The function add the parameter values to the orphan cf binding, so that can be adopted.
func (c *spaceClient) GetBinding(ctx context.Context, bindingOpts map[string]string) (*facade.Binding, error) {
var filterOpts bindingFilter
if bindingOpts["name"] != "" {
filterOpts = &bindingFilterName{name: bindingOpts["name"]}
} else {
filterOpts = &bindingFilterOwner{owner: bindingOpts["owner"]}
}
listOpts := filterOpts.getListOptions()
serviceBindings, err := c.client.ServiceCredentialBindings.ListAll(ctx, listOpts)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to list service credential bindings: %w", err)
}

if len(serviceBindings) == 0 {
return nil, nil
} else if len(serviceBindings) > 1 {
return nil, fmt.Errorf("found multiple service bindings with owner: %s", owner)
return nil, errors.New(fmt.Sprintf("found multiple service bindings with owner: %s", bindingOpts["owner"]))
}

serviceBinding := serviceBindings[0]

// add parameter values to the cf orphan binding
if bindingOpts["name"] != "" {
generationvalue := "0"
serviceBinding.Metadata.Annotations[annotationGeneration] = &generationvalue
parameterHashValue := "0"
serviceBinding.Metadata.Annotations[annotationParameterHash] = &parameterHashValue
}

guid := serviceBinding.GUID
name := serviceBinding.Name
generation, err := strconv.ParseInt(*serviceBinding.Metadata.Annotations[annotationGeneration], 10, 64)
Expand Down Expand Up @@ -71,7 +114,7 @@ func (c *spaceClient) GetBinding(ctx context.Context, owner string) (*facade.Bin
return &facade.Binding{
Guid: guid,
Name: name,
Owner: owner,
Owner: bindingOpts["owner"],
Generation: generation,
ParameterHash: parameterHash,
State: state,
Expand Down Expand Up @@ -101,12 +144,18 @@ func (c *spaceClient) CreateBinding(ctx context.Context, name string, serviceIns
}

// Required parameters (may not be initial): guid, generation
func (c *spaceClient) UpdateBinding(ctx context.Context, guid string, generation int64) error {
func (c *spaceClient) UpdateBinding(ctx context.Context, guid string, generation int64, parameters map[string]interface{}) error {
// TODO: why is there no cfresource.NewServiceCredentialBindingUpdate() method ?
req := &cfresource.ServiceCredentialBindingUpdate{}
req.Metadata = cfresource.NewMetadata().
WithAnnotation(annotationPrefix, annotationKeyGeneration, strconv.FormatInt(generation, 10))

if parameters != nil {
req.Metadata.WithAnnotation(annotationPrefix, annotationKeyParameterHash, facade.ObjectHash(parameters))
if parameters["owner"] != nil {
req.Metadata.WithLabel(labelPrefix, labelKeyOwner, parameters["owner"].(string))
}
}
_, err := c.client.ServiceCredentialBindings.Update(ctx, guid, req)
return err
}
Expand Down
10 changes: 5 additions & 5 deletions internal/cf/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ var _ = Describe("CF Client tests", Ordered, func() {
spaceClient, err := NewSpaceClient(OrgName, url, Username, Password)
Expect(err).To(BeNil())

spaceClient.GetInstance(ctx, Owner)
spaceClient.GetInstance(ctx, map[string]string{"owner": Owner})

// Discover UAA endpoint
Expect(server.ReceivedRequests()[0].Method).To(Equal("GET"))
Expand All @@ -239,10 +239,10 @@ var _ = Describe("CF Client tests", Ordered, func() {
spaceClient, err := NewSpaceClient(OrgName, url, Username, Password)
Expect(err).To(BeNil())

spaceClient.GetInstance(ctx, Owner)
spaceClient.GetInstance(ctx, map[string]string{"owner": Owner})
spaceClient, err = NewSpaceClient(OrgName, url, Username, Password)
Expect(err).To(BeNil())
spaceClient.GetInstance(ctx, Owner)
spaceClient.GetInstance(ctx, map[string]string{"owner": Owner})

// Discover UAA endpoint
Expect(server.ReceivedRequests()[0].Method).To(Equal("GET"))
Expand All @@ -265,7 +265,7 @@ var _ = Describe("CF Client tests", Ordered, func() {
// test space 1
spaceClient1, err1 := NewSpaceClient(SpaceName, url, Username, Password)
Expect(err1).To(BeNil())
spaceClient1.GetInstance(ctx, Owner)
spaceClient1.GetInstance(ctx, map[string]string{"owner": Owner})
// Discover UAA endpoint
Expect(server.ReceivedRequests()[0].Method).To(Equal("GET"))
Expect(server.ReceivedRequests()[0].URL.Path).To(Equal("/"))
Expand All @@ -279,7 +279,7 @@ var _ = Describe("CF Client tests", Ordered, func() {
// test space 2
spaceClient2, err2 := NewSpaceClient(SpaceName2, url, Username, Password)
Expect(err2).To(BeNil())
spaceClient2.GetInstance(ctx, Owner2)
spaceClient2.GetInstance(ctx, map[string]string{"owner": Owner2})
// no discovery of UAA endpoint or oAuth token here due to caching
// Get instance
Expect(server.ReceivedRequests()[3].Method).To(Equal("GET"))
Expand Down
58 changes: 53 additions & 5 deletions internal/cf/instance.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,65 @@ import (
"github.com/sap/cf-service-operator/internal/facade"
)

func (c *spaceClient) GetInstance(ctx context.Context, owner string) (*facade.Instance, error) {
type instanceFilter interface {
getListOptions() *cfclient.ServiceInstanceListOptions
}

type instanceFilterName struct {
name string
}
type instanceFilterOwner struct {
owner string
}

func (in *instanceFilterName) getListOptions() *cfclient.ServiceInstanceListOptions {
listOpts := cfclient.NewServiceInstanceListOptions()
listOpts.Names.EqualTo(in.name)
return listOpts
}

func (io *instanceFilterOwner) getListOptions() *cfclient.ServiceInstanceListOptions {
listOpts := cfclient.NewServiceInstanceListOptions()
listOpts.LabelSelector.EqualTo(labelPrefix + "/" + labelKeyOwner + "=" + owner)
listOpts.LabelSelector.EqualTo(fmt.Sprintf("%s/%s=%s", labelPrefix, labelKeyOwner, io.owner))
return listOpts
}

// GetInstance returns the instance with the given instanceOpts["owner"] or instanceOpts["name"].
// If instanceOpts["name"] is empty, the instance with the given instanceOpts["owner"] is returned.
// If instanceOpts["name"] is not empty, the instance with the given Name is returned for orphan instances.
// If no instance is found, nil is returned.
// If multiple instances are found, an error is returned.
// The function add the parameter values to the orphan cf instance, so that can be adopted.
func (c *spaceClient) GetInstance(ctx context.Context, instanceOpts map[string]string) (*facade.Instance, error) {

var filterOpts instanceFilter
if instanceOpts["name"] != "" {
filterOpts = &instanceFilterName{name: instanceOpts["name"]}
} else {
filterOpts = &instanceFilterOwner{owner: instanceOpts["owner"]}
}
listOpts := filterOpts.getListOptions()
serviceInstances, err := c.client.ServiceInstances.ListAll(ctx, listOpts)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to list service instances: %w", err)
}

if len(serviceInstances) == 0 {
return nil, nil
} else if len(serviceInstances) > 1 {
return nil, fmt.Errorf("found multiple service instances with owner: %s", owner)
return nil, errors.New(fmt.Sprintf("found multiple service instances with owner: %s", instanceOpts["owner"]))
}

serviceInstance := serviceInstances[0]

// add parameter values to the orphan cf instance
if instanceOpts["name"] != "" {
generationvalue := "0"
serviceInstance.Metadata.Annotations[annotationGeneration] = &generationvalue
parameterHashValue := "0"
serviceInstance.Metadata.Annotations[annotationParameterHash] = &parameterHashValue
}

guid := serviceInstance.GUID
name := serviceInstance.Name
servicePlanGuid := serviceInstance.Relationships.ServicePlan.Data.GUID
Expand Down Expand Up @@ -70,7 +114,7 @@ func (c *spaceClient) GetInstance(ctx context.Context, owner string) (*facade.In
Guid: guid,
Name: name,
ServicePlanGuid: servicePlanGuid,
Owner: owner,
Owner: instanceOpts["owner"],
Generation: generation,
ParameterHash: parameterHash,
State: state,
Expand Down Expand Up @@ -127,6 +171,10 @@ func (c *spaceClient) UpdateInstance(ctx context.Context, guid string, name stri
WithAnnotation(annotationPrefix, annotationKeyGeneration, strconv.FormatInt(generation, 10))
if parameters != nil {
req.Metadata.WithAnnotation(annotationPrefix, annotationKeyParameterHash, facade.ObjectHash(parameters))
if parameters["owner"] != nil {
// Adding label to the metadata for orphan instance
req.Metadata.WithLabel(labelPrefix, labelKeyOwner, parameters["owner"].(string))
}
}

_, _, err := c.client.ServiceInstances.UpdateManaged(ctx, guid, req)
Expand Down
46 changes: 42 additions & 4 deletions internal/controllers/servicebinding_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,13 +174,49 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque
}
}

// Retrieve cloud foundry instance
// Retrieve cloud foundry binding
var cfbinding *facade.Binding
bindingOpts := map[string]string{"name": "", "owner": string(serviceBinding.UID)}
if client != nil {
cfbinding, err = client.GetBinding(ctx, string(serviceBinding.UID))
cfbinding, err = client.GetBinding(ctx, bindingOpts)
if err != nil {
return ctrl.Result{}, err
}
orphan, exists := serviceBinding.Annotations[cfv1alpha1.AnnotationAdoptCFResources]
if exists && cfbinding == nil && orphan == "adopt" {
// find orphaned binding by name
bindingOpts["name"] = serviceBinding.Name
cfbinding, err = client.GetBinding(ctx, bindingOpts)
if err != nil {
return ctrl.Result{}, err
}

//Add parameters to adopt the orphaned binding
var parameterObjects []map[string]interface{}
paramMap := make(map[string]interface{})
paramMap["parameter-hash"] = cfbinding.ParameterHash
paramMap["owner"] = cfbinding.Owner
parameterObjects = append(parameterObjects, paramMap)
parameters, err := mergeObjects(parameterObjects...)
if err != nil {
return ctrl.Result{}, errors.Wrap(err, "failed to unmarshal/merge parameters")
}
// update the orphaned cloud foundry service binding
log.V(1).Info("triggering update")
if err := client.UpdateBinding(
ctx,
cfbinding.Guid,
serviceBinding.Generation,
parameters,
); err != nil {
return ctrl.Result{}, err
}
status.LastModifiedAt = &[]metav1.Time{metav1.Now()}[0]

// return the reconcile function to requeue inmediatly after the update
serviceBinding.SetReadyCondition(cfv1alpha1.ConditionUnknown, string(cfbinding.State), cfbinding.StateDescription)
return ctrl.Result{Requeue: true}, nil
}
}

if serviceBinding.DeletionTimestamp.IsZero() {
Expand Down Expand Up @@ -268,10 +304,12 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque
} else if cfbinding.Generation < serviceBinding.Generation {
// metadata updates (such as updating the generation here) are possible with service bindings
log.V(1).Info("triggering update")

if err := client.UpdateBinding(
ctx,
cfbinding.Guid,
serviceBinding.Generation,
nil,
); err != nil {
return ctrl.Result{}, err
}
Expand All @@ -284,8 +322,8 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque
status.ServiceInstanceDigest = serviceInstance.Status.ServiceInstanceDigest

if cfbinding == nil {
// Re-retrieve cloud foundry binding; this happens exactly if the binding was created or updated above
cfbinding, err = client.GetBinding(ctx, string(serviceBinding.UID))
// Re-retrieve cloud foundry binding by UID; this happens exactly if the binding was created or updated above
cfbinding, err = client.GetBinding(ctx, bindingOpts)
if err != nil {
return ctrl.Result{}, err
}
Expand Down
45 changes: 42 additions & 3 deletions internal/controllers/serviceinstance_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,49 @@ func (r *ServiceInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ

// Retrieve cloud foundry instance
var cfinstance *facade.Instance
instanceOpts := map[string]string{"name": "", "owner": string(serviceInstance.UID)}
if client != nil {
cfinstance, err = client.GetInstance(ctx, string(serviceInstance.UID))
cfinstance, err = client.GetInstance(ctx, instanceOpts)
if err != nil {
return ctrl.Result{}, err
}
orphan, exists := serviceInstance.Annotations[cfv1alpha1.AnnotationAdoptCFResources]
if exists && cfinstance == nil && orphan == "adopt" {
// find orphaned instance by name
instanceOpts["name"] = serviceInstance.Name
cfinstance, err = client.GetInstance(ctx, instanceOpts)
if err != nil {
return ctrl.Result{}, err
}

//Add parameters to adopt the orphaned instance
var parameterObjects []map[string]interface{}
paramMap := make(map[string]interface{})
paramMap["parameter-hash"] = cfinstance.ParameterHash
paramMap["owner"] = cfinstance.Owner
parameterObjects = append(parameterObjects, paramMap)
parameters, err := mergeObjects(parameterObjects...)
if err != nil {
return ctrl.Result{}, errors.Wrap(err, "failed to unmarshal/merge parameters")
}
// update the orphaned cloud foundry instance
log.V(1).Info("triggering update")
if err := client.UpdateInstance(
ctx,
cfinstance.Guid,
spec.Name,
"",
parameters,
nil,
serviceInstance.Generation,
); err != nil {
return ctrl.Result{}, err
}
status.LastModifiedAt = &[]metav1.Time{metav1.Now()}[0]
// return the reconcile function to requeue inmediatly after the update
serviceInstance.SetReadyCondition(cfv1alpha1.ConditionUnknown, string(cfinstance.State), cfinstance.StateDescription)
return ctrl.Result{Requeue: true}, nil
}
}

if serviceInstance.DeletionTimestamp.IsZero() {
Expand Down Expand Up @@ -243,6 +281,7 @@ func (r *ServiceInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ
return ctrl.Result{}, fmt.Errorf("secret key not found, secret name: %s, key: %s", secretName, pf.SecretKeyRef.Key)
}
}

parameters, err := mergeObjects(parameterObjects...)
if err != nil {
return ctrl.Result{}, errors.Wrap(err, "failed to unmarshal/merge parameters")
Expand Down Expand Up @@ -324,8 +363,8 @@ func (r *ServiceInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ
}

if cfinstance == nil {
// Re-retrieve cloud foundry instance; this happens exactly if the instance was created or updated above
cfinstance, err = client.GetInstance(ctx, string(serviceInstance.UID))
// Re-retrieve cloud foundry instance by UID; this happens exactly if the instance was created or updated above
cfinstance, err = client.GetInstance(ctx, instanceOpts)
if err != nil {
return ctrl.Result{}, err
}
Expand Down
6 changes: 3 additions & 3 deletions internal/facade/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,14 @@ type OrganizationClientBuilder func(string, string, string, string) (Organizatio

//counterfeiter:generate . SpaceClient
type SpaceClient interface {
GetInstance(ctx context.Context, owner string) (*Instance, error)
GetInstance(ctx context.Context, instanceOpts map[string]string) (*Instance, error)
CreateInstance(ctx context.Context, name string, servicePlanGuid string, parameters map[string]interface{}, tags []string, owner string, generation int64) error
UpdateInstance(ctx context.Context, guid string, name string, servicePlanGuid string, parameters map[string]interface{}, tags []string, generation int64) error
DeleteInstance(ctx context.Context, guid string) error

GetBinding(ctx context.Context, owner string) (*Binding, error)
GetBinding(ctx context.Context, bindingOpts map[string]string) (*Binding, error)
CreateBinding(ctx context.Context, name string, serviceInstanceGuid string, parameters map[string]interface{}, owner string, generation int64) error
UpdateBinding(ctx context.Context, guid string, generation int64) error
UpdateBinding(ctx context.Context, guid string, generation int64, parameters map[string]interface{}) error
DeleteBinding(ctx context.Context, guid string) error

FindServicePlan(ctx context.Context, serviceOfferingName string, servicePlanName string, spaceGuid string) (string, error)
Expand Down
Loading

0 comments on commit 3d152c6

Please sign in to comment.