fix issue Credential was deleted due to "Failed to update online stat…

…us" IBM#217

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

controllers/binding_controller.go

@@ -37,6 +37,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/util/retry"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
@@ -278,7 +279,7 @@ func (r *BindingReconciler) Reconcile(request ctrl.Request) (ctrl.Result, error)
 			return r.updateStatusError(instance, bindingStateFailed, err)
 		}
 
-		return r.updateStatusOnline(session, instance, serviceInstance, serviceClassType)
+		return r.updateStatusOnline(session, instance)
 	}
 
 	// The KeyInstanceID has been set (or is still inProgress), verify that the key and secret still exist
@@ -313,7 +314,7 @@ func (r *BindingReconciler) Reconcile(request ctrl.Request) (ctrl.Result, error)
 			logt.Info("Error creating secret", instance.Name, err.Error())
 			return r.updateStatusError(instance, bindingStateFailed, err)
 		}
-		return r.updateStatusOnline(session, instance, serviceInstance, serviceClassType)
+		return r.updateStatusOnline(session, instance)
 	}
 
 	// The secret exists, make sure it has the right content
@@ -335,9 +336,9 @@ func (r *BindingReconciler) Reconcile(request ctrl.Request) (ctrl.Result, error)
 			logt.Info("Error re-creating secret", instance.Name, err.Error())
 			return r.updateStatusError(instance, bindingStateFailed, err)
 		}
-		return r.updateStatusOnline(session, instance, serviceInstance, serviceClassType)
+		return r.updateStatusOnline(session, instance)
 	}
-	return r.updateStatusOnline(session, instance, serviceInstance, serviceClassType)
+	return r.updateStatusOnline(session, instance)
 }
 
 func (r *BindingReconciler) getServiceInstance(instance *ibmcloudv1.Binding) (*ibmcloudv1.Service, error) {
@@ -507,19 +508,23 @@ func (r *BindingReconciler) createSecret(instance *ibmcloudv1.Binding, keyConten
 	return nil
 }
 
-func (r *BindingReconciler) updateStatusOnline(session *session.Session, instance *ibmcloudv1.Binding, serviceInstance *ibmcloudv1.Service, serviceClassType string) (ctrl.Result, error) {
-	instance.Status.State = bindingStateOnline
-	instance.Status.Message = bindingStateOnline
-	instance.Status.SecretName = getSecretName(instance)
-	err := r.Status().Update(context.Background(), instance)
-	if err != nil {
-		r.Log.Info("Failed to update online status, will delete external resource ", instance.ObjectMeta.Name, err.Error())
-		err = r.deleteCredentials(session, instance, serviceClassType)
+func (r *BindingReconciler) updateStatusOnline(session *session.Session, instance *ibmcloudv1.Binding) (ctrl.Result, error) {
+	err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
+		currentBindingInstance := &ibmcloudv1.Binding{}
+		err := r.Get(context.Background(), types.NamespacedName{Namespace: instance.Namespace, Name: instance.Name}, currentBindingInstance)
 		if err != nil {
-			r.Log.Info("Failed to delete external resource, operator state and external resource might be in an inconsistent state", instance.ObjectMeta.Name, err.Error())
+			r.Log.Error(err, "Failed to fetch binding instance", "namespace", instance.Namespace, "name", instance.Name)
+			return err
 		}
+		currentBindingInstance.Status.State = bindingStateOnline
+		currentBindingInstance.Status.Message = bindingStateOnline
+		currentBindingInstance.Status.SecretName = getSecretName(currentBindingInstance)
+		return r.Status().Update(context.Background(), currentBindingInstance)
+	})
+	if err != nil {
+		r.Log.Error(err, "Failed to update binding instance after retry", "namespace", instance.Namespace, "name", instance.Name)
+		return ctrl.Result{Requeue: true}, err
 	}
-
 	return ctrl.Result{Requeue: true, RequeueAfter: config.Get().SyncPeriod}, nil
 }
 

controllers/binding_controller_test.go

@@ -2252,7 +2252,7 @@ func TestBindingDeleteCredentials(t *testing.T) {
 	}
 }
 
