Skip to content

Commit

Permalink
Create an overall Ready to indicate whether the SBR reconcile is co…
Browse files Browse the repository at this point in the history 
…mpleted successfully redhat-developer#665
  • Loading branch information
@qibobo
qibobo committed Nov 3, 2020
1 parent ad1b2c2 commit 767b344
Show file tree
Hide file tree
Showing 14 changed files with 316 additions and 23 deletions.
57 changes: 52 additions & 5 deletions pkg/controller/servicebinding/reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1"
corev1 "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -15,10 +16,13 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"

"github.com/redhat-developer/service-binding-operator/pkg/apis/operators/v1alpha1"
"github.com/redhat-developer/service-binding-operator/pkg/converter"
"github.com/redhat-developer/service-binding-operator/pkg/log"
)

const (
// BindingReady indicates that the overall sbr succeeded
BindingReady conditionsv1.ConditionType = "Ready"
// CollectionReady indicates readiness for collection and persistance of intermediate manifests
CollectionReady conditionsv1.ConditionType = "CollectionReady"
// InjectionReady indicates readiness to change application manifests to use those intermediate manifests
Expand All @@ -32,6 +36,8 @@ const (
EmptyApplicationReason = "EmptyApplication"
// ApplicationNotFoundReason is used when the application is not found.
ApplicationNotFoundReason = "ApplicationNotFound"
// ServiceNotFoundReason is used when the service is not found.
ServiceNotFoundReason = "ServiceNotFound"
)

// Reconciler reconciles a ServiceBinding object
Expand Down Expand Up @@ -131,10 +137,12 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err
Message: errEmptyServices.Error(),
},
conditionsv1.Condition{
Type: InjectionReady,
Status: corev1.ConditionFalse,
Reason: EmptyServiceSelectorsReason,
Message: errEmptyServices.Error(),
Type: InjectionReady,
Status: corev1.ConditionFalse,
},
conditionsv1.Condition{
Type: BindingReady,
Status: corev1.ConditionFalse,
},
)
if updateErr == nil {
Expand All @@ -157,9 +165,30 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err
r.restMapper,
)
if err != nil {
//handle service not found error
if k8serrors.IsNotFound(err) {
err = updateSBRConditions(r.dynClient, sbr,
conditionsv1.Condition{
Type: CollectionReady,
Status: corev1.ConditionFalse,
Reason: ServiceNotFoundReason,
Message: err.Error(),
},
conditionsv1.Condition{
Type: InjectionReady,
Status: corev1.ConditionFalse,
},
conditionsv1.Condition{
Type: BindingReady,
Status: corev1.ConditionFalse,
},
)
if err != nil {
logger.Error(err, "Failed to update SBR conditions", "sbr", sbr)
}
}
return requeueError(err)
}

binding, err := buildBinding(
r.dynClient,
sbr.Spec.CustomEnvVar,
Expand Down Expand Up @@ -220,3 +249,21 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err
logger.Info("Binding applications with intermediary secret...")
return sb.bind()
}

