Skip to content

Commit 5d5aaaa

Browse files
arkodgzirain
authored andcommitted
fix: use lock when accessing mergeGateways Set (#7124)
* fix: use lock when accessing mergeGateways Set * accessed in multiple goroutines to map proxy fleet to resource via labels Fixes this panic ``` fatal error: concurrent map read and map write 4 internal/runtime/maps.fatal({0x3b83570?, 0x8b3de58?}) 5 /opt/hostedtoolcache/go/1.24.7/x64/src/runtime/panic.go:1058 +0x18 6 k8s.io/apimachinery/pkg/util/sets.Set[...[].Has(...) 7 /home/runner/go/pkg/mod/k8s.io/apimachinery@v0.33.3/pkg/util/sets/set.go:78 8 github.com/envoyproxy/gateway/internal/provider/kubernetes.(*gatewayAPIReconciler).envoyObjectForGateway.func1({0x8bc2dc8, 0xc0054163f0}) 9 /home/runner/work/gateway/gateway/internal/provider/kubernetes/predicates.go:666 +0x7b 10 github.com/envoyproxy/gateway/internal/provider/kubernetes.(*gatewayAPIReconciler).envoyObjectForGateway(0xc0018858e8?, {0x8ba6648?, 0xc0009c8b90?}, 0x11?) 11 /home/runner/work/gateway/gateway/internal/provider/kubernetes/predicates.go:682 +0x5e 12 github.com/envoyproxy/gateway/internal/provider/kubernetes.(*gatewayAPIReconciler).updateStatusForGateway(0xc000516600, {0x8ba6648, 0xc0009c8b90}, 0xc00541a380) 13 /home/runner/work/gateway/gateway/internal/provider/kubernetes/status.go:579 +0x5a 14 github.com/envoyproxy/gateway/internal/provider/kubernetes.(*gatewayAPIReconciler).subscribeAndUpdateStatus.func2.1({{{0xc0096a7e46, 0x7}, {0xc001944cc0, 0x11}}, 0x0, 0xc0052fa690}, 0xc00a582a1 0) ``` Relates to #7115 (comment) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * lint Signed-off-by: Arko Dasgupta <arko@tetrate.io> --------- Signed-off-by: Arko Dasgupta <arko@tetrate.io>
1 parent a88aa0e commit 5d5aaaa

File tree

3 files changed

+32
-13
lines changed

3 files changed

+32
-13
lines changed

internal/provider/kubernetes/controller.go

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"context"
1010
"errors"
1111
"fmt"
12+
"sync"
1213
"time"
1314

1415
"github.com/telepresenceio/watchable"
@@ -68,6 +69,7 @@ type gatewayAPIReconciler struct {
6869
namespaceLabel *metav1.LabelSelector
6970
envoyGateway *egv1a1.EnvoyGateway
7071
mergeGateways sets.Set[string]
72+
mergeGatewaysMu sync.RWMutex
7173
resources *message.ProviderResources
7274
subscriptions *subscriptions
7375
extGVKs []schema.GroupVersionKind
@@ -93,6 +95,24 @@ type gatewayAPIReconciler struct {
9395
clusterTrustBundleExits bool
9496
}
9597

98+
// isGatewayClassMerged reports whether the supplied GatewayClass has mergeGateways enabled.
99+
func (r *gatewayAPIReconciler) isGatewayClassMerged(name string) bool {
100+
r.mergeGatewaysMu.RLock()
101+
defer r.mergeGatewaysMu.RUnlock()
102+
return r.mergeGateways.Has(name)
103+
}
104+
105+
// setGatewayClassMerge toggles mergeGateways tracking for the supplied GatewayClass.
106+
func (r *gatewayAPIReconciler) setGatewayClassMerge(name string, merged bool) {
107+
r.mergeGatewaysMu.Lock()
108+
defer r.mergeGatewaysMu.Unlock()
109+
if merged {
110+
r.mergeGateways.Insert(name)
111+
return
112+
}
113+
r.mergeGateways.Delete(name)
114+
}
115+
96116
type subscriptions struct {
97117
gatewayClassStatuses <-chan watchable.Snapshot[types.NamespacedName, *gwapiv1.GatewayClassStatus]
98118
gatewayStatuses <-chan watchable.Snapshot[types.NamespacedName, *gwapiv1.GatewayStatus]
@@ -492,11 +512,7 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Reques
492512
}
493513

494514
if gwcResource.EnvoyProxyForGatewayClass != nil && gwcResource.EnvoyProxyForGatewayClass.Spec.MergeGateways != nil {
495-
if *gwcResource.EnvoyProxyForGatewayClass.Spec.MergeGateways {
496-
r.mergeGateways.Insert(managedGC.Name)
497-
} else {
498-
r.mergeGateways.Delete(managedGC.Name)
499-
}
515+
r.setGatewayClassMerge(managedGC.Name, *gwcResource.EnvoyProxyForGatewayClass.Spec.MergeGateways)
500516
}
501517

502518
if len(gwcResource.Gateways) == 0 {
@@ -1402,7 +1418,7 @@ func (r *gatewayAPIReconciler) processGateways(ctx context.Context, managedGC *g
14021418
}
14031419

14041420
mergedGateways := false
1405-
if r.mergeGateways.Has(managedGC.Name) {
1421+
if r.isGatewayClassMerged(managedGC.Name) {
14061422
mergedGateways = true
14071423
// processGatewayClassParamsRef has been called for this gatewayclass, its EnvoyProxy should exist in resourceTree
14081424
r.processServiceClusterForGatewayClass(resourceTree.EnvoyProxyForGatewayClass, managedGC, resourceMap)

internal/provider/kubernetes/controller_offline.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import (
2828
// OfflineGatewayAPIReconciler can be used for non-kuberetes provider.
2929
// It can let other providers to have the same reconcile logic without rely on apiserver.
3030
type OfflineGatewayAPIReconciler struct {
31-
gatewayAPIReconciler
31+
*gatewayAPIReconciler
3232

3333
Client client.Client
3434
}
@@ -63,7 +63,7 @@ func NewOfflineGatewayAPIController(
6363
}
6464

6565
cli := newOfflineGatewayAPIClient()
66-
r := gatewayAPIReconciler{
66+
r := &gatewayAPIReconciler{
6767
client: cli,
6868
log: cfg.Logger,
6969
classController: gwapiv1.GatewayController(cfg.EnvoyGateway.Gateway.ControllerName),

internal/provider/kubernetes/predicates.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ func (r *gatewayAPIReconciler) validateServiceForReconcile(obj client.Object) bo
403403

404404
// Merged gateways will have only this label, update status of all Gateways under found GatewayClass.
405405
gcName, ok := labels[gatewayapi.OwningGatewayClassLabel]
406-
if ok && r.mergeGateways.Has(gcName) {
406+
if ok && r.isGatewayClassMerged(gcName) {
407407
if err := r.updateStatusForGatewaysUnderGatewayClass(ctx, gcName); err != nil {
408408
r.log.Info("no Gateways found under GatewayClass", "name", gcName)
409409
return false
@@ -641,7 +641,7 @@ func (r *gatewayAPIReconciler) validateObjectForReconcile(obj client.Object) boo
641641

642642
// Merged gateways will have only this label, update status of all Gateways under found GatewayClass.
643643
gcName, ok := labels[gatewayapi.OwningGatewayClassLabel]
644-
if ok && r.mergeGateways.Has(gcName) {
644+
if ok && r.isGatewayClassMerged(gcName) {
645645
if err := r.updateStatusForGatewaysUnderGatewayClass(ctx, gcName); err != nil {
646646
r.log.Info("no Gateways found under GatewayClass", "name", gcName)
647647
return false
@@ -663,9 +663,11 @@ func envoyObjectNamespace(r *gatewayAPIReconciler, gateway *gwapiv1.Gateway) str
663663
// envoyObjectForGateway returns the Envoy Deployment or DaemonSet, returning nil if neither exists.
664664
func (r *gatewayAPIReconciler) envoyObjectForGateway(ctx context.Context, gateway *gwapiv1.Gateway) (client.Object, error) {
665665
// Helper func to list and return the first object from results
666+
merged := r.isGatewayClassMerged(string(gateway.Spec.GatewayClassName))
667+
666668
listResource := func(list client.ObjectList) (client.Object, error) {
667669
if err := r.client.List(ctx, list, &client.ListOptions{
668-
LabelSelector: labels.SelectorFromSet(gatewayapi.OwnerLabels(gateway, r.mergeGateways.Has(string(gateway.Spec.GatewayClassName)))),
670+
LabelSelector: labels.SelectorFromSet(gatewayapi.OwnerLabels(gateway, merged)),
669671
Namespace: envoyObjectNamespace(r, gateway),
670672
}); err != nil {
671673
if !kerrors.IsNotFound(err) {
@@ -697,7 +699,8 @@ func (r *gatewayAPIReconciler) envoyObjectForGateway(ctx context.Context, gatewa
697699
// envoyServiceForGateway returns the Envoy service, returning nil if the service doesn't exist.
698700
func (r *gatewayAPIReconciler) envoyServiceForGateway(ctx context.Context, gateway *gwapiv1.Gateway) (*corev1.Service, error) {
699701
var services corev1.ServiceList
700-
labelSelector := labels.SelectorFromSet(labels.Set(gatewayapi.OwnerLabels(gateway, r.mergeGateways.Has(string(gateway.Spec.GatewayClassName)))))
702+
merged := r.isGatewayClassMerged(string(gateway.Spec.GatewayClassName))
703+
labelSelector := labels.SelectorFromSet(labels.Set(gatewayapi.OwnerLabels(gateway, merged)))
701704
if err := r.client.List(ctx, &services, &client.ListOptions{
702705
LabelSelector: labelSelector,
703706
Namespace: envoyObjectNamespace(r, gateway),
@@ -955,7 +958,7 @@ func (r *gatewayAPIReconciler) isProxyServiceCluster(labels map[string]string) b
955958
}
956959

957960
gcName, ok := labels[gatewayapi.OwningGatewayClassLabel]
958-
if ok && r.mergeGateways.Has(gcName) {
961+
if ok && r.isGatewayClassMerged(gcName) {
959962
return true
960963
}
961964

0 commit comments

Comments
 (0)