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 Oct 9, 2020
1 parent f179d71 commit aca8433
Show file tree
Hide file tree
Showing 14 changed files with 245 additions and 26 deletions.
48 changes: 47 additions & 1 deletion 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 @@ -136,6 +142,10 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err
Reason: EmptyServiceSelectorsReason,
Message: errEmptyServices.Error(),
},
conditionsv1.Condition{
Type: BindingReady,
Status: corev1.ConditionFalse,
},
)
if updateErr == nil {
return done()
Expand All @@ -157,9 +167,27 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err
r.restMapper,
)
if err != nil {
//handle service not found error
if k8serrors.IsNotFound(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,
},
)
}
return requeueError(err)
}

binding, err := buildBinding(
r.dynClient,
sbr.Spec.CustomEnvVar,
Expand Down Expand Up @@ -220,3 +248,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
8 changes: 1 addition & 7 deletions pkg/controller/servicebinding/sbrcontroller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,6 @@ func updateFunc(logger *log.Log) func(updateEvent event.UpdateEvent) bool {

shouldReconcile := !specsAreEqual || !statusAreEqual

logger.Debug("Resource update event received",
"GVK", e.ObjectNew.GetObjectKind().GroupVersionKind(),
"Name", e.MetaNew.GetName(),
"SpecsAreEqual", specsAreEqual,
"StatusAreEqual", statusAreEqual,
"ShouldReconcile", shouldReconcile,
)
return shouldReconcile
}
}
Expand All @@ -162,6 +155,7 @@ func buildGVKPredicate(logger *log.Log) predicate.Funcs {
UpdateFunc: updateFunc(logger),
DeleteFunc: func(e event.DeleteEvent) bool {
// evaluates to false if the object has been confirmed deleted
logger.Debug("Resource update event received", "GVK", e.Object.GetObjectKind(), "name", e.Meta.GetName())
return !e.DeleteStateUnknown
},
}
Expand Down
43 changes: 33 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,12 @@ func (b *serviceBinder) onError(
Reason: bindingFail,
Message: b.message(err),
})
conditionsv1.SetStatusCondition(&sbrStatus.Conditions, conditionsv1.Condition{
Type: BindingReady,
Status: corev1.ConditionFalse,
Reason: bindingFail,
Message: b.message(err),
})
newSbr, errStatus := b.updateStatusServiceBinding(sbr, sbrStatus)
if errStatus != nil {
return requeueError(errStatus)
Expand Down Expand Up @@ -274,14 +280,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 +319,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 +355,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 aca8433

Please sign in to comment.