Skip to content

Commit

Permalink
[opsrc] Do not delete csc during purge
Browse files Browse the repository at this point in the history
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.

- 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
  • Loading branch information
tkashem committed Feb 25, 2019
1 parent 7b53305 commit c582a47
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 56 deletions.
6 changes: 3 additions & 3 deletions pkg/operatorsource/configuring.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
11 changes: 0 additions & 11 deletions pkg/operatorsource/purging.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -56,16 +55,6 @@ func (r *purgingReconciler) Reconcile(ctx context.Context, in *v1alpha1.Operator

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
}

// Since all observable states stored in the Status resource might already
// be stale, we should Reset everything in Status except for 'Current Phase'
// to their default values.
Expand Down
42 changes: 0 additions & 42 deletions pkg/operatorsource/purging_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}

0 comments on commit c582a47

Please sign in to comment.