Skip to content
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.

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

follow-up: removal of the ratelimit ir #1451

Merged
merged 5 commits into from
May 30, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
19 changes: 7 additions & 12 deletions internal/cmd/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,14 +151,12 @@
return err
}

rateLimitInfraIR := new(message.RateLimitInfraIR)
// Start the Infra Manager Runner
// It subscribes to the infraIR, translates it into Envoy Proxy infrastructure
// resources such as K8s deployment and services.
infraRunner := infrarunner.New(&infrarunner.Config{
Server: *cfg,
InfraIR: infraIR,
RateLimitInfraIR: rateLimitInfraIR,
Server: *cfg,
InfraIR: infraIR,

Check warning on line 159 in internal/cmd/server.go

View check run for this annotation

Codecov / codecov/patch

internal/cmd/server.go#L158-L159

Added lines #L158 - L159 were not covered by tests
})
if err := infraRunner.Start(ctx); err != nil {
return err
Expand All @@ -175,15 +173,13 @@
return err
}

// Start the global rateLimit runner if it has been enabled through the config
// Start the global rateLimit if it has been enabled through the config
if cfg.EnvoyGateway.RateLimit != nil {
// Start the Global RateLimit Runner
// It subscribes to the xds Resources and translates it to Envoy Ratelimit Service
// infrastructure and configuration.
// Start the Global RateLimit xDS Server
// It subscribes to the xds Resources and translates it to Envoy Ratelimit configuration.

Check warning on line 179 in internal/cmd/server.go

View check run for this annotation

Codecov / codecov/patch

internal/cmd/server.go#L178-L179

Added lines #L178 - L179 were not covered by tests
rateLimitRunner := ratelimitrunner.New(&ratelimitrunner.Config{
Server: *cfg,
XdsIR: xdsIR,
RateLimitInfraIR: rateLimitInfraIR,
Server: *cfg,
XdsIR: xdsIR,

Check warning on line 182 in internal/cmd/server.go

View check run for this annotation

Codecov / codecov/patch

internal/cmd/server.go#L181-L182

Added lines #L181 - L182 were not covered by tests
})
if err := rateLimitRunner.Start(ctx); err != nil {
return err
Expand All @@ -196,7 +192,6 @@
pResources.Close()
xdsIR.Close()
infraIR.Close()
rateLimitInfraIR.Close()
xds.Close()

cfg.Logger.Info("shutting down")
Expand Down
27 changes: 8 additions & 19 deletions internal/globalratelimit/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,10 @@ const (

type Config struct {
config.Server
XdsIR *message.XdsIR
RateLimitInfraIR *message.RateLimitInfraIR
grpc *grpc.Server
cache cachev3.SnapshotCache
snapshotVersion int64
XdsIR *message.XdsIR
grpc *grpc.Server
cache cachev3.SnapshotCache
snapshotVersion int64
}

type Runner struct {
Expand Down Expand Up @@ -104,40 +103,30 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) {
r.Logger.Error(err, "failed to update the config snapshot")
}
} else {
// Translate to ratelimit infra IR and ratelimit xDS Config.
rlIR, rvt := r.translate(update.Value)
// Translate to ratelimit xDS Config.
rvt := r.translate(update.Value)

// Update ratelimit xDS config cache.
if rvt != nil {
r.updateSnapshot(ctx, rvt.XdsResources)
}

// Publish ratelimit infra IR.
if rlIR == nil {
r.RateLimitInfraIR.Delete(r.Name())
} else {
r.RateLimitInfraIR.Store(r.Name(), rlIR)
}
}
},
)
r.Logger.Info("subscriber shutting down")
}

