From ccd35744fd447d9face42415ed62efa63c71ebdd Mon Sep 17 00:00:00 2001 From: Kamil Kasperski Date: Tue, 24 Dec 2024 13:02:12 +0100 Subject: [PATCH] bug: RateLimit: replace Watch with polling (#1558) /kind bug /area api-gateway * Requeue reconciliation every 3 minutes, accepting downtime * Add warning state to the CR status * Adjust tests Restore watching should be handled in a follow-up anytime soon. --- .../ratelimit/v1alpha1/ratelimit_types.go | 12 +++++-- .../gateway.kyma-project.io_ratelimits.yaml | 2 +- config/rbac/role.yaml | 32 +++++++++++++++++++ .../gateway/ratelimit/ratelimit_controller.go | 24 ++++++++------ .../ratelimit_controller_integration_test.go | 5 +-- .../gateway/ratelimit/ratelimit_enabled.go | 8 +++-- internal/dependencies/dependencies.go | 8 +++++ 7 files changed, 74 insertions(+), 17 deletions(-) diff --git a/apis/gateway/ratelimit/v1alpha1/ratelimit_types.go b/apis/gateway/ratelimit/v1alpha1/ratelimit_types.go index d9e329ec7..b283a0934 100644 --- a/apis/gateway/ratelimit/v1alpha1/ratelimit_types.go +++ b/apis/gateway/ratelimit/v1alpha1/ratelimit_types.go @@ -21,8 +21,9 @@ import ( ) const ( - StatusReady = "Ready" - StatusError = "Error" + StatusReady = "Ready" + StatusWarning = "Warning" + StatusError = "Error" ) // BucketConfig represents a rate limit bucket configuration. @@ -70,7 +71,7 @@ type RateLimitSpec struct { type RateLimitStatus struct { // Description defines the description of current State of RateLimit. Description string `json:"description,omitempty"` - // State describes the overall status of RateLimit. Values are `Ready`, `Processing` and `Error` + // State describes the overall status of RateLimit. Values are `Ready`, `Processing`, `Warning` and `Error` State string `json:"state,omitempty"` } @@ -84,6 +85,11 @@ func (s *RateLimitStatus) Ready() { s.Description = "Finished reconciliation" } +func (s *RateLimitStatus) Warning(err error) { + s.State = StatusWarning + s.Description = err.Error() +} + //+kubebuilder:object:root=true //+kubebuilder:subresource:status diff --git a/config/crd/bases/gateway.kyma-project.io_ratelimits.yaml b/config/crd/bases/gateway.kyma-project.io_ratelimits.yaml index 3fc54c979..ce86b31bb 100644 --- a/config/crd/bases/gateway.kyma-project.io_ratelimits.yaml +++ b/config/crd/bases/gateway.kyma-project.io_ratelimits.yaml @@ -130,7 +130,7 @@ spec: type: string state: description: State describes the overall status of RateLimit. Values - are `Ready`, `Processing` and `Error` + are `Ready`, `Processing`, `Warning` and `Error` type: string type: object type: object diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 4a93f8b58..8f31c1d11 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -153,6 +153,38 @@ rules: - get - patch - update +- apiGroups: + - gateway.kyma-project.io + resources: + - ratelimits + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - gateway.kyma-project.io + resources: + - ratelimits/status + verbs: + - get + - patch + - update +- apiGroups: + - networking.istio.io + resources: + - envoyfilters + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - networking.istio.io resources: diff --git a/controllers/gateway/ratelimit/ratelimit_controller.go b/controllers/gateway/ratelimit/ratelimit_controller.go index a46fb35ce..840b72ac2 100644 --- a/controllers/gateway/ratelimit/ratelimit_controller.go +++ b/controllers/gateway/ratelimit/ratelimit_controller.go @@ -21,9 +21,9 @@ import ( "fmt" ratelimitv1alpha1 "github.com/kyma-project/api-gateway/apis/gateway/ratelimit/v1alpha1" "github.com/kyma-project/api-gateway/internal/builders/envoyfilter" + "github.com/kyma-project/api-gateway/internal/dependencies" "github.com/kyma-project/api-gateway/internal/ratelimit" "istio.io/api/networking/v1alpha3" - networkingv1alpha3 "istio.io/client-go/pkg/apis/networking/v1alpha3" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" @@ -33,17 +33,16 @@ import ( "time" ) -const defaultReconciliationPeriod = 30 * time.Minute - // RateLimitReconciler reconciles a RateLimit object type RateLimitReconciler struct { client.Client - Scheme *runtime.Scheme + Scheme *runtime.Scheme + ReconcilePeriod time.Duration } -// There should be no kubebuilder:rbac markers in this file as it's hard to modify the ClusterRole rules array in -// kustomize. The roles are managed in the file config/dev/kustomization.yaml. Once this feature is ready for release, -// the markers can be added again. +//+kubebuilder:rbac:groups=gateway.kyma-project.io,resources=ratelimits,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups=gateway.kyma-project.io,resources=ratelimits/status,verbs=get;update;patch +//+kubebuilder:rbac:groups=networking.istio.io,resources=envoyfilters,verbs=get;list;watch;create;update;patch;delete // Reconcile is part of the main Kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. @@ -59,6 +58,14 @@ func (r *RateLimitReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, client.IgnoreNotFound(err) } + if d, err := dependencies.RateLimit().AreAvailable(ctx, r.Client); err != nil { + rl.Status.Error(fmt.Errorf("dependency missing '%s': %w", d, err)) + if err := r.Status().Update(ctx, &rl); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, err + } + l.Info("Validating RateLimit resource") err := ratelimit.Validate(ctx, r.Client, rl) if err != nil { @@ -121,7 +128,7 @@ func (r *RateLimitReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( if err := r.Status().Update(ctx, &rl); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: defaultReconciliationPeriod}, nil + return ctrl.Result{RequeueAfter: r.ReconcilePeriod}, nil } func (r *RateLimitReconciler) createOrUpdate(ctx context.Context, obj client.Object, mutate func() error) error { @@ -151,6 +158,5 @@ func (r *RateLimitReconciler) createOrUpdate(ctx context.Context, obj client.Obj func (r *RateLimitReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&ratelimitv1alpha1.RateLimit{}). - Owns(&networkingv1alpha3.EnvoyFilter{}). Complete(r) } diff --git a/controllers/gateway/ratelimit/ratelimit_controller_integration_test.go b/controllers/gateway/ratelimit/ratelimit_controller_integration_test.go index 1b25c6542..511c15e83 100644 --- a/controllers/gateway/ratelimit/ratelimit_controller_integration_test.go +++ b/controllers/gateway/ratelimit/ratelimit_controller_integration_test.go @@ -55,8 +55,9 @@ var _ = BeforeSuite(func() { c, err = ctrlclient.New(cfg, ctrlclient.Options{Scheme: s}) Expect(err).ToNot(HaveOccurred()) rec := &ratelimit.RateLimitReconciler{ - Client: c, - Scheme: s, + Client: c, + Scheme: s, + ReconcilePeriod: time.Second, } Expect(rec.SetupWithManager(mgr)).Should(Succeed()) go func() { diff --git a/controllers/gateway/ratelimit/ratelimit_enabled.go b/controllers/gateway/ratelimit/ratelimit_enabled.go index a6f72b110..7146faf01 100644 --- a/controllers/gateway/ratelimit/ratelimit_enabled.go +++ b/controllers/gateway/ratelimit/ratelimit_enabled.go @@ -8,13 +8,17 @@ import ( "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "sigs.k8s.io/controller-runtime/pkg/manager" + "time" ) +const defaultReconciliationPeriod = 3 * time.Minute + func Setup(mgr manager.Manager, scheme *runtime.Scheme) error { utilruntime.Must(ratelimitv1alpha1.AddToScheme(scheme)) utilruntime.Must(networkingv1alpha3.AddToScheme(scheme)) return (&RateLimitReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + ReconcilePeriod: defaultReconciliationPeriod, }).SetupWithManager(mgr) } diff --git a/internal/dependencies/dependencies.go b/internal/dependencies/dependencies.go index 3df20256a..af8492dd9 100644 --- a/internal/dependencies/dependencies.go +++ b/internal/dependencies/dependencies.go @@ -29,6 +29,14 @@ func Gardener() Dependencies { } } +func RateLimit() Dependencies { + return &dependencies{ + CRDNames: []string{ + "envoyfilters.networking.istio.io", + }, + } +} + func APIRule() Dependencies { return &dependencies{ CRDNames: []string{