From c2de1ace238798843abd056e2d18904b5822be2c Mon Sep 17 00:00:00 2001 From: Mike Rostermund Date: Thu, 8 Aug 2024 14:45:24 +0200 Subject: [PATCH] Fix requeue and finalizers (#838) * Ensure call to setState doesn't impact what error is returned. This commit also fixes a bug that could cause the operator to not retry reconciling an object if e.g. HumioExternalCluster or the API token secret is gone to construct Humio client config when calling NewCluster(). In those cases, even if the HumioExternalCluster or the API token secret is back, the reconcile is never retried. * Fix example for HumioScheduledSearch * Refer directly to the humio API package for the query ownership type * Treat Delete on entities as successful if entities cannot be found. This will unblock cases where e.g. finalizers are blocking object deletion just because the operator keeps trying to delete something that is returned as EntityNotFound. There's technically more CRD's we could apply this to, but we'd first need to extend this EntityNotFound part to those resource types in the API package. --- controllers/humioaction_controller.go | 12 ++++++------ controllers/humioalert_controller.go | 12 ++++++------ .../humioexternalcluster_controller.go | 2 +- controllers/humiofilteralert_controller.go | 12 ++++++------ controllers/humioingesttoken_controller.go | 9 ++++----- controllers/humioparser_controller.go | 9 ++++----- controllers/humiorepository_controller.go | 9 ++++----- .../humioscheduledsearch_controller.go | 12 ++++++------ controllers/humioview_controller.go | 9 ++++----- examples/humioscheduledsearch.yaml | 4 ++-- pkg/humio/client.go | 19 +++++++++++++++++++ pkg/humio/filteralert_transform.go | 5 ++--- pkg/humio/scheduledsearch_transform.go | 5 ++--- 13 files changed, 66 insertions(+), 53 deletions(-) diff --git a/controllers/humioaction_controller.go b/controllers/humioaction_controller.go index 6037e372..49f38d73 100644 --- a/controllers/humioaction_controller.go +++ b/controllers/humioaction_controller.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "time" "github.com/go-logr/logr" "github.com/google/go-cmp/cmp" @@ -74,12 +75,11 @@ func (r *HumioActionReconciler) Reconcile(ctx context.Context, req ctrl.Request) cluster, err := helpers.NewCluster(ctx, r, ha.Spec.ManagedClusterName, ha.Spec.ExternalClusterName, ha.Namespace, helpers.UseCertManager(), true) if err != nil || cluster == nil || cluster.Config() == nil { - r.Log.Error(err, "unable to obtain humio client config") - err = r.setState(ctx, humiov1alpha1.HumioActionStateConfigError, ha) - if err != nil { - return reconcile.Result{}, r.logErrorAndReturn(err, "unable to set action state") + setStateErr := r.setState(ctx, humiov1alpha1.HumioActionStateConfigError, ha) + if setStateErr != nil { + return reconcile.Result{}, r.logErrorAndReturn(setStateErr, "unable to set action state") } - return reconcile.Result{}, err + return reconcile.Result{RequeueAfter: 5 * time.Second}, r.logErrorAndReturn(err, "unable to obtain humio client config") } err = r.resolveSecrets(ctx, ha) @@ -192,7 +192,7 @@ func (r *HumioActionReconciler) reconcileHumioAction(ctx context.Context, config } r.Log.Info("done reconciling, will requeue after 15 seconds") - return reconcile.Result{}, nil + return reconcile.Result{RequeueAfter: time.Second * 15}, nil } func (r *HumioActionReconciler) resolveSecrets(ctx context.Context, ha *humiov1alpha1.HumioAction) error { diff --git a/controllers/humioalert_controller.go b/controllers/humioalert_controller.go index 1f6f4a11..8fe8eaf6 100644 --- a/controllers/humioalert_controller.go +++ b/controllers/humioalert_controller.go @@ -22,6 +22,7 @@ import ( "fmt" "github.com/humio/humio-operator/pkg/kubernetes" "reflect" + "time" humioapi "github.com/humio/cli/api" @@ -77,12 +78,11 @@ func (r *HumioAlertReconciler) Reconcile(ctx context.Context, req ctrl.Request) cluster, err := helpers.NewCluster(ctx, r, ha.Spec.ManagedClusterName, ha.Spec.ExternalClusterName, ha.Namespace, helpers.UseCertManager(), true) if err != nil || cluster == nil || cluster.Config() == nil { - r.Log.Error(err, "unable to obtain humio client config") - err = r.setState(ctx, humiov1alpha1.HumioAlertStateConfigError, ha) - if err != nil { - return reconcile.Result{}, r.logErrorAndReturn(err, "unable to set alert state") + setStateErr := r.setState(ctx, humiov1alpha1.HumioAlertStateConfigError, ha) + if setStateErr != nil { + return reconcile.Result{}, r.logErrorAndReturn(setStateErr, "unable to set alert state") } - return reconcile.Result{}, err + return reconcile.Result{RequeueAfter: 5 * time.Second}, r.logErrorAndReturn(err, "unable to obtain humio client config") } defer func(ctx context.Context, humioClient humio.Client, ha *humiov1alpha1.HumioAlert) { @@ -187,7 +187,7 @@ func (r *HumioAlertReconciler) reconcileHumioAlert(ctx context.Context, config * } r.Log.Info("done reconciling, will requeue after 15 seconds") - return reconcile.Result{}, nil + return reconcile.Result{RequeueAfter: time.Second * 15}, nil } // SetupWithManager sets up the controller with the Manager. diff --git a/controllers/humioexternalcluster_controller.go b/controllers/humioexternalcluster_controller.go index 7ac083f5..6b56b179 100644 --- a/controllers/humioexternalcluster_controller.go +++ b/controllers/humioexternalcluster_controller.go @@ -81,7 +81,7 @@ func (r *HumioExternalClusterReconciler) Reconcile(ctx context.Context, req ctrl cluster, err := helpers.NewCluster(ctx, r, "", hec.Name, hec.Namespace, helpers.UseCertManager(), true) if err != nil || cluster.Config() == nil { - return reconcile.Result{}, r.logErrorAndReturn(err, "unable to obtain humio client config") + return reconcile.Result{}, r.logErrorAndReturn(fmt.Errorf("unable to obtain humio client config: %w", err), "unable to obtain humio client config") } err = r.HumioClient.TestAPIToken(cluster.Config(), req) diff --git a/controllers/humiofilteralert_controller.go b/controllers/humiofilteralert_controller.go index 946b0029..422dc8b6 100644 --- a/controllers/humiofilteralert_controller.go +++ b/controllers/humiofilteralert_controller.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "reflect" + "time" "github.com/humio/humio-operator/pkg/kubernetes" @@ -78,12 +79,11 @@ func (r *HumioFilterAlertReconciler) Reconcile(ctx context.Context, req ctrl.Req cluster, err := helpers.NewCluster(ctx, r, hfa.Spec.ManagedClusterName, hfa.Spec.ExternalClusterName, hfa.Namespace, helpers.UseCertManager(), true) if err != nil || cluster == nil || cluster.Config() == nil { - r.Log.Error(err, "unable to obtain humio client config") - err = r.setState(ctx, humiov1alpha1.HumioFilterAlertStateConfigError, hfa) - if err != nil { - return reconcile.Result{}, r.logErrorAndReturn(err, "unable to set filter alert state") + setStateErr := r.setState(ctx, humiov1alpha1.HumioFilterAlertStateConfigError, hfa) + if setStateErr != nil { + return reconcile.Result{}, r.logErrorAndReturn(setStateErr, "unable to set filter alert state") } - return reconcile.Result{}, err + return reconcile.Result{RequeueAfter: 5 * time.Second}, r.logErrorAndReturn(err, "unable to obtain humio client config") } defer func(ctx context.Context, humioClient humio.Client, hfa *humiov1alpha1.HumioFilterAlert) { @@ -193,7 +193,7 @@ func (r *HumioFilterAlertReconciler) reconcileHumioFilterAlert(ctx context.Conte } r.Log.Info("done reconciling, will requeue after 15 seconds") - return reconcile.Result{}, nil + return reconcile.Result{RequeueAfter: time.Second * 15}, nil } // SetupWithManager sets up the controller with the Manager. diff --git a/controllers/humioingesttoken_controller.go b/controllers/humioingesttoken_controller.go index 427c5892..7e399e76 100644 --- a/controllers/humioingesttoken_controller.go +++ b/controllers/humioingesttoken_controller.go @@ -78,12 +78,11 @@ func (r *HumioIngestTokenReconciler) Reconcile(ctx context.Context, req ctrl.Req cluster, err := helpers.NewCluster(ctx, r, hit.Spec.ManagedClusterName, hit.Spec.ExternalClusterName, hit.Namespace, helpers.UseCertManager(), true) if err != nil || cluster == nil || cluster.Config() == nil { - r.Log.Error(err, "unable to obtain humio client config") - err = r.setState(ctx, humiov1alpha1.HumioIngestTokenStateConfigError, hit) - if err != nil { - return reconcile.Result{}, r.logErrorAndReturn(err, "unable to set cluster state") + setStateErr := r.setState(ctx, humiov1alpha1.HumioIngestTokenStateConfigError, hit) + if setStateErr != nil { + return reconcile.Result{}, r.logErrorAndReturn(setStateErr, "unable to set cluster state") } - return reconcile.Result{RequeueAfter: time.Second * 15}, nil + return reconcile.Result{RequeueAfter: 5 * time.Second}, r.logErrorAndReturn(err, "unable to obtain humio client config") } r.Log.Info("Checking if ingest token is marked to be deleted") diff --git a/controllers/humioparser_controller.go b/controllers/humioparser_controller.go index 088153e7..af07b063 100644 --- a/controllers/humioparser_controller.go +++ b/controllers/humioparser_controller.go @@ -79,12 +79,11 @@ func (r *HumioParserReconciler) Reconcile(ctx context.Context, req ctrl.Request) cluster, err := helpers.NewCluster(ctx, r, hp.Spec.ManagedClusterName, hp.Spec.ExternalClusterName, hp.Namespace, helpers.UseCertManager(), true) if err != nil || cluster == nil || cluster.Config() == nil { - r.Log.Error(err, "unable to obtain humio client config") - err = r.setState(ctx, humiov1alpha1.HumioParserStateConfigError, hp) - if err != nil { - return reconcile.Result{}, r.logErrorAndReturn(err, "unable to set cluster state") + setStateErr := r.setState(ctx, humiov1alpha1.HumioParserStateConfigError, hp) + if setStateErr != nil { + return reconcile.Result{}, r.logErrorAndReturn(setStateErr, "unable to set cluster state") } - return reconcile.Result{RequeueAfter: time.Second * 15}, nil + return reconcile.Result{RequeueAfter: 5 * time.Second}, r.logErrorAndReturn(err, "unable to obtain humio client config") } r.Log.Info("Checking if parser is marked to be deleted") diff --git a/controllers/humiorepository_controller.go b/controllers/humiorepository_controller.go index 03fac547..bbb77660 100644 --- a/controllers/humiorepository_controller.go +++ b/controllers/humiorepository_controller.go @@ -77,12 +77,11 @@ func (r *HumioRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Requ cluster, err := helpers.NewCluster(ctx, r, hr.Spec.ManagedClusterName, hr.Spec.ExternalClusterName, hr.Namespace, helpers.UseCertManager(), true) if err != nil || cluster == nil || cluster.Config() == nil { - r.Log.Error(err, "unable to obtain humio client config") - err = r.setState(ctx, humiov1alpha1.HumioRepositoryStateConfigError, hr) - if err != nil { - return reconcile.Result{}, r.logErrorAndReturn(err, "unable to set cluster state") + setStateErr := r.setState(ctx, humiov1alpha1.HumioRepositoryStateConfigError, hr) + if setStateErr != nil { + return reconcile.Result{}, r.logErrorAndReturn(setStateErr, "unable to set cluster state") } - return reconcile.Result{RequeueAfter: time.Second * 15}, nil + return reconcile.Result{RequeueAfter: 5 * time.Second}, r.logErrorAndReturn(err, "unable to obtain humio client config") } r.Log.Info("Checking if repository is marked to be deleted") diff --git a/controllers/humioscheduledsearch_controller.go b/controllers/humioscheduledsearch_controller.go index 871efb68..71307a6f 100644 --- a/controllers/humioscheduledsearch_controller.go +++ b/controllers/humioscheduledsearch_controller.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "reflect" + "time" "github.com/humio/humio-operator/pkg/kubernetes" @@ -78,12 +79,11 @@ func (r *HumioScheduledSearchReconciler) Reconcile(ctx context.Context, req ctrl cluster, err := helpers.NewCluster(ctx, r, hss.Spec.ManagedClusterName, hss.Spec.ExternalClusterName, hss.Namespace, helpers.UseCertManager(), true) if err != nil || cluster == nil || cluster.Config() == nil { - r.Log.Error(err, "unable to obtain humio client config") - err = r.setState(ctx, humiov1alpha1.HumioScheduledSearchStateConfigError, hss) - if err != nil { - return reconcile.Result{}, r.logErrorAndReturn(err, "unable to set scheduled search state") + setStateErr := r.setState(ctx, humiov1alpha1.HumioScheduledSearchStateConfigError, hss) + if setStateErr != nil { + return reconcile.Result{}, r.logErrorAndReturn(setStateErr, "unable to set scheduled search state") } - return reconcile.Result{}, err + return reconcile.Result{RequeueAfter: 5 * time.Second}, r.logErrorAndReturn(err, "unable to obtain humio client config") } defer func(ctx context.Context, humioClient humio.Client, hss *humiov1alpha1.HumioScheduledSearch) { @@ -184,7 +184,7 @@ func (r *HumioScheduledSearchReconciler) reconcileHumioScheduledSearch(ctx conte } r.Log.Info("done reconciling, will requeue after 15 seconds") - return reconcile.Result{}, nil + return reconcile.Result{RequeueAfter: time.Second * 15}, nil } // SetupWithManager sets up the controller with the Manager. diff --git a/controllers/humioview_controller.go b/controllers/humioview_controller.go index bcb25e8b..146673b9 100644 --- a/controllers/humioview_controller.go +++ b/controllers/humioview_controller.go @@ -77,12 +77,11 @@ func (r *HumioViewReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( cluster, err := helpers.NewCluster(ctx, r, hv.Spec.ManagedClusterName, hv.Spec.ExternalClusterName, hv.Namespace, helpers.UseCertManager(), true) if err != nil || cluster == nil || cluster.Config() == nil { - r.Log.Error(err, "unable to obtain humio client config") - err = r.setState(ctx, humiov1alpha1.HumioParserStateConfigError, hv) - if err != nil { - return reconcile.Result{}, r.logErrorAndReturn(err, "unable to set cluster state") + setStateErr := r.setState(ctx, humiov1alpha1.HumioParserStateConfigError, hv) + if setStateErr != nil { + return reconcile.Result{}, r.logErrorAndReturn(setStateErr, "unable to set cluster state") } - return reconcile.Result{RequeueAfter: time.Second * 15}, nil + return reconcile.Result{RequeueAfter: 5 * time.Second}, r.logErrorAndReturn(err, "unable to obtain humio client config") } defer func(ctx context.Context, humioClient humio.Client, hv *humiov1alpha1.HumioView) { diff --git a/examples/humioscheduledsearch.yaml b/examples/humioscheduledsearch.yaml index 4b37d7be..1bc80ee1 100644 --- a/examples/humioscheduledsearch.yaml +++ b/examples/humioscheduledsearch.yaml @@ -9,7 +9,7 @@ spec: queryString: "#repo = humio | error = true | count()" queryStart: "1h" queryEnd: "now" - schedule: "1h" + schedule: "0 * * * *" timeZone: "UTC" backfillLimit: 3 enabled: true @@ -28,7 +28,7 @@ spec: queryString: "#repo = humio | error = true | count()" queryStart: "1h" queryEnd: "now" - schedule: "1h" + schedule: "0 * * * *" timeZone: "UTC" backfillLimit: 3 enabled: true diff --git a/pkg/humio/client.go b/pkg/humio/client.go index dd8f76fd..efc86883 100644 --- a/pkg/humio/client.go +++ b/pkg/humio/client.go @@ -17,6 +17,7 @@ limitations under the License. package humio import ( + "errors" "fmt" "net/http" "net/url" @@ -322,6 +323,10 @@ func (h *ClientConfig) UpdateParser(config *humioapi.Config, req reconcile.Reque } func (h *ClientConfig) DeleteParser(config *humioapi.Config, req reconcile.Request, hp *humiov1alpha1.HumioParser) error { + _, err := h.GetParser(config, req, hp) + if errors.As(err, &humioapi.EntityNotFound{}) { + return nil + } return h.GetHumioClient(config, req).Parsers().Delete(hp.Spec.RepositoryName, hp.Spec.Name) } @@ -563,6 +568,10 @@ func (h *ClientConfig) UpdateAction(config *humioapi.Config, req reconcile.Reque } func (h *ClientConfig) DeleteAction(config *humioapi.Config, req reconcile.Request, ha *humiov1alpha1.HumioAction) error { + _, err := h.GetAction(config, req, ha) + if errors.As(err, &humioapi.EntityNotFound{}) { + return nil + } return h.GetHumioClient(config, req).Actions().Delete(ha.Spec.ViewName, ha.Spec.Name) } @@ -650,6 +659,10 @@ func (h *ClientConfig) UpdateAlert(config *humioapi.Config, req reconcile.Reques } func (h *ClientConfig) DeleteAlert(config *humioapi.Config, req reconcile.Request, ha *humiov1alpha1.HumioAlert) error { + _, err := h.GetAlert(config, req, ha) + if errors.As(err, &humioapi.EntityNotFound{}) { + return nil + } return h.GetHumioClient(config, req).Alerts().Delete(ha.Spec.ViewName, ha.Spec.Name) } @@ -728,6 +741,9 @@ func (h *ClientConfig) UpdateFilterAlert(config *humioapi.Config, req reconcile. func (h *ClientConfig) DeleteFilterAlert(config *humioapi.Config, req reconcile.Request, hfa *humiov1alpha1.HumioFilterAlert) error { currentAlert, err := h.GetFilterAlert(config, req, hfa) + if errors.As(err, &humioapi.EntityNotFound{}) { + return nil + } if err != nil { return fmt.Errorf("could not find filter alert with name: %q", hfa.Name) } @@ -809,6 +825,9 @@ func (h *ClientConfig) UpdateScheduledSearch(config *humioapi.Config, req reconc func (h *ClientConfig) DeleteScheduledSearch(config *humioapi.Config, req reconcile.Request, hss *humiov1alpha1.HumioScheduledSearch) error { currentScheduledSearch, err := h.GetScheduledSearch(config, req, hss) + if errors.As(err, &humioapi.EntityNotFound{}) { + return nil + } if err != nil { return fmt.Errorf("could not find scheduled search with name: %q", hss.Name) } diff --git a/pkg/humio/filteralert_transform.go b/pkg/humio/filteralert_transform.go index 4fac57af..543f173c 100644 --- a/pkg/humio/filteralert_transform.go +++ b/pkg/humio/filteralert_transform.go @@ -7,8 +7,7 @@ import ( ) const ( - FilterAlertIdentifierAnnotation = "humio.com/filter-alert-id" - FilterAlertQueryOwnershipTypeDefault = "Organization" + FilterAlertIdentifierAnnotation = "humio.com/filter-alert-id" ) func FilterAlertTransform(hfa *humiov1alpha1.HumioFilterAlert) (*humioapi.FilterAlert, error) { @@ -21,7 +20,7 @@ func FilterAlertTransform(hfa *humiov1alpha1.HumioFilterAlert) (*humioapi.Filter Enabled: hfa.Spec.Enabled, ActionNames: hfa.Spec.Actions, Labels: hfa.Spec.Labels, - QueryOwnershipType: FilterAlertQueryOwnershipTypeDefault, + QueryOwnershipType: humioapi.QueryOwnershipTypeOrganization, } if _, ok := hfa.ObjectMeta.Annotations[FilterAlertIdentifierAnnotation]; ok { diff --git a/pkg/humio/scheduledsearch_transform.go b/pkg/humio/scheduledsearch_transform.go index 09c41426..e262ee8e 100644 --- a/pkg/humio/scheduledsearch_transform.go +++ b/pkg/humio/scheduledsearch_transform.go @@ -7,8 +7,7 @@ import ( ) const ( - ScheduledSearchIdentifierAnnotation = "humio.com/scheduled-search-id" - ScheduledSearchQueryOwnershipTypeDefault = "Organization" + ScheduledSearchIdentifierAnnotation = "humio.com/scheduled-search-id" ) func ScheduledSearchTransform(hss *humiov1alpha1.HumioScheduledSearch) (*humioapi.ScheduledSearch, error) { @@ -24,7 +23,7 @@ func ScheduledSearchTransform(hss *humiov1alpha1.HumioScheduledSearch) (*humioap Enabled: hss.Spec.Enabled, ActionNames: hss.Spec.Actions, Labels: hss.Spec.Labels, - QueryOwnershipType: ScheduledSearchQueryOwnershipTypeDefault, + QueryOwnershipType: humioapi.QueryOwnershipTypeOrganization, } if _, ok := hss.ObjectMeta.Annotations[ScheduledSearchIdentifierAnnotation]; ok {