From ff7087bbb7bdbc23288194300b62c3e54b337a2c Mon Sep 17 00:00:00 2001 From: Daniel Fan Date: Fri, 5 Apr 2024 12:38:19 -0700 Subject: [PATCH 1/3] Update OperatorConfig reconciliation and improve error handling Signed-off-by: Daniel Fan --- controllers/operator/manager.go | 5 ++++ .../operatorconfig_controller.go | 30 ++++++++++++++----- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/controllers/operator/manager.go b/controllers/operator/manager.go index b6bd3500..f12fe7bb 100644 --- a/controllers/operator/manager.go +++ b/controllers/operator/manager.go @@ -392,6 +392,11 @@ func (m *ODLMOperator) GetSubscription(ctx context.Context, name, operatorNs, se // GetClusterServiceVersion gets the ClusterServiceVersion from the subscription func (m *ODLMOperator) GetClusterServiceVersion(ctx context.Context, sub *olmv1alpha1.Subscription) (*olmv1alpha1.ClusterServiceVersion, error) { + // Check if subscription is nil + if sub == nil { + klog.Error("The subscription is nil") + return nil, fmt.Errorf("the subscription is nil") + } // Check the ClusterServiceVersion status in the subscription if sub.Status.InstalledCSV == "" { klog.Warningf("The ClusterServiceVersion for Subscription %s is not ready. Will check it again", sub.Name) diff --git a/controllers/operatorconfig/operatorconfig_controller.go b/controllers/operatorconfig/operatorconfig_controller.go index 6a39383b..9fe59d79 100644 --- a/controllers/operatorconfig/operatorconfig_controller.go +++ b/controllers/operatorconfig/operatorconfig_controller.go @@ -34,6 +34,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" operatorv1alpha1 "github.com/IBM/operand-deployment-lifecycle-manager/api/v1alpha1" + "github.com/IBM/operand-deployment-lifecycle-manager/controllers/constant" deploy "github.com/IBM/operand-deployment-lifecycle-manager/controllers/operator" ) @@ -73,19 +74,25 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu operand := u operator := registry.GetOperator(operand.Name) if operator.OperatorConfig == "" { - break + continue } var sub *olmv1alpha1.Subscription sub, err = r.GetSubscription(ctx, operator.Name, operator.Namespace, registry.Namespace, operator.PackageName) if err != nil { return ctrl.Result{}, err + } else if sub == nil { + klog.Infof("Subscription for Operator %s/%s not found", operator.Name, operator.PackageName) + return ctrl.Result{RequeueAfter: constant.DefaultRequeueDuration}, nil } var csv *olmv1alpha1.ClusterServiceVersion csv, err = r.GetClusterServiceVersion(ctx, sub) if err != nil { return ctrl.Result{}, err + } else if csv == nil { + klog.Infof("ClusterServiceVersion for Operator %s/%s not found", operator.Name, operator.PackageName) + return ctrl.Result{RequeueAfter: constant.DefaultRequeueDuration}, nil } klog.Infof("Fetching OperatorConfig: %s", operator.OperatorConfig) @@ -94,12 +101,17 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu Name: operator.OperatorConfig, Namespace: registry.Namespace, }, config); err != nil { - return ctrl.Result{}, client.IgnoreNotFound(err) + if client.IgnoreNotFound(err) != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) + } else { + klog.Infof("OperatorConfig %s/%s does not exist for oeprand %s in request %s, %s", registry.Namespace, operator.OperatorConfig, operator.Name, instance.Namespace, instance.Name) + continue + } } serviceConfig := config.GetConfigForOperator(operator.Name) if serviceConfig == nil { klog.Infof("OperatorConfig: %s, does not have configuration for operator: %s", operator.OperatorConfig, operator.Name) - return ctrl.Result{}, nil + continue } copyToCast, err := deepcopy.Anything(csv) @@ -108,13 +120,17 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } csvToUpdate := copyToCast.(*olmv1alpha1.ClusterServiceVersion) klog.Infof("Applying OperatorConfig: %s to Operator: %s via CSV: %s, %s", operator.OperatorConfig, operator.Name, csv.Name, csv.Namespace) - return r.configCsv(ctx, csvToUpdate, serviceConfig) + if err := r.configCsv(ctx, csvToUpdate, serviceConfig); err != nil { + klog.Errorf("Failed to apply OperatorConfig %s/%s to Operator: %s via CSV: %s, %s", registry.Namespace, operator.OperatorConfig, operator.Name, csv.Namespace, csv.Name) + return ctrl.Result{}, err + } } } + klog.Infof("Finished reconciling OperatorConfig for request: %s, %s", instance.Namespace, instance.Name) return ctrl.Result{}, nil } -func (r *Reconciler) configCsv(ctx context.Context, csv *olmv1alpha1.ClusterServiceVersion, config *operatorv1alpha1.ServiceOperatorConfig) (ctrl.Result, error) { +func (r *Reconciler) configCsv(ctx context.Context, csv *olmv1alpha1.ClusterServiceVersion, config *operatorv1alpha1.ServiceOperatorConfig) error { if config.Replicas != nil { csv.Spec.InstallStrategy.StrategySpec.DeploymentSpecs[0].Spec.Replicas = config.Replicas } @@ -125,9 +141,9 @@ func (r *Reconciler) configCsv(ctx context.Context, csv *olmv1alpha1.ClusterServ csv.Spec.InstallStrategy.StrategySpec.DeploymentSpecs[0].Spec.Template.Spec.TopologySpreadConstraints = config.TopologySpreadConstraints } if err := r.Client.Update(ctx, csv); err != nil { - return ctrl.Result{}, err + return err } - return ctrl.Result{}, nil + return nil } func (r *Reconciler) requestsFromMapFunc(ctx context.Context) handler.MapFunc { From 8fe3613ee2cd37f859882e381e16fd1a907e102b Mon Sep 17 00:00:00 2001 From: Daniel Fan Date: Fri, 5 Apr 2024 12:43:56 -0700 Subject: [PATCH 2/3] Requeue request every three hours for sync Signed-off-by: Daniel Fan --- controllers/operatorconfig/operatorconfig_controller.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/controllers/operatorconfig/operatorconfig_controller.go b/controllers/operatorconfig/operatorconfig_controller.go index 9fe59d79..cacc5157 100644 --- a/controllers/operatorconfig/operatorconfig_controller.go +++ b/controllers/operatorconfig/operatorconfig_controller.go @@ -102,9 +102,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu Namespace: registry.Namespace, }, config); err != nil { if client.IgnoreNotFound(err) != nil { - return ctrl.Result{}, client.IgnoreNotFound(err) + return ctrl.Result{}, err } else { - klog.Infof("OperatorConfig %s/%s does not exist for oeprand %s in request %s, %s", registry.Namespace, operator.OperatorConfig, operator.Name, instance.Namespace, instance.Name) + klog.Infof("OperatorConfig %s/%s does not exist for operand %s in request %s, %s", registry.Namespace, operator.OperatorConfig, operator.Name, instance.Namespace, instance.Name) continue } } @@ -127,7 +127,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } } klog.Infof("Finished reconciling OperatorConfig for request: %s, %s", instance.Namespace, instance.Name) - return ctrl.Result{}, nil + return ctrl.Result{RequeueAfter: constant.DefaultSyncPeriod}, nil } func (r *Reconciler) configCsv(ctx context.Context, csv *olmv1alpha1.ClusterServiceVersion, config *operatorv1alpha1.ServiceOperatorConfig) error { From 44fb73215498ed6c3379386c12beb65c5cd5fb7e Mon Sep 17 00:00:00 2001 From: Daniel Fan Date: Fri, 5 Apr 2024 13:02:33 -0700 Subject: [PATCH 3/3] drop unnecessary else statement Signed-off-by: Daniel Fan --- controllers/operatorconfig/operatorconfig_controller.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/controllers/operatorconfig/operatorconfig_controller.go b/controllers/operatorconfig/operatorconfig_controller.go index cacc5157..be180b1b 100644 --- a/controllers/operatorconfig/operatorconfig_controller.go +++ b/controllers/operatorconfig/operatorconfig_controller.go @@ -103,10 +103,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu }, config); err != nil { if client.IgnoreNotFound(err) != nil { return ctrl.Result{}, err - } else { - klog.Infof("OperatorConfig %s/%s does not exist for operand %s in request %s, %s", registry.Namespace, operator.OperatorConfig, operator.Name, instance.Namespace, instance.Name) - continue } + klog.Infof("OperatorConfig %s/%s does not exist for operand %s in request %s, %s", registry.Namespace, operator.OperatorConfig, operator.Name, instance.Namespace, instance.Name) + continue } serviceConfig := config.GetConfigForOperator(operator.Name) if serviceConfig == nil {