func updateSBRConditions(dynClient dynamic.Interface, sbr *v1alpha1.ServiceBinding, conditions ...conditionsv1.Condition) error {
for _, v := range conditions {
conditionsv1.SetStatusCondition(&sbr.Status.Conditions, v)
}
u, err := converter.ToUnstructured(sbr)
if err != nil {
return err
}

nsClient := dynClient.
Resource(groupVersion).
Namespace(sbr.GetNamespace())

_, err = nsClient.UpdateStatus(u, metav1.UpdateOptions{})

return err
}
87 changes: 87 additions & 0 deletions pkg/controller/servicebinding/reconciler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ func TestApplicationByName(t *testing.T) {

requireConditionPresentAndTrue(t, CollectionReady, sbrOutput.Status.Conditions)
requireConditionPresentAndTrue(t, InjectionReady, sbrOutput.Status.Conditions)
requireConditionPresentAndTrue(t, BindingReady, sbrOutput.Status.Conditions)

require.Equal(t, 1, len(sbrOutput.Status.Applications))
expectedStatus := v1alpha1.BoundApplication{
Expand Down Expand Up @@ -196,6 +197,7 @@ func TestReconcilerReconcileUsingSecret(t *testing.T) {

requireConditionPresentAndTrue(t, CollectionReady, sbrOutput.Status.Conditions)
requireConditionPresentAndTrue(t, InjectionReady, sbrOutput.Status.Conditions)
requireConditionPresentAndTrue(t, BindingReady, sbrOutput.Status.Conditions)

require.Equal(t, reconcilerName, sbrOutput.Status.Secret)

Expand Down Expand Up @@ -259,6 +261,59 @@ func TestReconcilerReconcileUsingVolumes(t *testing.T) {
})
}

func TestServiceNotFound(t *testing.T) {
backingServiceResourceRef := "backingServiceRef"
applicationResourceRef := "applicationRef"
f := mocks.NewFake(t, reconcilerNs)
f.AddMockedUnstructuredServiceBinding(reconcilerName, backingServiceResourceRef, applicationResourceRef, deploymentsGVR, nil)
f.AddMockedUnstructuredDatabaseCRD()
f.AddMockedUnstructuredDeployment(applicationResourceRef, nil)

fakeDynClient := f.FakeDynClient()
mapper := testutils.BuildTestRESTMapper()
r := &reconciler{dynClient: fakeDynClient, restMapper: mapper, scheme: f.S}
r.resourceWatcher = newFakeResourceWatcher(mapper)

// Reconcile without service
res, _ := r.Reconcile(reconcileRequest())
require.True(t, res.Requeue)

namespacedName := types.NamespacedName{Namespace: reconcilerNs, Name: reconcilerName}
sbrOutput, err := r.getServiceBinding(namespacedName)
require.NoError(t, err)

requireConditionPresentAndFalse(t, CollectionReady, sbrOutput.Status.Conditions)
requireConditionPresentAndFalse(t, InjectionReady, sbrOutput.Status.Conditions)
requireConditionPresentAndFalse(t, BindingReady, sbrOutput.Status.Conditions)
require.Len(t, sbrOutput.Status.Applications, 0)

// Reconcile with service
f.AddMockedUnstructuredDatabaseCR(backingServiceResourceRef)
f.AddMockedUnstructuredSecret("db-credentials")
fakeDynClient = f.FakeDynClient()
r = &reconciler{dynClient: fakeDynClient, restMapper: testutils.BuildTestRESTMapper(), scheme: f.S}
r.resourceWatcher = newFakeResourceWatcher(mapper)
res, err = r.Reconcile(reconcileRequest())
require.NoError(t, err)
require.False(t, res.Requeue)

u, err := fakeDynClient.Resource(deploymentsGVR).Get(applicationResourceRef, metav1.GetOptions{})
require.NoError(t, err)

d := appsv1.Deployment{}
err = runtime.DefaultUnstructuredConverter.FromUnstructured(u.Object, &d)
require.NoError(t, err)
sbrOutput2, err := r.getServiceBinding(namespacedName)
require.NoError(t, err)

requireConditionPresentAndTrue(t, CollectionReady, sbrOutput2.Status.Conditions)
requireConditionPresentAndTrue(t, InjectionReady, sbrOutput2.Status.Conditions)
requireConditionPresentAndTrue(t, BindingReady, sbrOutput2.Status.Conditions)

require.Equal(t, reconcilerName, sbrOutput2.Status.Secret)
require.Equal(t, 1, len(sbrOutput2.Status.Applications))
}

func TestApplicationNotFound(t *testing.T) {
backingServiceResourceRef := "backingService1"
matchLabels := map[string]string{
Expand Down Expand Up @@ -288,6 +343,7 @@ func TestApplicationNotFound(t *testing.T) {

requireConditionPresentAndTrue(t, CollectionReady, sbrOutput.Status.Conditions)
requireConditionPresentAndFalse(t, InjectionReady, sbrOutput.Status.Conditions)
requireConditionPresentAndFalse(t, BindingReady, sbrOutput.Status.Conditions)
require.Len(t, sbrOutput.Status.Applications, 0)

// Reconcile with deployment
Expand All @@ -310,6 +366,7 @@ func TestApplicationNotFound(t *testing.T) {

requireConditionPresentAndTrue(t, CollectionReady, sbrOutput2.Status.Conditions)
requireConditionPresentAndTrue(t, InjectionReady, sbrOutput2.Status.Conditions)
requireConditionPresentAndTrue(t, BindingReady, sbrOutput2.Status.Conditions)

require.Equal(t, reconcilerName, sbrOutput2.Status.Secret)
require.Equal(t, 1, len(sbrOutput2.Status.Applications))
Expand Down Expand Up @@ -361,6 +418,7 @@ func TestReconcilerUpdateCredentials(t *testing.T) {

requireConditionPresentAndTrue(t, CollectionReady, sbrOutput3.Status.Conditions)
requireConditionPresentAndTrue(t, InjectionReady, sbrOutput3.Status.Conditions)
requireConditionPresentAndTrue(t, BindingReady, sbrOutput3.Status.Conditions)

require.Equal(t, reconcilerName, sbrOutput3.Status.Secret)
require.Equal(t, s.Data["password"], []byte("abc123"))
Expand Down Expand Up @@ -420,6 +478,7 @@ func TestReconcilerReconcileWithConflictingAppSelc(t *testing.T) {

requireConditionPresentAndTrue(t, CollectionReady, sbrOutput.Status.Conditions)
requireConditionPresentAndTrue(t, InjectionReady, sbrOutput.Status.Conditions)
requireConditionPresentAndTrue(t, BindingReady, sbrOutput.Status.Conditions)

require.Equal(t, reconcilerName, sbrOutput.Status.Secret)
require.Len(t, sbrOutput.Status.Applications, 1)
Expand Down Expand Up @@ -449,6 +508,32 @@ func TestEmptyApplication(t *testing.T) {

requireConditionPresentAndTrue(t, CollectionReady, sbrOutput.Status.Conditions)
requireConditionPresentAndFalse(t, InjectionReady, sbrOutput.Status.Conditions)
requireConditionPresentAndTrue(t, BindingReady, sbrOutput.Status.Conditions)
}

// TestEmptyServiceSelector tests that CollectionReady,InjectionReady and BindingReady are all successfully updated to True when ServiceSelector is empty
func TestEmptyServiceSelectorAndAllConditionAreSetToFalse(t *testing.T) {
applicationResourceRef := "applicationRef"
f := mocks.NewFake(t, reconcilerNs)
f.AddMockedUnstructuredServiceBindingWithoutService(reconcilerName, applicationResourceRef, deploymentsGVR)

fakeDynClient := f.FakeDynClient()
mapper := testutils.BuildTestRESTMapper()
r := &reconciler{dynClient: fakeDynClient, restMapper: mapper, scheme: f.S}
r.resourceWatcher = newFakeResourceWatcher(mapper)

res, err := r.Reconcile(reconcileRequest())
require.NoError(t, err)
require.False(t, res.Requeue)

namespacedName := types.NamespacedName{Namespace: reconcilerNs, Name: reconcilerName}
sbrOutput, err := r.getServiceBinding(namespacedName)
require.NoError(t, err)

requireConditionPresentAndFalse(t, CollectionReady, sbrOutput.Status.Conditions)
requireConditionPresentAndFalse(t, InjectionReady, sbrOutput.Status.Conditions)
requireConditionPresentAndFalse(t, BindingReady, sbrOutput.Status.Conditions)

}

func TestBindTwoSbrsWithSingleApplication(t *testing.T) {
Expand Down Expand Up @@ -505,6 +590,7 @@ func TestBindTwoSbrsWithSingleApplication(t *testing.T) {

requireConditionPresentAndTrue(t, CollectionReady, sbrOutput.Status.Conditions)
requireConditionPresentAndTrue(t, InjectionReady, sbrOutput.Status.Conditions)
requireConditionPresentAndTrue(t, BindingReady, sbrOutput.Status.Conditions)
require.Equal(t, sbrName1, sbrOutput.Status.Secret)
require.Len(t, sbrOutput.Status.Applications, 1)
require.True(t, reflect.DeepEqual(expectedStatus, sbrOutput.Status.Applications[0]))
Expand Down Expand Up @@ -542,6 +628,7 @@ func TestBindTwoSbrsWithSingleApplication(t *testing.T) {

requireConditionPresentAndTrue(t, CollectionReady, sbrOutput.Status.Conditions)
requireConditionPresentAndTrue(t, InjectionReady, sbrOutput.Status.Conditions)
requireConditionPresentAndTrue(t, BindingReady, sbrOutput.Status.Conditions)
require.Equal(t, sbrName2, sbrOutput.Status.Secret)
require.Len(t, sbrOutput.Status.Applications, 1)
require.True(t, reflect.DeepEqual(expectedStatus, sbrOutput.Status.Applications[0]))
Expand Down
41 changes: 31 additions & 10 deletions pkg/controller/servicebinding/servicebinder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,10 @@ func (b *serviceBinder) onError(
Reason: bindingFail,
Message: b.message(err),
})
conditionsv1.SetStatusCondition(&sbrStatus.Conditions, conditionsv1.Condition{
Type: BindingReady,
Status: corev1.ConditionFalse,
})
newSbr, errStatus := b.updateStatusServiceBinding(sbr, sbrStatus)
if errStatus != nil {
return requeueError(errStatus)
Expand Down Expand Up @@ -274,14 +278,7 @@ func removeFinalizer(sbr *v1alpha1.ServiceBinding) {
// handleApplicationError handles scenarios when:
// 1. application not declared in the Service Binding
// 2. application not found
func (b *serviceBinder) handleApplicationError(reason string, applicationError error, sbrStatus *v1alpha1.ServiceBindingStatus) (reconcile.Result, error) {
conditionsv1.SetStatusCondition(&sbrStatus.Conditions, conditionsv1.Condition{
Type: InjectionReady,
Status: corev1.ConditionFalse,
Reason: reason,
Message: applicationError.Error(),
})

func (b *serviceBinder) handleApplicationError(applicationError error, sbrStatus *v1alpha1.ServiceBindingStatus) (reconcile.Result, error) {
// updating status of request instance
sbr, err := b.updateStatusServiceBinding(b.sbr, sbrStatus)
if err != nil {
Expand Down Expand Up @@ -320,13 +317,33 @@ func (b *serviceBinder) bind() (reconcile.Result, error) {
})

if isApplicationEmpty(b.sbr.Spec.Application) {
return b.handleApplicationError(EmptyApplicationReason, errEmptyApplication, sbrStatus)
conditionsv1.SetStatusCondition(&sbrStatus.Conditions, conditionsv1.Condition{
Type: InjectionReady,
Status: corev1.ConditionFalse,
Reason: EmptyApplicationReason,
Message: errEmptyApplication.Error(),
})
conditionsv1.SetStatusCondition(&sbrStatus.Conditions, conditionsv1.Condition{
Type: BindingReady,
Status: corev1.ConditionTrue,
})
return b.handleApplicationError(errEmptyApplication, sbrStatus)
}
updatedObjects, err := b.binder.bind()
if err != nil {
b.logger.Error(err, "On binding application.")
if errors.Is(err, errApplicationNotFound) {
return b.handleApplicationError(ApplicationNotFoundReason, errApplicationNotFound, sbrStatus)
conditionsv1.SetStatusCondition(&sbrStatus.Conditions, conditionsv1.Condition{
Type: InjectionReady,
Status: corev1.ConditionFalse,
Reason: ApplicationNotFoundReason,
Message: errApplicationNotFound.Error(),
})
conditionsv1.SetStatusCondition(&sbrStatus.Conditions, conditionsv1.Condition{
Type: BindingReady,
Status: corev1.ConditionFalse,
})
return b.handleApplicationError(errApplicationNotFound, sbrStatus)
}
return b.onError(err, b.sbr, sbrStatus, nil)
}
Expand All @@ -336,6 +353,10 @@ func (b *serviceBinder) bind() (reconcile.Result, error) {
Type: InjectionReady,
Status: corev1.ConditionTrue,
})
conditionsv1.SetStatusCondition(&sbrStatus.Conditions, conditionsv1.Condition{
Type: BindingReady,
Status: corev1.ConditionTrue,
})

// updating status of request instance
sbr, err := b.updateStatusServiceBinding(b.sbr, sbrStatus)
Expand Down
Loading

0 comments on commit 767b344

Please sign in to comment.