From f9d37cc029515804df567045c9f161f0d5637180 Mon Sep 17 00:00:00 2001 From: Abu Kashem Date: Mon, 25 Feb 2019 15:41:33 -0500 Subject: [PATCH] [opsrc] Do not delete csc during purge On restart, marketplace operator deletes and recreates the CatalogSourceConfig object(s) associated with each OperatorSource. These are CatalogSourceConfig object(s) having the following label: opsrc-datastore: "true" Resolution: - Do not delete CatalogSourceConfig object(s) during purge, instead let the CatalogSourceConfig(s) be updated during the reconciliation process ( in configuring phase ). - Make the log a bit clearer by mentioning whether it is creating or updating a CatalogSource object in 'Configuring' phase. https://jira.coreos.com/browse/MKTPLC-236 https://bugzilla.redhat.com/show_bug.cgi?id=1682928 --- pkg/operatorsource/configuring.go | 6 ++--- pkg/operatorsource/purging.go | 14 +++------- pkg/operatorsource/purging_test.go | 42 ------------------------------ 3 files changed, 6 insertions(+), 56 deletions(-) diff --git a/pkg/operatorsource/configuring.go b/pkg/operatorsource/configuring.go index f8cd332b2..078a979d3 100644 --- a/pkg/operatorsource/configuring.go +++ b/pkg/operatorsource/configuring.go @@ -78,7 +78,7 @@ func (r *configuringReconciler) Reconcile(ctx context.Context, in *v1alpha1.Oper if err == nil { nextPhase = phase.GetNext(phase.Succeeded) - r.logger.Info("The object has been successfully reconciled") + r.logger.Info("CatalogSourceConfig object has been created successfully") return } @@ -115,8 +115,8 @@ func (r *configuringReconciler) Reconcile(ctx context.Context, in *v1alpha1.Oper return } - nextPhase = phase.GetNext(phase.Succeeded) - r.logger.Info("The object has been successfully reconciled") + r.logger.Info("CatalogSourceConfig object has been updated successfully") + nextPhase = phase.GetNext(phase.Succeeded) return } diff --git a/pkg/operatorsource/purging.go b/pkg/operatorsource/purging.go index 3a7d188e4..b6156ff8f 100644 --- a/pkg/operatorsource/purging.go +++ b/pkg/operatorsource/purging.go @@ -7,7 +7,6 @@ import ( "github.com/operator-framework/operator-marketplace/pkg/datastore" "github.com/operator-framework/operator-marketplace/pkg/phase" log "github.com/sirupsen/logrus" - k8s_errors "k8s.io/apimachinery/pkg/api/errors" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -52,19 +51,12 @@ func (r *purgingReconciler) Reconcile(ctx context.Context, in *v1alpha1.Operator out = in.DeepCopy() - r.logger.Info("Purging all resource(s)") + // We will purge the datastore and leave the CatalogSourceConfig object + // alone. It will be updated accordingly by the reconciliation loop. r.datastore.RemoveOperatorSource(in.GetUID()) - builder := &CatalogSourceConfigBuilder{} - csc := builder.WithTypeMeta(). - WithNamespacedName(in.Namespace, in.Name). - CatalogSourceConfig() - - if err = r.client.Delete(ctx, csc); err != nil && !k8s_errors.IsNotFound(err) { - nextPhase = phase.GetNextWithMessage(phase.OperatorSourcePurging, err.Error()) - return - } + r.logger.Info("Purged datastore. No change(s) were made to corresponding CatalogSourceConfig") // Since all observable states stored in the Status resource might already // be stale, we should Reset everything in Status except for 'Current Phase' diff --git a/pkg/operatorsource/purging_test.go b/pkg/operatorsource/purging_test.go index 9dfbfecba..5026bd280 100644 --- a/pkg/operatorsource/purging_test.go +++ b/pkg/operatorsource/purging_test.go @@ -5,8 +5,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - k8s_errors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime/schema" gomock "github.com/golang/mock/gomock" "github.com/operator-framework/operator-marketplace/pkg/apis/marketplace/v1alpha1" @@ -37,51 +35,11 @@ func TestReconcileWithPurging(t *testing.T) { reconciler := operatorsource.NewPurgingReconciler(helperGetContextLogger(), datastore, client) // We expect the operator source to be removed from the datastore. - csc := helperNewCatalogSourceConfig(opsrcIn.Namespace, opsrcIn.Name) datastore.EXPECT().RemoveOperatorSource(opsrcIn.GetUID()).Times(1) - // We expect the associated CatalogConfigSource object to be deleted. - client.EXPECT().Delete(ctx, csc) - opsrcGot, nextPhaseGot, errGot := reconciler.Reconcile(ctx, opsrcIn) assert.NoError(t, errGot) assert.Equal(t, opsrcWant, opsrcGot) assert.Equal(t, nextPhaseWant, nextPhaseGot) } - -// In the event the associated CatalogSourceConfig object is not found while -// purging is in progress, we expect NotFound error to be ignored and the next -// phase set to "Initial" so that reconciliation can start anew. -func TestReconcileWithPurgingWithCatalogSourceConfigNotFound(t *testing.T) { - controller := gomock.NewController(t) - defer controller.Finish() - - ctx := context.TODO() - - opsrcIn := helperNewOperatorSourceWithPhase("marketplace", "foo", phase.OperatorSourcePurging) - opsrcWant := opsrcIn.DeepCopy() - - nextPhaseWant := &v1alpha1.Phase{ - Name: phase.Initial, - Message: phase.GetMessage(phase.Initial), - } - - datastore := mocks.NewDatastoreWriter(controller) - client := mocks.NewKubeClient(controller) - reconciler := operatorsource.NewPurgingReconciler(helperGetContextLogger(), datastore, client) - - // We expect the operator source to be removed from the datastore. - csc := helperNewCatalogSourceConfig(opsrcIn.Namespace, opsrcIn.Name) - datastore.EXPECT().RemoveOperatorSource(opsrcIn.GetUID()) - - // We expect kube client to throw a NotFound error. - notFoundErr := k8s_errors.NewNotFound(schema.GroupResource{}, "CatalogSourceConfig not found") - client.EXPECT().Delete(ctx, csc).Return(notFoundErr) - - opsrcGot, nextPhaseGot, errGot := reconciler.Reconcile(ctx, opsrcIn) - - assert.Error(t, errGot) - assert.Equal(t, opsrcGot, opsrcWant) - assert.Equal(t, nextPhaseWant, nextPhaseGot) -}