Skip to content

Commit

Permalink
fix panic when updating the envoy-gateway-config configMap
Browse files Browse the repository at this point in the history
Signed-off-by: Huabing Zhao <[email protected]>
  • Loading branch information
zhaohuabing committed Jan 15, 2025
1 parent 18207c1 commit d9edb07
Show file tree
Hide file tree
Showing 10 changed files with 36 additions and 38 deletions.
4 changes: 4 additions & 0 deletions internal/cmd/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ func getConfigByPath(cfgPath string) (*config.Server, error) {
// setupRunners starts all the runners required for the Envoy Gateway to
// fulfill its tasks.
func setupRunners(ctx context.Context, cfg *config.Server) (err error) {
// The Elected channel is used to block the tasks that are waiting for the leader to be elected.
// It will be closed once the leader is elected in the controller manager.
cfg.Elected = make(chan struct{})

// Setup the Extension Manager
var extMgr types.Manager
if cfg.EnvoyGateway.Provider.Type == egv1a1.ProviderTypeKubernetes {
Expand Down
17 changes: 6 additions & 11 deletions internal/envoygateway/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package config

import (
"errors"
"sync"

egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
"github.com/envoyproxy/gateway/api/v1alpha1/validation"
Expand Down Expand Up @@ -37,23 +36,19 @@ type Server struct {
DNSDomain string
// Logger is the logr implementation used by Envoy Gateway.
Logger logging.Logger
// Elected chan is used to signal what a leader is elected
Elected *sync.WaitGroup
// Elected chan is used to signal when an EG instance is elected as leader.
Elected chan struct{}
}

// New returns a Server with default parameters.
func New() (*Server, error) {
server := &Server{
return &Server{
EnvoyGateway: egv1a1.DefaultEnvoyGateway(),
Namespace: env.Lookup("ENVOY_GATEWAY_NAMESPACE", DefaultNamespace),
DNSDomain: env.Lookup("KUBERNETES_CLUSTER_DOMAIN", DefaultDNSDomain),
// the default logger
Logger: logging.DefaultLogger(egv1a1.LogLevelInfo),
Elected: &sync.WaitGroup{},
}
// Block the tasks that are waiting for the leader to be elected
server.Elected.Add(1)
return server, nil
Logger: logging.DefaultLogger(egv1a1.LogLevelInfo),
Elected: make(chan struct{}),
}, nil
}

// Validate validates a Server config.
Expand Down
8 changes: 6 additions & 2 deletions internal/infrastructure/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,12 @@ func (r *Runner) Start(ctx context.Context) (err error) {
if r.EnvoyGateway.Provider.Type == egv1a1.ProviderTypeKubernetes &&
!ptr.Deref(r.EnvoyGateway.Provider.Kubernetes.LeaderElection.Disable, false) {
go func() {
r.Elected.Wait()
initInfra()
select {
case <-ctx.Done():
return
case <-r.Elected:
initInfra()
}
}()
return
}
Expand Down
12 changes: 7 additions & 5 deletions internal/provider/kubernetes/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,9 @@ type gatewayAPIReconciler struct {
}

// newGatewayAPIController
func newGatewayAPIController(mgr manager.Manager, cfg *config.Server, su Updater,
func newGatewayAPIController(ctx context.Context, mgr manager.Manager, cfg *config.Server, su Updater,
resources *message.ProviderResources,
) error {
ctx := context.Background()

// Gather additional resources to watch from registered extensions
var extServerPoliciesGVKs []schema.GroupVersionKind
var extGVKs []schema.GroupVersionKind
Expand Down Expand Up @@ -138,8 +136,12 @@ func newGatewayAPIController(mgr manager.Manager, cfg *config.Server, su Updater
if cfg.EnvoyGateway.Provider.Type == egv1a1.ProviderTypeKubernetes &&
!ptr.Deref(cfg.EnvoyGateway.Provider.Kubernetes.LeaderElection.Disable, false) {
go func() {
cfg.Elected.Wait()
r.subscribeAndUpdateStatus(ctx, cfg.EnvoyGateway.EnvoyGatewaySpec.ExtensionManager != nil)
select {
case <-ctx.Done():
return
case <-cfg.Elected:
r.subscribeAndUpdateStatus(ctx, cfg.EnvoyGateway.EnvoyGatewaySpec.ExtensionManager != nil)
}
}()
} else {
r.subscribeAndUpdateStatus(ctx, cfg.EnvoyGateway.EnvoyGatewaySpec.ExtensionManager != nil)
Expand Down
6 changes: 3 additions & 3 deletions internal/provider/kubernetes/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type Provider struct {
}

// New creates a new Provider from the provided EnvoyGateway.
func New(restCfg *rest.Config, svrCfg *ec.Server, resources *message.ProviderResources) (*Provider, error) {
func New(ctx context.Context, restCfg *rest.Config, svrCfg *ec.Server, resources *message.ProviderResources) (*Provider, error) {
// TODO: Decide which mgr opts should be exposed through envoygateway.provider.kubernetes API.

mgrOpts := manager.Options{
Expand Down Expand Up @@ -95,7 +95,7 @@ func New(restCfg *rest.Config, svrCfg *ec.Server, resources *message.ProviderRes
}

// Create and register the controllers with the manager.
if err := newGatewayAPIController(mgr, svrCfg, updateHandler.Writer(), resources); err != nil {
if err := newGatewayAPIController(ctx, mgr, svrCfg, updateHandler.Writer(), resources); err != nil {
return nil, fmt.Errorf("failted to create gatewayapi controller: %w", err)
}

Expand All @@ -112,7 +112,7 @@ func New(restCfg *rest.Config, svrCfg *ec.Server, resources *message.ProviderRes
// Emit elected & continue with the tasks that require leadership.
go func() {
<-mgr.Elected()
svrCfg.Elected.Done()
close(svrCfg.Elected)
}()

return &Provider{
Expand Down
6 changes: 3 additions & 3 deletions internal/provider/kubernetes/kubernetes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestProvider(t *testing.T) {
svr, err := config.New()
require.NoError(t, err)
resources := new(message.ProviderResources)
provider, err := New(cliCfg, svr, resources)
provider, err := New(context.Background(), cliCfg, svr, resources)
require.NoError(t, err)
ctx, cancel := context.WithCancel(ctrl.SetupSignalHandler())
go func() {
Expand Down Expand Up @@ -1274,7 +1274,7 @@ func TestNamespacedProvider(t *testing.T) {
LeaderElection: egv1a1.DefaultLeaderElection(),
}
resources := new(message.ProviderResources)
provider, err := New(cliCfg, svr, resources)
provider, err := New(context.Background(), cliCfg, svr, resources)
require.NoError(t, err)
ctx, cancel := context.WithCancel(context.Background())
go func() {
Expand Down Expand Up @@ -1334,7 +1334,7 @@ func TestNamespaceSelectorProvider(t *testing.T) {
LeaderElection: egv1a1.DefaultLeaderElection(),
}
resources := new(message.ProviderResources)
provider, err := New(cliCfg, svr, resources)
provider, err := New(context.Background(), cliCfg, svr, resources)
require.NoError(t, err)
ctx, cancel := context.WithCancel(context.Background())
go func() {
Expand Down
6 changes: 3 additions & 3 deletions internal/provider/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func (r *Runner) Start(ctx context.Context) (err error) {
var p provider.Provider
switch r.EnvoyGateway.Provider.Type {
case egv1a1.ProviderTypeKubernetes:
p, err = r.createKubernetesProvider()
p, err = r.createKubernetesProvider(ctx)
if err != nil {
return fmt.Errorf("failed to create kubernetes provider: %w", err)
}
Expand All @@ -69,13 +69,13 @@ func (r *Runner) Start(ctx context.Context) (err error) {
return nil
}

func (r *Runner) createKubernetesProvider() (*kubernetes.Provider, error) {
func (r *Runner) createKubernetesProvider(ctx context.Context) (*kubernetes.Provider, error) {
cfg, err := ctrl.GetConfig()
if err != nil {
return nil, fmt.Errorf("failed to get kubeconfig: %w", err)
}

p, err := kubernetes.New(cfg, &r.Config.Server, r.ProviderResources)
p, err := kubernetes.New(ctx, cfg, &r.Config.Server, r.ProviderResources)
if err != nil {
return nil, fmt.Errorf("failed to create provider %s: %w", egv1a1.ProviderTypeKubernetes, err)
}
Expand Down
2 changes: 1 addition & 1 deletion release-notes/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ new features: |
Added support for GEP-1731 (HTTPRoute Retries)
bug fixes: |
Fixed a panic that occurred following update to the envoy-gateway-config ConfigMap
# Enhancements that improve performance.
performance improvements: |
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func TestE2E(t *testing.T) {
Debug: *flags.ShowDebug,
CleanupBaseResources: *flags.CleanupBaseResources,
ManifestFS: []fs.FS{Manifests},
RunTest: *flags.RunTest,
RunTest: tests.RateLimitBasedJwtClaimsTest.ShortName,
// SupportedFeatures cannot be empty, so we set it to SupportGateway
// All e2e tests should leave Features empty.
SupportedFeatures: sets.New[features.FeatureName](features.SupportGateway),
Expand Down
11 changes: 2 additions & 9 deletions tools/make/kube.mk
Original file line number Diff line number Diff line change
Expand Up @@ -170,16 +170,9 @@ e2e-prepare: prepare-ip-family ## Prepare the environment for running e2e tests

.PHONY: run-e2e
run-e2e: e2e-prepare ## Run e2e tests
@$(LOG_TARGET)
ifeq ($(E2E_RUN_TEST),)

go test $(E2E_TEST_ARGS) ./test/e2e --gateway-class=envoy-gateway --debug=true --cleanup-base-resources=false
go test $(E2E_TEST_ARGS) ./test/e2e/merge_gateways --gateway-class=merge-gateways --debug=true --cleanup-base-resources=false
go test $(E2E_TEST_ARGS) ./test/e2e/multiple_gc --debug=true --cleanup-base-resources=true
LAST_VERSION_TAG=$(shell cat VERSION) go test $(E2E_TEST_ARGS) ./test/e2e/upgrade --gateway-class=upgrade --debug=true --cleanup-base-resources=$(E2E_CLEANUP)
else
go test $(E2E_TEST_ARGS) ./test/e2e --gateway-class=envoy-gateway --debug=true --cleanup-base-resources=$(E2E_CLEANUP) \
--run-test $(E2E_RUN_TEST)
endif


.PHONY: run-resilience
run-resilience: ## Run resilience tests
Expand Down

0 comments on commit d9edb07

Please sign in to comment.