Use non-cached CR on reconciliation (#940)

Signed-off-by: Juraci Paixão Kröhling <[email protected]>

pkg/controller/jaeger/jaeger_controller.go

@@ -99,7 +99,7 @@ func (r *ReconcileJaeger) Reconcile(request reconcile.Request) (reconcile.Result
 
 	// Fetch the Jaeger instance
 	instance := &v1.Jaeger{}
-	err := r.client.Get(ctx, request.NamespacedName, instance)
+	err := r.rClient.Get(ctx, request.NamespacedName, instance)
 	if err != nil {
 		if k8serrors.IsNotFound(err) {
 			// Request object not found, could have been deleted after reconcile request.

pkg/controller/jaeger/jaeger_controller_test.go

@@ -7,6 +7,7 @@ import (
 	osv1 "github.com/openshift/api/route/v1"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
+	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -71,7 +72,7 @@ func TestDeletedInstance(t *testing.T) {
 	// no known objects
 	cl := fake.NewFakeClient()
 
-	r := &ReconcileJaeger{client: cl, scheme: s}
+	r := &ReconcileJaeger{client: cl, scheme: s, rClient: cl}
 
 	req := reconcile.Request{
 		NamespacedName: types.NamespacedName{
@@ -132,7 +133,7 @@ func TestSkipOnNonOwnedCR(t *testing.T) {
 	s := scheme.Scheme
 	s.AddKnownTypes(v1.SchemeGroupVersion, jaeger)
 	cl := fake.NewFakeClient(jaeger)
-	r := &ReconcileJaeger{client: cl, scheme: s}
+	r := &ReconcileJaeger{client: cl, scheme: s, rClient: cl}
 	req := reconcile.Request{NamespacedName: nsn}
 
 	// test
@@ -149,6 +150,35 @@ func TestSkipOnNonOwnedCR(t *testing.T) {
 	assert.Equal(t, v1.JaegerPhase(""), persisted.Status.Phase)
 }
 
+func TestGetResourceFromNonCachedClient(t *testing.T) {
+	// prepare
+	nsn := types.NamespacedName{Name: "my-instance"}
+	jaeger := v1.NewJaeger(nsn)
+
+	s := scheme.Scheme
+	s.AddKnownTypes(v1.SchemeGroupVersion, jaeger)
+
+	// simulates the case where the cache is stale: the instance has been deleted (client) but the cache hasn't been updated (cachedClient)
+	// we trigger the reconciliation and expect it to finish without errors, while we expect to not have an instance afterwards
+	// if the code is using the cached client, we would end up either with an error (trying to update an instance that does not exist)
+	// or we'd end up with an instance
+	cachedClient := fake.NewFakeClient(jaeger)
+	client := fake.NewFakeClient()
+
+	r := &ReconcileJaeger{client: cachedClient, scheme: s, rClient: client}
+	req := reconcile.Request{NamespacedName: nsn}
+
+	// test
+	_, err := r.Reconcile(req)
+
+	// verify
+	assert.NoError(t, err)
+	persisted := &v1.Jaeger{}
+	err = client.Get(context.Background(), req.NamespacedName, persisted)
+	assert.Error(t, err)
+	assert.True(t, errors.IsNotFound(err))
+}
+
 func getReconciler(objs []runtime.Object) (*ReconcileJaeger, client.Client) {
 	s := scheme.Scheme