Skip to content

Commit

Permalink
fix panic when updating the envoy-gateway-config configMap (#5066) (#…
Browse files Browse the repository at this point in the history
…5115)

* fix panic when updating the envoy-gateway-config configMap


(cherry picked from commit 42ae06d)

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
  • Loading branch information
zhaohuabing authored Jan 22, 2025
1 parent 51a420a commit 24873a4
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 32 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
8 changes: 5 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,9 @@ 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 the elected channel to signal that this EG instance has been elected as leader.
// This allows the tasks that require leadership to proceed.
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
6 changes: 1 addition & 5 deletions release-notes/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,9 @@ security updates: |
# New features or capabilities added in this release.
new features: |
Add support for modifying container securityContext for Envoy Gateway deployment in Helm
bug fixes: |
Fixed a nil pointer error that occurs when a SecurityPolicy refers to a UDS backend
Fixed the Gateway API translator didn't use the TLS configuration from the BackendTLSPolicy when connecting to the OIDC provider's well-known endpoint.
Fixed a validation failure when multiple HTTPRoutes refer to the same extension filter
Fixed a panic that occurred following update to the envoy-gateway-config ConfigMap
# Enhancements that improve performance.
performance improvements: |
Expand All @@ -27,4 +24,3 @@ deprecations: |
# Other notable changes not covered by the above sections.
Other changes: |
Bumped the version of Envoy to 1.32.3.

0 comments on commit 24873a4

Please sign in to comment.