Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bug 1908431: Preserve custom catsrc w/ default catsrc name on restart #373

Merged
merged 1 commit into from Jan 15, 2021
Merged

Bug 1908431: Preserve custom catsrc w/ default catsrc name on restart #373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 2 additions & 34 deletions pkg/controller/catalogsource/catalogsource_controller.go
View file
Open in desktop
Expand Up @@ -2,14 +2,13 @@ package catalogsource

import (
"context"
"time"

olm "github.com/operator-framework/operator-marketplace/pkg/apis/olm/v1alpha1"
v1 "github.com/operator-framework/operator-marketplace/pkg/apis/operators/v1"
"github.com/operator-framework/operator-marketplace/pkg/controller/options"
"github.com/operator-framework/operator-marketplace/pkg/defaults"
"github.com/operator-framework/operator-marketplace/pkg/operatorhub"
log "github.com/sirupsen/logrus"
"k8s.io/apimachinery/pkg/api/errors"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/event"
Expand Down Expand Up @@ -89,39 +88,8 @@ type ReconcileCatalogSource struct {
}

func (r *ReconcileCatalogSource) Reconcile(request reconcile.Request) (reconcile.Result, error) {

_, defaultCatalogsources := defaults.GetGlobalDefinitions()
defaultCatsrcDef := defaultCatalogsources[request.Name]

if operatorhub.GetSingleton().Get()[defaultCatsrcDef.Name] {
log.Infof("%s disabled. Not taking any action", defaultCatsrcDef.Name)
return reconcile.Result{}, nil
}
log.Infof("Reconciling default CatalogSource %s", request.Name)

// Fetch the CatalogSource instance
instance := &olm.CatalogSource{}
err := r.client.Get(context.TODO(), request.NamespacedName, instance)
if err != nil {
if errors.IsNotFound(err) {
createNewCatsrcInstance(r.client, defaultCatsrcDef)
return reconcile.Result{}, nil
}
// Error reading the object - requeue the request.
return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 5}, err
}

if instance.DeletionTimestamp != nil {
return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 5}, nil
}

if !defaults.AreCatsrcSpecsEqual(&defaultCatsrcDef.Spec, &instance.Spec) {
if err := r.client.Delete(context.TODO(), instance); err != nil {
log.Warnf("Could not set default CatalogSource %s's spec back to desired default state. Error in deleting updated CatalogSource: %s", defaultCatsrcDef.GetName(), err.Error())
}
return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 5}, nil
}
return reconcile.Result{}, nil
return reconcile.Result{}, defaults.New(map[string]v1.OperatorSource{}, defaultCatalogsources, operatorhub.GetSingleton().Get()).Ensure(r.client, request.Name)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

}

func createNewCatsrcInstance(client client.Client, catsrc olm.CatalogSource) error {
Expand Down
18 changes: 15 additions & 3 deletions pkg/defaults/catsrcHelpers.go
View file
Open in desktop
Expand Up @@ -64,7 +64,9 @@ func processCatsrc(client wrapper.Client, def olm.CatalogSource, disable bool) e
}

