Skip to content

Commit

Permalink
controllers: surface dependent operator upgradeable conditions
Browse files Browse the repository at this point in the history
- odf-operator brings dependencies using OLM
- this PR ensures odf-operator sets it's own operator condition based on
upgradeability of it's dependents.

Signed-off-by: Leela Venkaiah G <lgangava@ibm.com>
  • Loading branch information
leelavg committed Dec 11, 2023
1 parent 9f22ea4 commit 4f1bed4
Show file tree
Hide file tree
Showing 23 changed files with 1,060 additions and 16 deletions.
10 changes: 9 additions & 1 deletion bundle/manifests/odf-operator.clusterserviceversion.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ metadata:
categories: Storage
console.openshift.io/plugins: '["odf-console"]'
containerImage: quay.io/ocs-dev/odf-operator:latest
createdAt: "2023-11-28T10:45:09Z"
createdAt: "2023-12-08T11:10:09Z"
description: OpenShift Data Foundation provides a common control plane for storage
solutions on OpenShift Container Platform.
features.operators.openshift.io/token-auth-aws: "true"
Expand Down Expand Up @@ -336,6 +336,14 @@ spec:
- patch
- update
- watch
- apiGroups:
- operators.coreos.com
resources:
- operatorconditions
verbs:
- get
- list
- watch
- apiGroups:
- operators.coreos.com
resources:
Expand Down
8 changes: 8 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,14 @@ rules:
- patch
- update
- watch
- apiGroups:
- operators.coreos.com
resources:
- operatorconditions
verbs:
- get
- list
- watch
- apiGroups:
- operators.coreos.com
resources:
Expand Down
144 changes: 141 additions & 3 deletions controllers/storagesystem_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,21 @@ import (
"go.uber.org/multierr"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

operatorv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
operatorv2 "github.com/operator-framework/api/pkg/operators/v2"
"github.com/operator-framework/operator-lib/conditions"
odfv1alpha1 "github.com/red-hat-storage/odf-operator/api/v1alpha1"
"github.com/red-hat-storage/odf-operator/metrics"
"github.com/red-hat-storage/odf-operator/pkg/util"
Expand All @@ -46,9 +51,11 @@ const (
// StorageSystemReconciler reconciles a StorageSystem object
type StorageSystemReconciler struct {
client.Client
Log logr.Logger
Scheme *runtime.Scheme
Recorder *EventReporter
Log logr.Logger
Scheme *runtime.Scheme
Recorder *EventReporter
ConditionName string
OperatorCondition conditions.Condition
}

//+kubebuilder:rbac:groups=odf.openshift.io,resources=storagesystems,verbs=get;list;watch;create;update;patch;delete
Expand All @@ -60,6 +67,7 @@ type StorageSystemReconciler struct {
//+kubebuilder:rbac:groups=operators.coreos.com,resources=clusterserviceversions,verbs=get;list;watch;create;update;patch;delete
//+kubebuilder:rbac:groups=operators.coreos.com,resources=subscriptions,verbs=get;list;watch;create;update;patch;delete
//+kubebuilder:rbac:groups=operators.coreos.com,resources=subscriptions/finalizers,verbs=update
//+kubebuilder:rbac:groups=operators.coreos.com,resources=operatorconditions,verbs=get;list;watch
//+kubebuilder:rbac:groups=console.openshift.io,resources=consolequickstarts,verbs=get;list;create;update;delete
//+kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get;list;watch;create;update

Expand Down Expand Up @@ -154,6 +162,11 @@ func (r *StorageSystemReconciler) reconcile(instance *odfv1alpha1.StorageSystem,
return ctrl.Result{}, err
}

err = r.setOperatorCondition(instance, logger)
if err != nil {
return ctrl.Result{}, err
}

err = r.ensureSubscriptions(instance, logger)
if err != nil {
return ctrl.Result{}, err
Expand Down Expand Up @@ -197,6 +210,58 @@ func (r *StorageSystemReconciler) validateStorageSystemSpec(instance *odfv1alpha
return nil
}

func (r *StorageSystemReconciler) setOperatorCondition(instance *odfv1alpha1.StorageSystem, logger logr.Logger) error {
ocdList := &operatorv2.OperatorConditionList{}
err := r.Client.List(context.TODO(), ocdList, client.InNamespace(instance.Namespace))
if err != nil {
logger.Error(err, "failed to list OperatorConditions")
return err
}

var condNotUpgradeable *metav1.Condition
var ocdName string
for ocdIdx := range ocdList.Items {
// skip reading our own operator condition
if ocdList.Items[ocdIdx].GetName() == r.ConditionName {
continue
}

ocd := &ocdList.Items[ocdIdx]
// overrides is of higher priority than operator set conditions
cond := util.Find(ocd.Spec.Overrides, func(cd *metav1.Condition) bool {
return cd.Type == operatorv2.Upgradeable
})
if cond != nil {
if cond.Status != "True" {
condNotUpgradeable = cond
ocdName = ocd.GetName()
}
break
}

condNotUpgradeable = util.Find(ocd.Spec.Conditions, func(cd *metav1.Condition) bool {
return cd.Type == operatorv2.Upgradeable && cd.Status != "True"
})
if condNotUpgradeable != nil {
ocdName = ocd.GetName()
break
}
}

if condNotUpgradeable != nil {
logger.Info("setting operator upgradeable status", "status", condNotUpgradeable.Status)
// atleast one of the operator is not upgradeable

Check failure on line 253 in controllers/storagesystem_controller.go

View workflow job for this annotation

GitHub Actions / codespell

atleast ==> at least
return r.OperatorCondition.Set(context.TODO(), condNotUpgradeable.Status,
conditions.WithReason(condNotUpgradeable.Reason), conditions.WithMessage(ocdName+":"+condNotUpgradeable.Message))
}

// all operators are upgradeable
status := metav1.ConditionTrue
logger.Info("setting operator upgradeable status", "status", status)
return r.OperatorCondition.Set(context.TODO(), status,
conditions.WithReason("Dependents"), conditions.WithMessage("No dependent reports not upgradeable status"))
}

func (r *StorageSystemReconciler) ensureSubscriptions(instance *odfv1alpha1.StorageSystem, logger logr.Logger) error {

var err error
Expand Down Expand Up @@ -256,9 +321,82 @@ func (r *StorageSystemReconciler) SetupWithManager(mgr ctrl.Manager) error {
},
}

conditionPredicate := predicate.Funcs{
CreateFunc: func(e event.CreateEvent) bool {
return false
},
UpdateFunc: func(e event.UpdateEvent) bool {
// not mandatory but these checks wouldn't harm
if e.ObjectOld == nil {
return false
}
if e.ObjectNew == nil {
return false
}
old, _ := e.ObjectOld.(*operatorv2.OperatorCondition)
new, _ := e.ObjectNew.(*operatorv2.OperatorCondition)
if old == nil || new == nil {
return false
}

// change in admin set conditions for upgradeability
oldOverride := util.Find(old.Spec.Overrides, func(cond *metav1.Condition) bool {
return cond.Type == operatorv2.Upgradeable
})
newOverride := util.Find(new.Spec.Overrides, func(cond *metav1.Condition) bool {
return cond.Type == operatorv2.Upgradeable
})
if oldOverride != nil && newOverride == nil {
// override is removed
return true
}
if newOverride != nil {
if oldOverride == nil {
return true
}
return oldOverride.Status != newOverride.Status
}

// change in operator set conditions for upgradeability
oldCond := util.Find(old.Spec.Conditions, func(cond *metav1.Condition) bool {
return cond.Type == operatorv2.Upgradeable
})
newCond := util.Find(new.Spec.Conditions, func(cond *metav1.Condition) bool {
return cond.Type == operatorv2.Upgradeable
})
if newCond != nil {
if oldCond == nil {
return true
}
return oldCond.Status != newCond.Status
}

return false
},
}
enqueueFromCondition := handler.EnqueueRequestsFromMapFunc(
func(ctx context.Context, obj client.Object) []reconcile.Request {
if _, ok := obj.(*operatorv2.OperatorCondition); !ok {
return []reconcile.Request{}
}
ssList := &odfv1alpha1.StorageSystemList{}
err := r.Client.List(ctx, ssList, client.InNamespace(obj.GetNamespace()), client.Limit(1))
if err != nil {
r.Log.Error(err, "Unable to list StorageSystem objects")
return []reconcile.Request{}
}
if len(ssList.Items) == 0 {
return []reconcile.Request{}
}
ss := ssList.Items[0]
return []reconcile.Request{{NamespacedName: types.NamespacedName{Name: ss.Name, Namespace: ss.Namespace}}}
},
)

return ctrl.NewControllerManagedBy(mgr).
For(&odfv1alpha1.StorageSystem{}, builder.WithPredicates(generationChangedPredicate)).
Owns(&operatorv1alpha1.Subscription{}, builder.WithPredicates(generationChangedPredicate, ignoreCreatePredicate)).
WithOptions(controller.Options{MaxConcurrentReconciles: 1}).
Watches(&operatorv2.OperatorCondition{}, enqueueFromCondition, builder.WithPredicates(conditionPredicate)).
Complete(r)
}
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ require (
github.com/onsi/gomega v1.27.10
github.com/openshift/api v0.0.0-20231010191030-1f9525271dda
github.com/openshift/custom-resource-status v1.1.2
github.com/operator-framework/api v0.17.7-0.20230626210316-aa3e49803e7b
github.com/operator-framework/api v0.20.0
github.com/prometheus/client_golang v1.17.0
github.com/red-hat-storage/ocs-operator/v4 v4.0.0-20231129111953-fda031ed2e1e
github.com/stretchr/testify v1.8.4
Expand Down Expand Up @@ -76,6 +76,7 @@ require (
github.com/modern-go/reflect2 v1.0.2 // indirect
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
github.com/noobaa/noobaa-operator/v5 v5.0.0-20230919064207-0b6979c00d6a // indirect
github.com/operator-framework/operator-lib v0.11.1-0.20230717184314-6efbe3a22f6f
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
github.com/prometheus/client_model v0.5.0 // indirect
Expand Down
6 changes: 4 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -650,8 +650,10 @@ github.com/openshift/build-machinery-go v0.0.0-20200917070002-f171684f77ab/go.mo
github.com/openshift/client-go v0.0.0-20210112165513-ebc401615f47/go.mod h1:u7NRAjtYVAKokiI9LouzTv4mhds8P4S1TwdVAfbjKSk=
github.com/openshift/custom-resource-status v1.1.2 h1:C3DL44LEbvlbItfd8mT5jWrqPfHnSOQoQf/sypqA6A4=
github.com/openshift/custom-resource-status v1.1.2/go.mod h1:DB/Mf2oTeiAmVVX1gN+NEqweonAPY0TKUwADizj8+ZA=
github.com/operator-framework/api v0.17.7-0.20230626210316-aa3e49803e7b h1:prJEMyFQde4yxxaTuvqx1A/ukuCg/EZ2MbfdZiJwlls=
github.com/operator-framework/api v0.17.7-0.20230626210316-aa3e49803e7b/go.mod h1:lnurXgadLnoZ7pufKMHkErr2BVOIZSpHtvEkHBcKvdk=
github.com/operator-framework/api v0.20.0 h1:A2YCRhr+6s0k3pRJacnwjh1Ue8BqjIGuQ2jvPg9XCB4=
github.com/operator-framework/api v0.20.0/go.mod h1:rXPOhrQ6mMeXqCmpDgt1ALoar9ZlHL+Iy5qut9R99a4=
github.com/operator-framework/operator-lib v0.11.1-0.20230717184314-6efbe3a22f6f h1:CQkdjRsbPtDd3YvENaPMZzw1eHPfujiZTrCzzSCPCsw=
github.com/operator-framework/operator-lib v0.11.1-0.20230717184314-6efbe3a22f6f/go.mod h1:fmVTfDgR/OMPg7eJvXWlyBVzCXUfHAOxIXO8W51HvKY=
github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
github.com/pborman/uuid v0.0.0-20170612153648-e790cca94e6c/go.mod h1:VyrYX9gd7irzKovcSS6BIIEwPRkP2Wm2m9ufcdFSJ34=
github.com/pborman/uuid v1.2.0 h1:J7Q5mO4ysT1dv8hyrUGHb9+ooztCXu1D8MY8DZYsu3g=
Expand Down
49 changes: 45 additions & 4 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,29 @@ limitations under the License.
package main

import (
"context"
"flag"
"os"

// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
// to ensure that exec-entrypoint and run can make use of them.
_ "k8s.io/client-go/plugin/pkg/client/auth"
"k8s.io/client-go/util/retry"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/wait"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/cache"
apiclient "sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/healthz"
"sigs.k8s.io/controller-runtime/pkg/log/zap"

operatorv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
operatorv2 "github.com/operator-framework/api/pkg/operators/v2"
"github.com/operator-framework/operator-lib/conditions"

ibmv1alpha1 "github.com/IBM/ibm-storage-odf-operator/api/v1alpha1"
ocsv1 "github.com/red-hat-storage/ocs-operator/v4/api/v1"
Expand Down Expand Up @@ -62,6 +69,7 @@ func init() {
utilruntime.Must(ibmv1alpha1.AddToScheme(scheme))

utilruntime.Must(operatorv1alpha1.AddToScheme(scheme))
utilruntime.Must(operatorv2.AddToScheme(scheme))
//+kubebuilder:scaffold:scheme

utilruntime.Must(consolev1.AddToScheme(scheme))
Expand Down Expand Up @@ -115,16 +123,49 @@ func main() {
os.Exit(1)
}

factory := conditions.InClusterFactory{Client: mgr.GetClient()}
namespacedName, err := factory.GetNamespacedName()
if err != nil {
setupLog.Error(err, "unable to get condition name")
os.Exit(1)
}
condition, err := factory.NewCondition(operatorv2.ConditionType(operatorv2.Upgradeable))
if err != nil {
setupLog.Error(err, "unable to get OperatorCondition")
os.Exit(1)
}
storageSystemReconciler := &controllers.StorageSystemReconciler{
Client: mgr.GetClient(),
Log: ctrl.Log.WithName("controllers").WithName("StorageSystem"),
Scheme: mgr.GetScheme(),
Recorder: controllers.NewEventReporter(mgr.GetEventRecorderFor("StorageSystem controller")),
Client: mgr.GetClient(),
Log: ctrl.Log.WithName("controllers").WithName("StorageSystem"),
Scheme: mgr.GetScheme(),
Recorder: controllers.NewEventReporter(mgr.GetEventRecorderFor("StorageSystem controller")),
ConditionName: namespacedName.Name,
OperatorCondition: condition,
}
if err = storageSystemReconciler.SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "StorageSystem")
os.Exit(1)
}
apiClient, err := apiclient.New(mgr.GetConfig(), apiclient.Options{
Scheme: mgr.GetScheme(),
})
if err != nil {
setupLog.Error(err, "Unable to get Client")
os.Exit(1)
}
condition, err = conditions.InClusterFactory{Client: apiClient}.NewCondition(operatorv2.ConditionType(operatorv2.Upgradeable))
if err != nil {
setupLog.Error(err, "Unable to get OperatorCondition")
os.Exit(1)
}
err = wait.ExponentialBackoff(retry.DefaultRetry, func() (bool, error) {
err = condition.Set(context.TODO(), metav1.ConditionTrue, conditions.WithMessage("Operator is ready"), conditions.WithReason("Ready"))
return err == nil, err
})
if err != nil {
setupLog.Error(err, "Unable to update OperatorCondition")
os.Exit(1)
}

subscriptionReconciler := &controllers.SubscriptionReconciler{
Client: mgr.GetClient(),
Expand Down
11 changes: 11 additions & 0 deletions pkg/util/strings.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,14 @@ func RemoveFromSlice(slice []string, s string) (result []string) {
}
return
}

// Find returns the first entry matching the function "f" or else return nil
func Find[T any](list []T, f func(item *T) bool) *T {
for idx := range list {
ele := &list[idx]
if f(ele) {
return ele
}
}
return nil
}
Loading

0 comments on commit 4f1bed4

Please sign in to comment.