fix issue SBR can not be deleted if backend service has been deleted #832

Signed-off-by: qibobo <[email protected]>

Author: qibobo

pkg/controller/servicebinding/reconciler.go

@@ -165,29 +165,34 @@ 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)
+		if sbr.GetDeletionTimestamp() != nil {
+			serviceCtxs = serviceContextList{}
+		} else {
+			//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)
 		}
-		return requeueError(err)
+
 	}
 	binding, err := buildBinding(
 		r.dynClient,

pkg/controller/servicebinding/reconciler_test.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"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/testutils"
 	"github.com/stretchr/testify/require"
 	appsv1 "k8s.io/api/apps/v1"
@@ -648,3 +649,98 @@ func TestBindTwoSbrsWithSingleApplication(t *testing.T) {
 		require.Equal(t, sbrName2, dep.Spec.Template.Spec.Containers[0].EnvFrom[1].SecretRef.LocalObjectReference.Name)
 	})
 }
+
+// TestApplicationByName tests discovery of application by name
+func TestUnBind(t *testing.T) {
+	//create a successful SBR first
+	backingServiceResourceRef := "test-using-secret"
+	matchLabels := map[string]string{
+		"connects-to": "database",
+		"environment": "reconciler",
+	}
+	f := mocks.NewFake(t, reconcilerNs)
+	f.AddMockedUnstructuredServiceBinding(reconcilerName, backingServiceResourceRef, reconcilerName, deploymentsGVR, matchLabels)
+	f.AddMockedUnstructuredCSV("cluster-service-version-list")
+	f.AddMockedUnstructuredDatabaseCRD()
+	f.AddMockedUnstructuredDatabaseCR(backingServiceResourceRef)
+	f.AddMockedUnstructuredDeployment(reconcilerName, matchLabels)
+	f.AddMockedUnstructuredSecret("db-credentials")
+
+	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}
+
+	u, err := fakeDynClient.Resource(deploymentsGVR).Get(reconcilerName, metav1.GetOptions{})
+	require.NoError(t, err)
+
+	d := appsv1.Deployment{}
+	err = runtime.DefaultUnstructuredConverter.FromUnstructured(u.Object, &d)
+	require.NoError(t, err)
+
+	containers := d.Spec.Template.Spec.Containers
+	require.Equal(t, 1, len(containers))
+	require.Equal(t, 1, len(containers[0].EnvFrom))
+	require.NotNil(t, containers[0].EnvFrom[0].SecretRef)
+	require.Equal(t, reconcilerName, containers[0].EnvFrom[0].SecretRef.Name)
+
+	namespacedName = types.NamespacedName{Namespace: reconcilerNs, Name: reconcilerName}
+	sbrOutput, err := r.getServiceBinding(namespacedName)
+	require.NoError(t, err)
+
+	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.Equal(t, 1, len(sbrOutput.Status.Applications))
+	expectedStatus := v1alpha1.BoundApplication{
+		GroupVersionKind: v1.GroupVersionKind{
+			Group:   deploymentsGVR.Group,
+			Version: deploymentsGVR.Version,
+			Kind:    "Deployment",
+		},
+		LocalObjectReference: corev1.LocalObjectReference{
+			Name: namespacedName.Name,
+		},
+	}
+	require.True(t, reflect.DeepEqual(expectedStatus, sbrOutput.Status.Applications[0]))
+
+	//do the unbind
+	nsClient := fakeDynClient.
+		Resource(groupVersion).
+		Namespace(reconcilerNs)
+	u, err = nsClient.Get(reconcilerName, metav1.GetOptions{})
+	require.NoError(t, err)
+
+	sbr := &v1alpha1.ServiceBinding{}
+	err = runtime.DefaultUnstructuredConverter.FromUnstructured(u.Object, sbr)
+	require.NoError(t, err)
+	deletionTime := metav1.Now()
+	sbr.SetDeletionTimestamp(&deletionTime)
+	u, err = converter.ToUnstructured(sbr)
+	require.NoError(t, err)
+	_, err = nsClient.Update(u, v1.UpdateOptions{})
+	require.NoError(t, err)
+	res, err = r.Reconcile(reconcileRequest())
+	require.NoError(t, err)
+	require.False(t, res.Requeue)
+
+	u, err = fakeDynClient.Resource(deploymentsGVR).Get(reconcilerName, metav1.GetOptions{})
+	require.NoError(t, err)
+
+	d = appsv1.Deployment{}
+	err = runtime.DefaultUnstructuredConverter.FromUnstructured(u.Object, &d)
+	require.NoError(t, err)
+
+	containers = d.Spec.Template.Spec.Containers
+	require.Equal(t, 1, len(containers))
+	require.Equal(t, 0, len(containers[0].EnvFrom))
+}

test/acceptance/features/unbindAppToService.feature

@@ -49,3 +49,60 @@ Feature: Unbind an application from a service
 
         Then The env var "BACKEND_HOST" is not available to the application
         And The env var "BACKEND_USERNAME" is not available to the application
+
+
+    Scenario: Unbind a generic test application from the backing service when the backing service has been deleted
+        Given OLM Operator "backend" is running
+        * The Custom Resource is present
+            """
+            apiVersion: "stable.example.com/v1"
+            kind: Backend
+            metadata:
+                name: example-backend
+                annotations:
+                    service.binding/host: path={.spec.host}
+                    service.binding/username: path={.spec.username}
+            spec:
+                host: example.com
+                username: foo
+            """
+        * Generic test application "generic-app-a-d-u" is running
+        * Service Binding is applied
+            """
+            apiVersion: operators.coreos.com/v1alpha1
+            kind: ServiceBinding
+            metadata:
+                name: binding-request-a-d-u
+            spec:
+                application:
+                    name: generic-app-a-d-u
+                    group: apps
+                    version: v1
+                    resource: deployments
+                services:
+                -   group: stable.example.com
+                    version: v1
+                    kind: Backend
+                    name: example-backend
+                    id: backend
+            """
+        * The application env var "BACKEND_HOST" has value "example.com"
+        * The application env var "BACKEND_USERNAME" has value "foo"
+
+        When BackingService is deleted
+          """
+          apiVersion: "stable.example.com/v1"
+          kind: Backend
+          metadata:
+              name: example-backend
+              annotations:
+                  service.binding/host: path={.spec.host}
+                  service.binding/username: path={.spec.username}
+          spec:
+              host: example.com
+              username: foo
+          """
+        When Service binding "binding-request-a-d-u" is deleted
+
+        Then The env var "BACKEND_HOST" is not available to the application
+        And The env var "BACKEND_USERNAME" is not available to the application