Skip to content

refactor no olm controller code to handle cleanup on operandrequest deletion #1108

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

Merged
bitscuit merged 11 commits into from
Feb 4, 2025
Merged

refactor no olm controller code to handle cleanup on operandrequest deletion #1108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4b541f4
refactor no olm controller code to handle deletion
bluzarraga Jan 31, 2025
d2a6185
cleanup
bluzarraga Jan 31, 2025
0e2967b
update build params for testing
bluzarraga Jan 31, 2025
3e360d8
update controller to watch configmap changes
bluzarraga Feb 3, 2025
d680360
add watched by odlm label
bluzarraga Feb 3, 2025
2ffafeb
add debugging statements
bluzarraga Feb 3, 2025
3c3b79f
more debugging logs
bluzarraga Feb 3, 2025
b914f6e
more debug statements
bluzarraga Feb 4, 2025
5a04795
remove user managed condition
bluzarraga Feb 4, 2025
abaabf1
debug statements
bluzarraga Feb 4, 2025
920800f
disable, edit debugging statements
bluzarraga Feb 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ OPERATOR_VERSION ?= 4.3.9
# Kind cluster name
KIND_CLUSTER_NAME ?= "odlm"
# Operator image tag for test
OPERATOR_TEST_TAG ?= nolm-controller3
OPERATOR_TEST_TAG ?= nolm-controller-cleanup

# Options for 'bundle-build'
ifneq ($(origin CHANNELS), undefined)
Expand Down
2 changes: 2 additions & 0 deletions api/v1alpha1/operandrequest_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ const (
ResourceTypeCsv ResourceType = "csv"
ResourceTypeOperator ResourceType = "operator"
ResourceTypeOperand ResourceType = "operands"
ResourceTypeConfigmap ResourceType = "configmap"
ResourceTypeDeployment ResourceType = "deployment"
)

