Repository: redhat-developer/service-binding-operator
Pull request #98 (2019-08-05 16:51:45)

Exposes service binding request status
Partially solves: https://jira.coreos.com/browse/ODC-1327

This PR exposes the binding status - `success`, `failure`, `inProgress`

To Do:
- [x] Add tests

Note:
This PR exposes only one status field `BindingStatus` . There will be follow up PR for exposing other fields.

openshift-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **dhritishikhar**
You can assign the PR to them by writing `/assign @dhritishikhar` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=redhat-developer%2Fservice-binding-operator).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/redhat-developer/service-binding-operator/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["dhritishikhar"]} -->
openshift-ci-robot: @DhritiShikhar: The following test **failed**, say `/retest` to rerun them all:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
ci/prow/unit | 0a5afeed3c4b3569918a43a7c495a0642c0a7756 | [link](https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/redhat-developer_service-binding-operator/98/pull-ci-redhat-developer-service-binding-operator-master-unit/242) | `/test unit`

[Full PR test history](https://prow.svc.ci.openshift.org/pr-history?org=redhat-developer&repo=service-binding-operator&pr=98). [Your PR dashboard](https://prow.svc.ci.openshift.org/pr?query=is:pr+state:open+author:DhritiShikhar). Please help us cut down on flakes by [linking to](https://github.com/kubernetes/community/blob/master/contributors/devel/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/redhat-developer/service-binding-operator/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
openshift-ci-robot: @DhritiShikhar: The following test **failed**, say `/retest` to rerun them all:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
ci/prow/unit | ffb3a57d3c3c6a7965ac2d71eb5298afa3978c1c | [link](https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/redhat-developer_service-binding-operator/98/pull-ci-redhat-developer-service-binding-operator-master-unit/243) | `/test unit`

[Full PR test history](https://prow.svc.ci.openshift.org/pr-history?org=redhat-developer&repo=service-binding-operator&pr=98). [Your PR dashboard](https://prow.svc.ci.openshift.org/pr?query=is:pr+state:open+author:DhritiShikhar). Please help us cut down on flakes by [linking to](https://github.com/kubernetes/community/blob/master/contributors/devel/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/redhat-developer/service-binding-operator/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
sbose78: To ensure that update of status don't trigger reconcile, you need to filter out that event by using Predicates

https://github.com/operator-framework/operator-sdk/blob/master/doc/user/event-filtering.md#using-predicates
sbose78: .. ie, this https://github.com/redhat-developer/service-binding-operator/pull/99/files
codecov[bot]: # [Codecov](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/98?src=pr&el=h1) Report
> Merging [#98](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/98?src=pr&el=desc) into [master](https://codecov.io/gh/redhat-developer/service-binding-operator/commit/5e13c0de1532b88aa8e036232bef4b61ccc416f0?src=pr&el=desc) will **decrease** coverage by `1.76%`.
> The diff coverage is `18.75%`.

[![Impacted file tree graph](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/98/graphs/tree.svg?width=650&token=CICBeVWlrC&height=150&src=pr)](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/98?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##           master     #98      +/-   ##
=========================================
- Coverage   66.56%   64.8%   -1.77%     
=========================================
  Files           5       6       +1     
  Lines         338     358      +20     
=========================================
+ Hits          225     232       +7     
- Misses         88     101      +13     
  Partials       25      25
```


| [Impacted Files](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/98?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/controller/servicebindingrequest/reconciler.go](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/98/diff?src=pr&el=tree#diff-cGtnL2NvbnRyb2xsZXIvc2VydmljZWJpbmRpbmdyZXF1ZXN0L3JlY29uY2lsZXIuZ28=) | `53.62% <10%> (-3.41%)` | :arrow_down: |
| [pkg/controller/servicebindingrequest/status.go](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/98/diff?src=pr&el=tree#diff-cGtnL2NvbnRyb2xsZXIvc2VydmljZWJpbmRpbmdyZXF1ZXN0L3N0YXR1cy5nbw==) | `33.33% <33.33%> (ø)` | |
| [pkg/controller/servicebindingrequest/retriever.go](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/98/diff?src=pr&el=tree#diff-cGtnL2NvbnRyb2xsZXIvc2VydmljZWJpbmRpbmdyZXF1ZXN0L3JldHJpZXZlci5nbw==) | `84.25% <0%> (+0.51%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/98?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/98?src=pr&el=footer). Last update [5e13c0d...e9b6f5d](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/98?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

baijum: /lgtm
/approve
openshift-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/redhat-developer/service-binding-operator/pull/98#issuecomment-519056604" title="Approved">baijum</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=redhat-developer%2Fservice-binding-operator).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/redhat-developer/service-binding-operator/blob/master/OWNERS)~~ [baijum]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
baijum: /hold
DhritiShikhar: /hold cancel

Patch:
diff --git a/pkg/apis/apps/v1alpha1/servicebindingrequest_types.go b/pkg/apis/apps/v1alpha1/servicebindingrequest_types.go
index 5d11b361..1793b315 100644
--- a/pkg/apis/apps/v1alpha1/servicebindingrequest_types.go
+++ b/pkg/apis/apps/v1alpha1/servicebindingrequest_types.go
@@ -71,9 +71,20 @@ type ApplicationSelector struct {
 	ResourceKind string            `json:"resourceKind"`
 }
 
+type BindingStatus string
+
+const (
+	BindingSuccess    BindingStatus = "success"
+	BindingInProgress BindingStatus = "inProgress"
+	BindingFail       BindingStatus = "fail"
+)
+
 // ServiceBindingRequestStatus defines the observed state of ServiceBindingRequest
 // +k8s:openapi-gen=true
-type ServiceBindingRequestStatus struct{}
+type ServiceBindingRequestStatus struct {
+	// BindingStatus is the status of the service binding request. Possible values are Success, Failure, InProgress.
+	BindingStatus BindingStatus `json:"BindingStatus"`
+}
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 
diff --git a/pkg/controller/servicebindingrequest/reconciler.go b/pkg/controller/servicebindingrequest/reconciler.go
index a28b17cd..896ee0d4 100644
--- a/pkg/controller/servicebindingrequest/reconciler.go
+++ b/pkg/controller/servicebindingrequest/reconciler.go
@@ -104,6 +104,12 @@ func (r *Reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err
 	instance := &v1alpha1.ServiceBindingRequest{}
 	err := r.client.Get(ctx, request.NamespacedName, instance)
 	if err != nil {
+		// Update Status
+		r.setBindingInProgressStatus(instance)
+		err = r.client.Status().Update(context.TODO(), instance)
+		if err != nil {
+			return reconcile.Result{}, err
+		}
 		return RequeueOnNotFound(err)
 	}
 
@@ -113,11 +119,23 @@ func (r *Reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err
 	plnr := planner.NewPlanner(ctx, r.client, request.Namespace, instance)
 	plan, err := plnr.Plan()
 	if err != nil {
+		// Update Status
+		r.setBindingInProgressStatus(instance)
+		err = r.client.Status().Update(context.TODO(), instance)
+		if err != nil {
+			return reconcile.Result{}, err
+		}
 		return RequeueOnNotFound(err)
 	}
 
 	retriever := NewRetriever(ctx, r.client, plan, instance.Spec.EnvVarPrefix)
 	if err = retriever.Retrieve(); err != nil {
+		// Update Status
+		r.setBindingInProgressStatus(instance)
+		err = r.client.Status().Update(context.TODO(), instance)
+		if err != nil {
+			return reconcile.Result{}, err
+		}
 		return RequeueOnNotFound(err)
 	}
 
@@ -143,11 +161,23 @@ func (r *Reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err
 		deploymentConfigListObj := &osappsv1.DeploymentConfigList{}
 		err = r.client.List(ctx, &searchByLabelsOpts, deploymentConfigListObj)
 		if err != nil {
+			// Update Status
+			r.setBindingInProgressStatus(instance)
+			err = r.client.Status().Update(context.TODO(), instance)
+			if err != nil {
+				return reconcile.Result{}, err
+			}
 			return RequeueOnNotFound(err)
 		}
 
 		if len(deploymentConfigListObj.Items) == 0 {
 			logger.Info("No DeploymentConfig objects found, requeueing request!")
+			// Update Status
+			r.setBindingInProgressStatus(instance)
+			err = r.client.Status().Update(context.TODO(), instance)
+			if err != nil {
+				return reconcile.Result{}, err
+			}
 			return Requeue()
 		}
 
@@ -178,6 +208,12 @@ func (r *Reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err
 			err = r.client.Update(ctx, &deploymentConfigObj)
 			if err != nil {
 				logger.Error(err, "Error on updating object!")
+				// Update Status
+				r.setBindingFailStatus(instance)
+				err = r.client.Status().Update(context.TODO(), instance)
+				if err != nil {
+					return reconcile.Result{}, err
+				}
 				return reconcile.Result{}, err
 			}
 		}
@@ -187,11 +223,23 @@ func (r *Reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err
 		deploymentListObj := &extv1beta1.DeploymentList{}
 		err = r.client.List(ctx, &searchByLabelsOpts, deploymentListObj)
 		if err != nil {
+			// Update Status
+			r.setBindingInProgressStatus(instance)
+			err = r.client.Status().Update(context.TODO(), instance)
+			if err != nil {
+				return reconcile.Result{}, err
+			}
 			return RequeueOnNotFound(err)
 		}
 
 		if len(deploymentListObj.Items) == 0 {
 			logger.Info("No Deployment objects found, requeueing request!")
+			// Update Status
+			r.setBindingInProgressStatus(instance)
+			err = r.client.Status().Update(context.TODO(), instance)
+			if err != nil {
+				return reconcile.Result{}, err
+			}
 			return Requeue()
 		}
 
@@ -223,12 +271,24 @@ func (r *Reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err
 			logger.Info("Updating Deployment object")
 			err = r.client.Update(ctx, &deploymentObj)
 			if err != nil {
+				// Update Status
+				r.setBindingFailStatus(instance)
+				err = r.client.Status().Update(context.TODO(), instance)
+				if err != nil {
+					return reconcile.Result{}, err
+				}
 				logger.Error(err, "Error on updating object!")
 				return reconcile.Result{}, err
 			}
 		}
 	}
 
+	// Update Status
+	r.setBindingSuccessStatus(instance)
+	err = r.client.Status().Update(context.TODO(), instance)
+	if err != nil {
+		return reconcile.Result{}, err
+	}
 	logger.Info("All done!")
 	return Done()
 }
diff --git a/pkg/controller/servicebindingrequest/status.go b/pkg/controller/servicebindingrequest/status.go
new file mode 100644
index 00000000..4744b8d3
--- /dev/null
+++ b/pkg/controller/servicebindingrequest/status.go
@@ -0,0 +1,17 @@
+package servicebindingrequest
+
+import (
+	"github.com/redhat-developer/service-binding-operator/pkg/apis/apps/v1alpha1"
+)
+
+func (r *Reconciler) setBindingSuccessStatus(sbr *v1alpha1.ServiceBindingRequest) {
+	sbr.Status.BindingStatus = v1alpha1.BindingSuccess
+}
+
+func (r *Reconciler) setBindingInProgressStatus(sbr *v1alpha1.ServiceBindingRequest) {
+	sbr.Status.BindingStatus = v1alpha1.BindingInProgress
+}
+
+func (r *Reconciler) setBindingFailStatus(sbr *v1alpha1.ServiceBindingRequest) {
+	sbr.Status.BindingStatus = v1alpha1.BindingFail
+}


Test patch:
diff --git a/pkg/controller/servicebindingrequest/reconciler_test.go b/pkg/controller/servicebindingrequest/reconciler_test.go
index c6c745fe..47a99283 100644
--- a/pkg/controller/servicebindingrequest/reconciler_test.go
+++ b/pkg/controller/servicebindingrequest/reconciler_test.go
@@ -84,6 +84,9 @@ func TestReconcilerNew(t *testing.T) {
 		assert.NotNil(t, containers[0].EnvFrom[0].SecretRef)
 		assert.Equal(t, reconcilerName, containers[0].EnvFrom[0].SecretRef.Name)
 
+		sbrOutput := v1alpha1.ServiceBindingRequest{}
+		require.Nil(t, reconcilerFakeClient.Get(context.TODO(), namespacedName, &sbrOutput))
+		require.Equal(t, v1alpha1.BindingSuccess, sbrOutput.Status.BindingStatus)
 	})
 
 }
@@ -153,7 +156,5 @@ func TestReconcilerVolumeMount(t *testing.T) {
 		assert.Equal(t, 1, len(volumes))
 		assert.Equal(t, reconcilerName, volumes[0].Name)
 		assert.Equal(t, reconcilerName, volumes[0].VolumeSource.Secret.SecretName)
-
 	})
-
 }