-func TestBindingUpdateStatusOnlineFailed(t *testing.T) {
+func TestBindingUpdateStatusOnlineFailedWithConflictError(t *testing.T) {
 	t.Parallel()
 	scheme := schemas(t)
 	binding := &ibmcloudv1.Binding{
@@ -2264,9 +2264,19 @@ func TestBindingUpdateStatusOnlineFailed(t *testing.T) {
 		Spec:       ibmcloudv1.ServiceSpec{},
 	}
 
+	errChan := make(chan error, 10)
+
+	//It return conflict error at first so retry will be triggered and succeed with no error and the function will succeed
+	errChan <- &k8sErrors.StatusError{metav1.Status{
+		Status: metav1.StatusFailure,
+		Code:   409,
+		Reason: metav1.StatusReasonConflict,
+	}}
+	errChan <- nil
+
 	client := newMockClient(
 		fake.NewFakeClientWithScheme(scheme, binding, service),
-		MockConfig{StatusUpdateErr: fmt.Errorf("status failed")},
+		MockConfig{ErrChan: errChan},
 	)
 	r := &BindingReconciler{
 		Client: client,
@@ -2278,21 +2288,48 @@ func TestBindingUpdateStatusOnlineFailed(t *testing.T) {
 		},
 	}
 
-	result, err := r.updateStatusOnline(nil, binding, service, "")
+	result, err := r.updateStatusOnline(nil, binding)
 	assert.Equal(t, ctrl.Result{
 		Requeue:      true,
 		RequeueAfter: config.Get().SyncPeriod,
 	}, result)
 	assert.NoError(t, err)
-	assert.Equal(t, &ibmcloudv1.Binding{
+}
+
+func TestBindingUpdateStatusOnlineFailedWithOtherErrror(t *testing.T) {
+	t.Parallel()
+	scheme := schemas(t)
+	binding := &ibmcloudv1.Binding{
 		ObjectMeta: metav1.ObjectMeta{Name: "myservice", Namespace: "mynamespace"},
-		Status: ibmcloudv1.BindingStatus{
-			State:      bindingStateOnline,
-			Message:    bindingStateOnline,
-			SecretName: "myservice",
+		Spec:       ibmcloudv1.BindingSpec{},
+	}
+	service := &ibmcloudv1.Service{
+		ObjectMeta: metav1.ObjectMeta{Name: "myservice", Namespace: "mynamespace"},
+		Spec:       ibmcloudv1.ServiceSpec{},
+	}
+
+	errChan := make(chan error, 10)
+
+	errChan <- fmt.Errorf("status failed")
+	client := newMockClient(
+		fake.NewFakeClientWithScheme(scheme, binding, service),
+		MockConfig{ErrChan: errChan},
+	)
+	r := &BindingReconciler{
+		Client: client,
+		Log:    testLogger(t),
+		Scheme: scheme,
+
+		DeleteResourceServiceKey: func(session *session.Session, keyID string) error {
+			return fmt.Errorf("failed")
 		},
-		Spec: ibmcloudv1.BindingSpec{},
-	}, client.LastStatusUpdate())
+	}
+
+	result, err := r.updateStatusOnline(nil, binding)
+	assert.Equal(t, ctrl.Result{
+		Requeue: true,
+	}, result)
+	assert.Error(t, err)
 }
 
 func TestBindingSetupWithManager(t *testing.T) {

controllers/mock_client_test.go

@@ -45,6 +45,8 @@ type MockConfig struct {
 	StatusPatchErr  error
 	StatusUpdateErr error
 	UpdateErr       error
+
+	ErrChan chan error
 }
 
 func newMockClient(client client.Client, config MockConfig) MockClient {
@@ -107,7 +109,13 @@ func (m *mockClient) Status() client.StatusWriter {
 
 func (s *mockStatusWriter) Update(ctx context.Context, obj runtime.Object, opts ...client.UpdateOption) error {
 	s.lastStatusUpdate = obj.DeepCopyObject()
-	return s.StatusUpdateErr
+	if s.ErrChan != nil {
+		err := <-s.ErrChan
+		return err
+	} else {
+		return s.StatusUpdateErr
+
+	}
 }
 
 func (m *mockClient) LastStatusUpdate() runtime.Object {