func (r *Runner) translate(xdsIR *ir.Xds) (*ir.RateLimitInfra, *types.ResourceVersionTable) {
ratelimitInfra := new(ir.RateLimitInfra)
func (r *Runner) translate(xdsIR *ir.Xds) *types.ResourceVersionTable {
resourceVT := new(types.ResourceVersionTable)

for _, listener := range xdsIR.HTTP {
cfg := translator.BuildRateLimitServiceConfig(listener)
if cfg != nil {
// Add to xDS Config resources.
resourceVT.AddXdsResource(resourcev3.RateLimitConfigType, cfg)

ratelimitInfra.ServiceNames = append(ratelimitInfra.ServiceNames, listener.Name)
}
}
return ratelimitInfra, resourceVT
return resourceVT
}

func (r *Runner) updateSnapshot(ctx context.Context, resource types.XdsResources) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,20 @@ import (

egcfgv1a1 "github.com/envoyproxy/gateway/api/config/v1alpha1"
"github.com/envoyproxy/gateway/internal/infrastructure/kubernetes/resource"
"github.com/envoyproxy/gateway/internal/ir"
)

type ResourceRender struct {
// Namespace is the Namespace used for managed infra.
Namespace string

infra *ir.RateLimitInfra
rateLimit *egcfgv1a1.RateLimit
rateLimitDeployment *egcfgv1a1.KubernetesDeploymentSpec
}

// NewResourceRender returns a new ResourceRender.
func NewResourceRender(ns string, infra *ir.RateLimitInfra, gateway *egcfgv1a1.EnvoyGateway) *ResourceRender {
func NewResourceRender(ns string, gateway *egcfgv1a1.EnvoyGateway) *ResourceRender {
return &ResourceRender{
Namespace: ns,
infra: infra,
rateLimit: gateway.RateLimit,
rateLimitDeployment: gateway.GetEnvoyGatewayProvider().GetEnvoyGatewayKubeProvider().RateLimitDeployment,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,13 @@ import (

egcfgv1a1 "github.com/envoyproxy/gateway/api/config/v1alpha1"
"github.com/envoyproxy/gateway/internal/envoygateway/config"
"github.com/envoyproxy/gateway/internal/ir"
)

const (
// RedisAuthEnvVar is the redis auth.
RedisAuthEnvVar = "REDIS_AUTH"
)

var (
rateLimitListener = "ratelimit-listener"
)

func TestRateLimitLabels(t *testing.T) {
cases := []struct {
name string
Expand Down Expand Up @@ -61,7 +56,6 @@ func TestServiceAccount(t *testing.T) {
cfg, err := config.New()
require.NoError(t, err)

rateLimitInfra := new(ir.RateLimitInfra)
cfg.EnvoyGateway.RateLimit = &egcfgv1a1.RateLimit{
Backend: egcfgv1a1.RateLimitDatabaseBackend{
Type: egcfgv1a1.RedisBackendType,
Expand All @@ -70,7 +64,7 @@ func TestServiceAccount(t *testing.T) {
},
},
}
r := NewResourceRender(cfg.Namespace, rateLimitInfra, cfg.EnvoyGateway)
r := NewResourceRender(cfg.Namespace, cfg.EnvoyGateway)

sa, err := r.ServiceAccount()
require.NoError(t, err)
Expand All @@ -95,9 +89,6 @@ func TestService(t *testing.T) {
cfg, err := config.New()
require.NoError(t, err)

rateLimitInfra := &ir.RateLimitInfra{
ServiceNames: []string{rateLimitListener},
}
cfg.EnvoyGateway.RateLimit = &egcfgv1a1.RateLimit{
Backend: egcfgv1a1.RateLimitDatabaseBackend{
Type: egcfgv1a1.RedisBackendType,
Expand All @@ -106,7 +97,7 @@ func TestService(t *testing.T) {
},
},
}
r := NewResourceRender(cfg.Namespace, rateLimitInfra, cfg.EnvoyGateway)
r := NewResourceRender(cfg.Namespace, cfg.EnvoyGateway)
svc, err := r.Service()
require.NoError(t, err)

Expand Down Expand Up @@ -477,18 +468,14 @@ func TestDeployment(t *testing.T) {
}
for _, tc := range cases {
t.Run(tc.caseName, func(t *testing.T) {
rateLimitInfra := &ir.RateLimitInfra{
ServiceNames: []string{rateLimitListener},
}

cfg.EnvoyGateway.RateLimit = tc.rateLimit

cfg.EnvoyGateway.Provider = &egcfgv1a1.EnvoyGatewayProvider{
Type: egcfgv1a1.ProviderTypeKubernetes,
Kubernetes: &egcfgv1a1.EnvoyGatewayKubernetesProvider{
RateLimitDeployment: tc.deploy,
}}
r := NewResourceRender(cfg.Namespace, rateLimitInfra, cfg.EnvoyGateway)
r := NewResourceRender(cfg.Namespace, cfg.EnvoyGateway)
dp, err := r.Deployment()
require.NoError(t, err)

Expand Down
13 changes: 3 additions & 10 deletions internal/infrastructure/kubernetes/ratelimit_deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,12 @@ import (
"github.com/envoyproxy/gateway/internal/envoygateway"
"github.com/envoyproxy/gateway/internal/envoygateway/config"
"github.com/envoyproxy/gateway/internal/infrastructure/kubernetes/ratelimit"
"github.com/envoyproxy/gateway/internal/ir"
)

func TestCreateOrUpdateRateLimitDeployment(t *testing.T) {
cfg, err := config.New()
require.NoError(t, err)

rateLimitInfra := new(ir.RateLimitInfra)
cfg.EnvoyGateway.RateLimit = &egcfgv1a1.RateLimit{
Backend: egcfgv1a1.RateLimitDatabaseBackend{
Type: egcfgv1a1.RedisBackendType,
Expand All @@ -36,30 +34,26 @@ func TestCreateOrUpdateRateLimitDeployment(t *testing.T) {
},
}

r := ratelimit.NewResourceRender(cfg.Namespace, rateLimitInfra, cfg.EnvoyGateway)
r := ratelimit.NewResourceRender(cfg.Namespace, cfg.EnvoyGateway)
deployment, err := r.Deployment()
require.NoError(t, err)

testCases := []struct {
name string
in *ir.RateLimitInfra
current *appsv1.Deployment
want *appsv1.Deployment
}{
{
name: "create ratelimit deployment",
in: rateLimitInfra,
want: deployment,
},
{
name: "ratelimit deployment exists",
in: rateLimitInfra,
current: deployment,
want: deployment,
},
{
name: "update ratelimit deployment image",
in: &ir.RateLimitInfra{},
current: deployment,
want: deploymentWithImage(deployment, egcfgv1a1.DefaultRateLimitImage),
},
Expand All @@ -77,7 +71,7 @@ func TestCreateOrUpdateRateLimitDeployment(t *testing.T) {

kube := NewInfra(cli, cfg)
kube.EnvoyGateway.RateLimit = cfg.EnvoyGateway.RateLimit
r := ratelimit.NewResourceRender(kube.Namespace, tc.in, kube.EnvoyGateway)
r := ratelimit.NewResourceRender(kube.Namespace, kube.EnvoyGateway)
err := kube.createOrUpdateDeployment(context.Background(), r)
require.NoError(t, err)

Expand Down Expand Up @@ -118,9 +112,8 @@ func TestDeleteRateLimitDeployment(t *testing.T) {
tc := tc
t.Run(tc.name, func(t *testing.T) {
kube := newTestInfra(t)
rateLimitInfra := new(ir.RateLimitInfra)
kube.EnvoyGateway.RateLimit = rl
r := ratelimit.NewResourceRender(kube.Namespace, rateLimitInfra, kube.EnvoyGateway)
r := ratelimit.NewResourceRender(kube.Namespace, kube.EnvoyGateway)
err := kube.createOrUpdateDeployment(context.Background(), r)
require.NoError(t, err)

Expand Down
18 changes: 4 additions & 14 deletions internal/infrastructure/kubernetes/ratelimit_infra.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,36 +7,26 @@ package kubernetes

import (
"context"
"errors"

"github.com/envoyproxy/gateway/internal/infrastructure/kubernetes/ratelimit"
"github.com/envoyproxy/gateway/internal/ir"
)

// CreateOrUpdateRateLimitInfra creates the managed kube rate limit infra, if it doesn't exist.
func (i *Infra) CreateOrUpdateRateLimitInfra(ctx context.Context, infra *ir.RateLimitInfra) error {
if infra == nil {
return errors.New("ratelimit infra ir is nil")
}

func (i *Infra) CreateOrUpdateRateLimitInfra(ctx context.Context) error {
if err := ratelimit.Validate(ctx, i.Client.Client, i.EnvoyGateway, i.Namespace); err != nil {
return err
}

r := ratelimit.NewResourceRender(i.Namespace, infra, i.EnvoyGateway)
r := ratelimit.NewResourceRender(i.Namespace, i.EnvoyGateway)
return i.createOrUpdate(ctx, r)
}

// DeleteRateLimitInfra removes the managed kube infra, if it doesn't exist.
func (i *Infra) DeleteRateLimitInfra(ctx context.Context, infra *ir.RateLimitInfra) error {
if infra == nil {
return errors.New("ratelimit infra ir is nil")
}

func (i *Infra) DeleteRateLimitInfra(ctx context.Context) error {
if err := ratelimit.Validate(ctx, i.Client.Client, i.EnvoyGateway, i.Namespace); err != nil {
return err
}

r := ratelimit.NewResourceRender(i.Namespace, infra, i.EnvoyGateway)
r := ratelimit.NewResourceRender(i.Namespace, i.EnvoyGateway)
return i.delete(ctx, r)
}
21 changes: 2 additions & 19 deletions internal/infrastructure/kubernetes/ratelimit_infra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/envoyproxy/gateway/internal/infrastructure/kubernetes/ratelimit"
"github.com/envoyproxy/gateway/internal/ir"
"github.com/envoyproxy/gateway/internal/provider/kubernetes"
)

Expand All @@ -41,21 +40,12 @@ func createRateLimitTLSSecret(t *testing.T, client client.Client) {
}

func TestCreateRateLimitInfra(t *testing.T) {
rateLimitInfra := new(ir.RateLimitInfra)

testCases := []struct {
name string
in *ir.RateLimitInfra
expect bool
}{
{
name: "nil-infra",
in: nil,
expect: false,
},
{
name: "default infra",
in: rateLimitInfra,
expect: true,
},
}
Expand All @@ -68,7 +58,7 @@ func TestCreateRateLimitInfra(t *testing.T) {

createRateLimitTLSSecret(t, kube.Client.Client)

err := kube.CreateOrUpdateRateLimitInfra(context.Background(), tc.in)
err := kube.CreateOrUpdateRateLimitInfra(context.Background())
if !tc.expect {
require.Error(t, err)
} else {
Expand Down Expand Up @@ -106,17 +96,10 @@ func TestCreateRateLimitInfra(t *testing.T) {
func TestDeleteRateLimitInfra(t *testing.T) {
testCases := []struct {
name string
in *ir.RateLimitInfra
expect bool
}{
{
name: "nil infra",
in: nil,
expect: false,
},
{
name: "default infra",
in: new(ir.RateLimitInfra),
expect: true,
},
}
Expand All @@ -129,7 +112,7 @@ func TestDeleteRateLimitInfra(t *testing.T) {

createRateLimitTLSSecret(t, kube.Client.Client)

err := kube.DeleteRateLimitInfra(context.Background(), tc.in)
err := kube.DeleteRateLimitInfra(context.Background())
if !tc.expect {
require.Error(t, err)
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (

egcfgv1a1 "github.com/envoyproxy/gateway/api/config/v1alpha1"
"github.com/envoyproxy/gateway/internal/infrastructure/kubernetes/ratelimit"
"github.com/envoyproxy/gateway/internal/ir"
)

func TestDeleteRateLimitService(t *testing.T) {
Expand All @@ -39,9 +38,8 @@ func TestDeleteRateLimitService(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
kube := newTestInfra(t)

rateLimitInfra := new(ir.RateLimitInfra)
kube.EnvoyGateway.RateLimit = rl
r := ratelimit.NewResourceRender(kube.Namespace, rateLimitInfra, kube.EnvoyGateway)
r := ratelimit.NewResourceRender(kube.Namespace, kube.EnvoyGateway)
err := kube.createOrUpdateService(context.Background(), r)
require.NoError(t, err)

Expand Down
Loading