// Condition represents the current state of the Request Service.
Expand Down
60 changes: 24 additions & 36 deletions controllers/operandrequestnoolm/operandrequestnoolm_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ import (
"time"

gset "github.com/deckarep/golang-set"
olmv1 "github.com/operator-framework/api/pkg/operators/v1"
olmv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
"github.com/pkg/errors"
authorizationv1 "k8s.io/api/authorization/v1"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -56,9 +54,8 @@ type Reconciler struct {
Mutex sync.Mutex
}
type clusterObjects struct {
namespace *corev1.Namespace
operatorGroup *olmv1.OperatorGroup
subscription *olmv1alpha1.Subscription
namespace *corev1.Namespace
configmap *corev1.ConfigMap
}

//+kubebuilder:rbac:groups=operator.ibm.com,resources=certmanagers;auditloggings,verbs=get;delete
Expand Down Expand Up @@ -114,13 +111,13 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re

// Remove finalizer when DeletionTimestamp none zero
if !requestInstance.ObjectMeta.DeletionTimestamp.IsZero() {
//TODO determine if necessary
// Check and clean up the subscriptions
// err := r.checkFinalizer(ctx, requestInstance)
// if err != nil {
// klog.Errorf("failed to clean up the subscriptions for OperandRequest %s: %v", req.NamespacedName.String(), err)
// return ctrl.Result{}, err
// }
//Checkfinalizers calls the delete function, not necessarily subscription based
//Check and clean up the operands
err := r.checkFinalizer(ctx, requestInstance)
if err != nil {
klog.Errorf("failed to clean up the operands for OperandRequest %s: %v", req.NamespacedName.String(), err)
return ctrl.Result{}, err
}

originalReq := requestInstance.DeepCopy()
// Update finalizer to allow delete CR
Expand Down Expand Up @@ -163,19 +160,21 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
return ctrl.Result{Requeue: true}, err
}

// TODO remove this section
//Many of the cleanup functions are run through reconcileoperator as of 1.31.25
//technically nothing is done to the operator deployment as of this writing (1.31.25)
// Reconcile Operators
// if err := r.reconcileOperator(ctx, requestInstance); err != nil {
// klog.Errorf("failed to reconcile Operators for OperandRequest %s: %v", req.NamespacedName.String(), err)
// return ctrl.Result{}, err
// }
if err := r.reconcileOperator(ctx, requestInstance); err != nil {
klog.Errorf("failed to reconcile Operators for OperandRequest %s: %v", req.NamespacedName.String(), err)
return ctrl.Result{}, err
}

// Reconcile Operands
if merr := r.reconcileOperand(ctx, requestInstance); len(merr.Errors) != 0 {
klog.Errorf("failed to reconcile Operands for OperandRequest %s: %v", req.NamespacedName.String(), merr)
return ctrl.Result{}, merr
}

//TODO update this block to check deployments and their operands instead
// Check if all csv deploy succeed
if requestInstance.Status.Phase != operatorv1alpha1.ClusterPhaseRunning {
klog.V(2).Info("Waiting for all operators and operands to be deployed successfully ...")
Expand Down Expand Up @@ -244,22 +243,10 @@ func (r *Reconciler) checkFinalizer(ctx context.Context, requestInstance *operat
klog.V(1).Infof("Deleting OperandRequest %s in the namespace %s", requestInstance.Name, requestInstance.Namespace)
remainingOperands := gset.NewSet()
for _, m := range requestInstance.Status.Members {
klog.V(3).Infof("Operand %s added to deletion list", m.Name)
remainingOperands.Add(m.Name)
}
// TODO: update to check OperandRequest status to see if member is user managed or not
// existingSub := &olmv1alpha1.SubscriptionList{}

// opts := []client.ListOption{
// client.MatchingLabels(map[string]string{constant.OpreqLabel: "true"}),
// }

// if err := r.Client.List(ctx, existingSub, opts...); err != nil {
// return err
// }
// if len(existingSub.Items) == 0 {
// return nil
// }
// Delete all the subscriptions that created by current request
// Delete all the operands and configmaps that created by current request
if err := r.absentOperatorsAndOperands(ctx, requestInstance, &remainingOperands); err != nil {
return err
}
Expand Down Expand Up @@ -395,14 +382,15 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
WithOptions(options).
For(&operatorv1alpha1.OperandRequest{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})).
Watches(&source.Kind{Type: &olmv1alpha1.Subscription{}}, handler.EnqueueRequestsFromMapFunc(r.getSubToRequestMapper()), builder.WithPredicates(predicate.Funcs{
Watches(&source.Kind{Type: &corev1.ConfigMap{}}, handler.EnqueueRequestsFromMapFunc(r.getReferenceToRequestMapper()), builder.WithPredicates(predicate.Funcs{
UpdateFunc: func(e event.UpdateEvent) bool {
oldObject := e.ObjectOld.(*olmv1alpha1.Subscription)
newObject := e.ObjectNew.(*olmv1alpha1.Subscription)
oldObject := e.ObjectOld.(*corev1.ConfigMap)
newObject := e.ObjectNew.(*corev1.ConfigMap)
if oldObject.Labels != nil && oldObject.Labels[constant.OpreqLabel] == "true" {
statusToggle := (oldObject.Status.InstalledCSV != "" && newObject.Status.InstalledCSV != "" && oldObject.Status.InstalledCSV != newObject.Status.InstalledCSV)
// statusToggle := (oldObject.Status.InstalledCSV != "" && newObject.Status.InstalledCSV != "" && oldObject.Status.InstalledCSV != newObject.Status.InstalledCSV)
metadataToggle := !reflect.DeepEqual(oldObject.Annotations, newObject.Annotations)
return statusToggle || metadataToggle
// return statusToggle || metadataToggle
return metadataToggle
}
return false
},
Expand Down
65 changes: 0 additions & 65 deletions controllers/operandrequestnoolm/reconcile_operand.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,55 +97,8 @@ func (r *Reconciler) reconcileOperand(ctx context.Context, requestInstance *oper
// Looking for the CSV
namespace := r.GetOperatorNamespace(opdRegistry.InstallMode, opdRegistry.Namespace)

//TODO remove this section
// sub, err := r.GetSubscription(ctx, operatorName, namespace, registryInstance.Namespace, opdRegistry.PackageName)
// if err != nil {
// merr.Add(errors.Wrapf(err, "failed to get the Subscription %s in the namespace %s and %s", operatorName, namespace, registryInstance.Namespace))
// return merr
// }

// if !opdRegistry.UserManaged {
// if sub == nil {
// klog.Warningf("There is no Subscription %s or %s in the namespace %s and %s", operatorName, opdRegistry.PackageName, namespace, registryInstance.Namespace)
// continue
// }

// if _, ok := sub.Labels[constant.OpreqLabel]; !ok {
// // Subscription existing and not managed by OperandRequest controller
// klog.Warningf("Subscription %s in the namespace %s isn't created by ODLM", sub.Name, sub.Namespace)
// }

// // It the installplan is not created yet, ODLM will try later
// if sub.Status.Install == nil || sub.Status.InstallPlanRef.Name == "" {
// klog.Warningf("The Installplan for Subscription %s is not ready. Will check it again", sub.Name)
// requestInstance.SetMemberStatus(operand.Name, operatorv1alpha1.OperatorInstalling, "", &r.Mutex)
// continue
// }

// // If the installplan is deleted after is completed, ODLM won't block the CR update.
// ipName := sub.Status.InstallPlanRef.Name
// ipNamespace := sub.Namespace
// ip := &olmv1alpha1.InstallPlan{}
// ipKey := types.NamespacedName{
// Name: ipName,
// Namespace: ipNamespace,
// }
// if err := r.Client.Get(ctx, ipKey, ip); err != nil {
// if !apierrors.IsNotFound(err) {
// merr.Add(errors.Wrapf(err, "failed to get Installplan"))
// }
// } else if ip.Status.Phase == olmv1alpha1.InstallPlanPhaseFailed {
// klog.Errorf("installplan %s/%s is failed", ipNamespace, ipName)
// requestInstance.SetMemberStatus(operand.Name, operatorv1alpha1.OperatorFailed, "", &r.Mutex)
// continue
// }

// }

//var csv *olmv1alpha1.ClusterServiceVersion
var deployment *appsv1.Deployment

//TODO need to translate this if block to look for deployments and not CSVs
deploymentList, err := r.GetDeploymentListFromPackage(ctx, opdRegistry.PackageName, opdRegistry.Namespace)
if err != nil {
merr.Add(err)
Expand All @@ -154,7 +107,6 @@ func (r *Reconciler) reconcileOperand(ctx context.Context, requestInstance *oper
}
deployment = deploymentList[0]

//TODO change this block to deal with an empty list of deployments instead of csvs
if deployment == nil {
klog.Warningf("Deployment for %s in the namespace %s is not ready yet, retry", operatorName, namespace)
requestInstance.SetMemberStatus(operand.Name, operatorv1alpha1.OperatorInstalling, "", &r.Mutex)
Expand All @@ -169,19 +121,6 @@ func (r *Reconciler) reconcileOperand(ctx context.Context, requestInstance *oper
// continue
// }

//TODO git rid of this section
// if !opdRegistry.UserManaged {
// // find the OperandRequest which has the same operator's channel or fallback channels as existing subscription.
// // ODLM will only reconcile Operand based on OperandConfig for this OperandRequest
// channels := []string{opdRegistry.Channel}
// if channels = append(channels, opdRegistry.FallbackChannels...); !util.Contains(channels, sub.Spec.Channel) {
// klog.Infof("Subscription %s in the namespace %s is NOT managed by %s/%s, Skip reconciling Operands", sub.Name, sub.Namespace, requestInstance.Namespace, requestInstance.Name)
// requestInstance.SetMemberStatus(operand.Name, operatorv1alpha1.OperatorFailed, "", &r.Mutex)
// continue
// }
// }

//TODO update to deployment name
klog.V(3).Info("Generating customresource base on Deployment: ", deployment.GetName())
requestInstance.SetMemberStatus(operand.Name, operatorv1alpha1.OperatorRunning, "", &r.Mutex)

Expand All @@ -195,7 +134,6 @@ func (r *Reconciler) reconcileOperand(ctx context.Context, requestInstance *oper
klog.V(2).Infof("There is no service: %s from the OperandConfig instance: %s/%s, Skip reconciling Operands", operand.Name, registryKey.Namespace, req.Registry)
continue
}
//TODO pass through deployment values here
err = r.reconcileCRwithConfig(ctx, opdConfig, configInstance.Name, configInstance.Namespace, deployment, requestInstance, operand.Name, deployment.Namespace, &r.Mutex)
if err != nil {
merr.Add(err)
Expand Down Expand Up @@ -263,7 +201,6 @@ func (r *Reconciler) reconcileCRwithConfig(ctx context.Context, service *operato
}
}

//TODO change this to deployment, make sure GetAnnotations works for deployments the same way
almExamples := deployment.GetAnnotations()["alm-examples"]

// Convert CR template string to slice
Expand Down Expand Up @@ -320,7 +257,6 @@ func (r *Reconciler) reconcileCRwithConfig(ctx context.Context, service *operato
}

if !foundInConfig {
//TODO change to deployment
klog.Warningf("%v in the alm-example doesn't exist in the OperandConfig for %v", crFromALM.GetKind(), deployment.GetName())
continue
}
Expand Down Expand Up @@ -365,7 +301,6 @@ func (r *Reconciler) reconcileCRwithConfig(ctx context.Context, service *operato

for cr, found := range foundMap {
if !found {
//TODO change to deployment
klog.Warningf("Custom resource %v doesn't exist in the alm-example of %v", cr, deployment.GetName())
}
}
Expand Down
Loading