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.
humio · Aug 8, 2024 · c2de1ac · c2de1ac
1 parent 5927362
commit c2de1ac
Show file tree

Hide file tree

Showing 13 changed files with 66 additions and 53 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 {