Skip to content

Commit

Permalink
Add binding controller unit tests (#190)
Browse files Browse the repository at this point in the history 
Closes #174

Signed-off-by: John Starich <[email protected]>
  • Loading branch information
@JohnStarich
JohnStarich authored Sep 2, 2020
1 parent 25652ce commit 2b9afb2
Show file tree
Hide file tree
Showing 12 changed files with 2,367 additions and 42 deletions.
2 changes: 1 addition & 1 deletion Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ cache/bin/kustomize: cache/bin

.PHONY: test
test: generate manifests kubebuilder
go test ./... -coverprofile cover.out
go test -race -coverprofile cover.out ./...

.PHONY: test-e2e
test-e2e:
Expand Down
49 changes: 31 additions & 18 deletions controllers/binding_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@ import (
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
)

const (
bindingFinalizer = "binding.ibmcloud.ibm.com"
inProgress = "IN PROGRESS"
notFound = "Not Found"
idkey = "ibmcloud.ibm.com/keyId"
requeueFast = 10 * time.Second
)

const (
Expand All @@ -68,13 +68,19 @@ type BindingReconciler struct {
CreateCFServiceKey cfservice.KeyCreator
DeleteResourceServiceKey resource.KeyDeleter
DeleteCFServiceKey cfservice.KeyDeleter
GetIBMCloudInfo IBMCloudInfoGetter
GetResourceServiceKey resource.KeyGetter
GetServiceInstanceCRN resource.ServiceInstanceCRNGetter
GetCFServiceKeyCredentials cfservice.KeyGetter
GetServiceName resource.ServiceNameGetter
GetServiceRoleCRN iam.ServiceRolesGetter
SetControllerReference ControllerReferenceSetter
}

type ControllerReferenceSetter func(owner, controlled metav1.Object, scheme *runtime.Scheme) error

type IBMCloudInfoGetter func(logt logr.Logger, r client.Client, instance *ibmcloudv1beta1.Service) (*ibmcloud.Info, error)

func (r *BindingReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&ibmcloudv1beta1.Binding{}).
Expand Down Expand Up @@ -113,8 +119,9 @@ func (r *BindingReconciler) Reconcile(request ctrl.Request) (ctrl.Result, error)
if reflect.DeepEqual(instance.Status, ibmcloudv1beta1.BindingStatus{}) {
instance.Status.State = bindingStatePending
instance.Status.Message = "Processing Resource"
if err := r.Status().Update(context.Background(), instance); err != nil {
if err := r.Status().Update(ctx, instance); err != nil {
logt.Info("Binding could not update Status", instance.Name, err.Error())
// TODO(johnstarich): Shouldn't this be a failure so it can be requeued?
return ctrl.Result{}, nil
}
}
Expand All @@ -130,7 +137,7 @@ func (r *BindingReconciler) Reconcile(request ctrl.Request) (ctrl.Result, error)
// the credentials do not exist on the cloud, since the service cannot be found.
// Also by removing the Binding instance, any correponding secret will also be deleted by Kubernetes.
instance.ObjectMeta.Finalizers = deleteBindingFinalizer(instance)
if err := r.Update(context.Background(), instance); err != nil {
if err := r.Update(ctx, instance); err != nil {
logt.Info("Error removing finalizers", "in deletion", err.Error())
// No further action required, object was modified, another reconcile will finish the job.
}
Expand All @@ -142,17 +149,17 @@ func (r *BindingReconciler) Reconcile(request ctrl.Request) (ctrl.Result, error)
return r.resetResource(instance)
}

return ctrl.Result{Requeue: true, RequeueAfter: time.Second * 10}, nil //Requeue fast
return ctrl.Result{Requeue: true, RequeueAfter: requeueFast}, nil
}

// Set an owner reference if service and binding are in the same namespace
if serviceInstance.Namespace == instance.Namespace {
if err := controllerutil.SetControllerReference(serviceInstance, instance, r.Scheme); err != nil {
logt.Info("Binding could not update constroller reference", instance.Name, err.Error())
if err := r.SetControllerReference(serviceInstance, instance, r.Scheme); err != nil {
logt.Info("Binding could not update controller reference", instance.Name, err.Error())
return ctrl.Result{}, err
}

if err := r.Update(context.Background(), instance); err != nil {
if err := r.Update(ctx, instance); err != nil {
logt.Info("Error setting controller reference", instance.Name, err.Error())
return ctrl.Result{}, nil
}
Expand All @@ -162,20 +169,20 @@ func (r *BindingReconciler) Reconcile(request ctrl.Request) (ctrl.Result, error)
if serviceInstance.Status.InstanceID == "" || serviceInstance.Status.InstanceID == inProgress {
// The parent service has not been initialized fully yet
logt.Info("Parent service", "not yet initialized", instance.Name)
return ctrl.Result{Requeue: true, RequeueAfter: time.Second * 10}, nil //Requeue fast
return ctrl.Result{Requeue: true, RequeueAfter: requeueFast}, nil
}

var serviceClassType string
var session *session.Session
{
ibmCloudInfo, err := ibmcloud.GetInfo(logt, r.Client, serviceInstance)
ibmCloudInfo, err := r.GetIBMCloudInfo(logt, r.Client, serviceInstance)
if err != nil {
logt.Info("Unable to get", "ibmcloudInfo", instance.Name)
logt.Info("Unable to get IBM Cloud info", "ibmcloudInfo", instance.Name)
if errors.IsNotFound(err) && containsBindingFinalizer(instance) &&
!instance.ObjectMeta.DeletionTimestamp.IsZero() {
logt.Info("Cannot get IBMCloud related secrets and configmaps, just remove finalizers", "in deletion", err.Error())
instance.ObjectMeta.Finalizers = deleteBindingFinalizer(instance)
if err := r.Update(context.Background(), instance); err != nil {
if err := r.Update(ctx, instance); err != nil {
logt.Info("Error removing finalizers", "in deletion", err.Error())
}
return ctrl.Result{}, nil
Expand All @@ -192,7 +199,7 @@ func (r *BindingReconciler) Reconcile(request ctrl.Request) (ctrl.Result, error)
// Instance is not being deleted, add the finalizer if not present
if !containsBindingFinalizer(instance) {
instance.ObjectMeta.Finalizers = append(instance.ObjectMeta.Finalizers, bindingFinalizer)
if err := r.Update(context.Background(), instance); err != nil {
if err := r.Update(ctx, instance); err != nil {
logt.Info("Error adding finalizer", instance.Name, err.Error())
return ctrl.Result{}, nil
}
Expand All @@ -204,12 +211,12 @@ func (r *BindingReconciler) Reconcile(request ctrl.Request) (ctrl.Result, error)
err := r.deleteCredentials(session, instance, serviceClassType)
if err != nil {
logt.Info("Error deleting credentials", "in deletion", err.Error())
return ctrl.Result{Requeue: true, RequeueAfter: time.Second * 10}, nil
return ctrl.Result{Requeue: true, RequeueAfter: requeueFast}, nil
}

// remove our finalizer from the list and update it.
instance.ObjectMeta.Finalizers = deleteBindingFinalizer(instance)
if err := r.Update(context.Background(), instance); err != nil {
if err := r.Update(ctx, instance); err != nil {
logt.Info("Error removing finalizers", "in deletion", err.Error())
}
return ctrl.Result{}, nil
Expand All @@ -234,8 +241,9 @@ func (r *BindingReconciler) Reconcile(request ctrl.Request) (ctrl.Result, error)
// Now instance.Status.IntanceID has been set properly
if instance.Status.KeyInstanceID == "" { // The KeyInstanceID has not been set, need to create the key
instance.Status.KeyInstanceID = inProgress
if err := r.Status().Update(context.Background(), instance); err != nil {
if err := r.Status().Update(ctx, instance); err != nil {
logt.Info("Error updating KeyInstanceID to be in progress", "Error", err.Error())
// TODO(johnstarich): Shouldn't this be a failure so it can be requeued?
return ctrl.Result{}, nil
}

Expand Down Expand Up @@ -291,6 +299,8 @@ func (r *BindingReconciler) Reconcile(request ctrl.Request) (ctrl.Result, error)
return r.updateStatusError(instance, bindingStateFailed, err)
}
instance.Status.KeyInstanceID = keyInstanceID
} else if err != nil {
logt.Error(err, "Failed to fetch credentials") // TODO(johnstarich): should this fail and requeue?
}
}
secret, err := getSecret(r, instance)
Expand All @@ -310,7 +320,9 @@ func (r *BindingReconciler) Reconcile(request ctrl.Request) (ctrl.Result, error)
logt.Info("Error checking if key contents have changed", instance.Name, err.Error())
return r.updateStatusError(instance, bindingStateFailed, err)
}
if instance.Status.KeyInstanceID != secret.Annotations["service-key-id"] || changed { // Warning: the deep comparison may not be needed, the key is probably enough
instanceIDMismatch := instance.Status.KeyInstanceID != secret.Annotations["service-key-id"]
if instanceIDMismatch || changed { // Warning: the deep comparison may not be needed, the key is probably enough
logt.Info("Updating secret", "key contents changed", changed, "status key ID and annotation mismatch", instanceIDMismatch)
err := r.deleteSecret(instance)
if err != nil {
logt.Info("Error deleting secret before recreating", instance.Name, err.Error())
Expand Down Expand Up @@ -355,6 +367,7 @@ func (r *BindingReconciler) resetResource(instance *ibmcloudv1beta1.Binding) (ct
instance.Status.SecretName = ""
if err := r.Status().Update(context.Background(), instance); err != nil {
r.Log.Info("Binding could not reset Status", instance.Name, err.Error())
// TODO(johnstarich): Shouldn't this be a failure so it can be requeued?
return ctrl.Result{}, nil
}
return ctrl.Result{Requeue: true, RequeueAfter: config.Get().SyncPeriod}, nil
Expand Down Expand Up @@ -421,7 +434,7 @@ func (r *BindingReconciler) getAliasCredentials(logt logr.Logger, session *sessi
}

if keyName != name { // alias name and keyid annotations are inconsistent
return "", nil, fmt.Errorf("Alias credential name and keyid do not match")
return "", nil, fmt.Errorf("alias credential name and keyid do not match. Key name: %q, Alias name: %q", keyName, name)
}

_, contentsContainRedacted := credentials["REDACTED"]
Expand Down Expand Up @@ -483,7 +496,7 @@ func (r *BindingReconciler) createSecret(instance *ibmcloudv1beta1.Binding, keyC
},
Data: datamap,
}
if err := controllerutil.SetControllerReference(instance, secret, r.Scheme); err != nil {
if err := r.SetControllerReference(instance, secret, r.Scheme); err != nil {
return err
}
if err := r.Create(context.Background(), secret); err != nil {
Expand Down
Loading

0 comments on commit 2b9afb2

Please sign in to comment.