if disable {
err = ensureCatsrcAbsent(client, def, cluster)
if cluster.Annotations[defaultCatsrcAnnotationKey] == defaultCatsrcAnnotationValue {
benluddy marked this conversation as resolved.
Show resolved Hide resolved
err = ensureCatsrcAbsent(client, def, cluster)
}
} else {
err = ensureCatsrcPresent(client, def, cluster)
}
Expand Down Expand Up @@ -96,6 +98,12 @@ func ensureCatsrcAbsent(client wrapper.Client, def olm.CatalogSource, cluster *o

// ensureCatsrcPresent ensure that that the default CatalogSource is present on the cluster
func ensureCatsrcPresent(client wrapper.Client, def olm.CatalogSource, cluster *olm.CatalogSource) error {
def = *def.DeepCopy()
if def.Annotations == nil {
def.Annotations = make(map[string]string)
}
def.Annotations[defaultCatsrcAnnotationKey] = defaultCatsrcAnnotationValue

// Create if not present or is deleted
if cluster.Name == "" || (!cluster.ObjectMeta.DeletionTimestamp.IsZero() && len(cluster.Finalizers) == 0) {
err := client.Create(context.TODO(), &def)
Expand All @@ -106,13 +114,17 @@ func ensureCatsrcPresent(client wrapper.Client, def olm.CatalogSource, cluster *
return nil
}

if AreCatsrcSpecsEqual(&def.Spec, &cluster.Spec) {
logrus.Infof("[defaults] CatalogSource %s default and on cluster specs are same", def.Name)
if cluster.Annotations[defaultCatsrcAnnotationKey] == defaultCatsrcAnnotationValue && AreCatsrcSpecsEqual(&def.Spec, &cluster.Spec) {
benluddy marked this conversation as resolved.
Show resolved Hide resolved
logrus.Infof("[defaults] CatalogSource %s is annotated and its spec is the same as the default spec", def.Name)
return nil
}

// Update if the spec has changed
cluster.Spec = def.Spec
if cluster.Annotations == nil {
cluster.Annotations = make(map[string]string)
}
cluster.Annotations[defaultCatsrcAnnotationKey] = defaultCatsrcAnnotationValue
err := client.Update(context.TODO(), cluster)
if err != nil {
return err
Expand Down
7 changes: 6 additions & 1 deletion pkg/defaults/defaults.go
View file
Open in desktop
Expand Up @@ -6,7 +6,7 @@ import (
"os"

olm "github.com/operator-framework/operator-marketplace/pkg/apis/olm/v1alpha1"
"github.com/operator-framework/operator-marketplace/pkg/apis/operators/v1"
v1 "github.com/operator-framework/operator-marketplace/pkg/apis/operators/v1"
wrapper "github.com/operator-framework/operator-marketplace/pkg/client"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -38,6 +38,11 @@ var (
defaultConfig = make(map[string]bool)
)

const (
defaultCatsrcAnnotationKey string = "operatorframework.io/managed-by"
defaultCatsrcAnnotationValue string = "marketplace-operator"
)

// Defaults is the interface that can be used to ensure default OperatorSources
// are always present on the cluster.
type Defaults interface {
Expand Down
25 changes: 0 additions & 25 deletions test/helpers/helpers.go
View file
Open in desktop
Expand Up @@ -391,31 +391,6 @@ func WaitForDeploymentScaled(client test.FrameworkClient, name, namespace string
return nil
}

// RestartMarketplace scales the marketplace deployment down to zero and then scales
// it back up to it's original number of replicas, and waits for a successful deployment.
func RestartMarketplace(client test.FrameworkClient, namespace string) error {
marketplace := &apps.Deployment{}
err := client.Get(context.TODO(), types.NamespacedName{Name: "marketplace-operator", Namespace: namespace}, marketplace)
if err != nil {
return err
}
initialReplicas := marketplace.Spec.Replicas

// Scale down deployment
err = ScaleMarketplace(client, namespace, int32(0))
if err != nil {
return err
}

// Now scale it back up
ScaleMarketplace(client, namespace, *initialReplicas)
if err != nil {
return err
}

return nil
}

// ScaleMarketplace scales the marketplace deployment to the specified replica scale size
func ScaleMarketplace(client test.FrameworkClient, namespace string, scale int32) error {
marketplace := &apps.Deployment{}
Expand Down
3 changes: 3 additions & 0 deletions test/testsuites/defaultcatsrctests.go
View file
Open in desktop
Expand Up @@ -171,6 +171,9 @@ func testDefaultCatsrcWhileDisabled(t *testing.T) {
})
require.NoError(t, err, "The update on the custom CatalogSource was reverted back")

customCatsrc, err = checkForCatsrc("redhat-operators", namespace)
require.NoError(t, err, "Custom CatalogSource redhat-operators was removed from the cluster after marketplace was restarted")

err = toggle(t, 4, false, false) //Re-enable all default CatalogSources
require.NoError(t, err, "Could not enable default CatalogSources")

Expand Down
8 changes: 0 additions & 8 deletions test/testsuites/operatorhubtests.go
View file
Open in desktop
Expand Up @@ -243,10 +243,6 @@ func testClusterStatusDefaultsDisabled(t *testing.T) {
err = checkClusterOperatorHub(t, 4)
assert.NoError(t, err, "Incorrect cluster OperatorHub")

// Restart marketplace operator
err = helpers.RestartMarketplace(test.Global.Client, namespace)
require.NoError(t, err, "Could not restart marketplace operator")

// Check that the ClusterOperator resource has the correct status
clusterOperatorName := "marketplace"
expectedTypeStatus := map[apiconfigv1.ClusterStatusConditionType]apiconfigv1.ConditionStatus{
Expand Down Expand Up @@ -301,10 +297,6 @@ func testSomeClusterStatusDefaultsDisabled(t *testing.T) {
err = checkClusterOperatorHub(t, 2)
assert.NoError(t, err, "Incorrect cluster OperatorHub")

// Restart marketplace operator
err = helpers.RestartMarketplace(test.Global.Client, namespace)
require.NoError(t, err, "Could not restart marketplace operator")

// Check that the ClusterOperator resource has the correct status
clusterOperatorName := "marketplace"
expectedTypeStatus := map[apiconfigv1.ClusterStatusConditionType]apiconfigv1.ConditionStatus{
Expand Down