From 824876526ed053c08c52efc800c46cd0a9e625cc Mon Sep 17 00:00:00 2001 From: Lukasz Dziedziak Date: Thu, 19 Dec 2024 14:44:21 +0100 Subject: [PATCH] fix(k8s): clone annotations before modyfing (#12338) ## Motivation I was investigating a test fail on release-2.8 and noticed that we had a panic https://github.com/kumahq/kuma/actions/runs/12401863260/job/34624092612 ``` fatal error: concurrent map read and map write goroutine 1264 [running]: github.com/kumahq/kuma/pkg/plugins/runtime/k8s/metadata.Annotations.getWithDefault(0x4004194e70, {0x247fb40?, 0x505e860?}, 0x4001a8d7c8, {0x4001a8d848?, 0x3272270?, 0x4001a8d7f8?}) github.com/kumahq/kuma/pkg/plugins/runtime/k8s/metadata/annotations.go:347 +0x94 github.com/kumahq/kuma/pkg/plugins/runtime/k8s/metadata.Annotations.GetBooleanWithDefault(0x4001a8d858?, 0x0, 0x3c?, {0x4001a8d848?, 0x2203be4?, 0x25c85e0?}) github.com/kumahq/kuma/pkg/plugins/runtime/k8s/metadata/annotations.go:231 +0x64 github.com/kumahq/kuma/pkg/plugins/runtime/k8s/metadata.Annotations.GetEnabledWithDefault(...) github.com/kumahq/kuma/pkg/plugins/runtime/k8s/metadata/annotations.go:227 github.com/kumahq/kuma/pkg/plugins/runtime/k8s/metadata.Annotations.GetEnabled(0x1?, {0x4001a8d848?, 0x4000cc6d88?, 0x40008b1590?}) github.com/kumahq/kuma/pkg/plugins/runtime/k8s/metadata/annotations.go:219 +0x34 github.com/kumahq/kuma/pkg/plugins/runtime/k8s/controllers.(*PodReconciler).findMatchingServices.Ignored.func1(0x25deb80?) github.com/kumahq/kuma/pkg/plugins/runtime/k8s/util/util.go:56 +0x4c github.com/kumahq/kuma/pkg/plugins/runtime/k8s/util.MatchService(...) github.com/kumahq/kuma/pkg/plugins/runtime/k8s/util/util.go:63 github.com/kumahq/kuma/pkg/plugins/runtime/k8s/controllers.(*PodReconciler).findMatchingServices(0x400069ab40, {0x32d27f0, 0x4003e92ab0}, 0x4000cc6d88) github.com/kumahq/kuma/pkg/plugins/runtime/k8s/controllers/pod_controller.go:264 +0x5b4 github.com/kumahq/kuma/pkg/plugins/runtime/k8s/controllers.(*PodReconciler).reconcileDataplane(0x400069ab40, {0x32d27f0, 0x4003e92ab0}, 0x4000cc6d88, {{0x32da180?, 0x4003e92ae0?}, 0x0?}) github.com/kumahq/kuma/pkg/plugins/runtime/k8s/controllers/pod_controller.go:132 +0x2a0 github.com/kumahq/kuma/pkg/plugins/runtime/k8s/controllers.(*PodReconciler).Reconcile(0x400069ab40, {0x32d27f0, 0x4003e92ab0}, {{{0x40026b05a0, 0x11}, {0x4002644ba0, 0x22}}}) github.com/kumahq/kuma/pkg/plugins/runtime/k8s/controllers/pod_controller.go:101 +0x41c sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile(0x32da180?, {0x32d27f0?, 0x4003e92ab0?}, {{{0x40026b05a0?, 0xb?}, {0x4002644ba0?, 0x0?}}}) sigs.k8s.io/controller-runtime@v0.17.3/pkg/internal/controller/controller.go:119 +0x8c ``` It's not an issue in > 2.8 since this change https://github.com/kumahq/kuma/pull/10561 ## Implementation information While investigating a stack trace, I noticed that another goroutine was modifying a service object that we were also working with. I realized that we were not cloning the map before making edits to it. After reviewing the Kubernetes controller code, I identified the areas where these changes occur. I updated a few places to clone the map first before adding labels or making modifications. ``` goroutine 867 [runnable]: github.com/kumahq/kuma/pkg/plugins/runtime/k8s/controllers.(*ServiceReconciler).Reconcile(0x40007a8cf0, {0x32d27f0, 0x4004043320}, {{{0x40027c2c48, 0x11}, {0x4003e3e447, 0x7}}}) github.com/kumahq/kuma/pkg/plugins/runtime/k8s/controllers/service_controller.go:83 +0x7f4 sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile(0x32da180?, {0x32d27f0?, 0x4004043320?}, {{{0x40027c2c48?, 0xb?}, {0x4003e3e447?, 0x0?}}}) sigs.k8s.io/controller-runtime@v0.17.3/pkg/internal/controller/controller.go:119 +0x8c sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler(0x4000651220, {0x32d2828, 0x40009d0370}, {0x27baca0, 0x40012fb460}) ``` ## Supporting documentation Fix #XX Signed-off-by: Lukasz Dziedziak --- pkg/plugins/runtime/k8s/controllers/service_controller.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/plugins/runtime/k8s/controllers/service_controller.go b/pkg/plugins/runtime/k8s/controllers/service_controller.go index 98bc1b39322a..52d7a4089756 100644 --- a/pkg/plugins/runtime/k8s/controllers/service_controller.go +++ b/pkg/plugins/runtime/k8s/controllers/service_controller.go @@ -3,6 +3,7 @@ package controllers import ( "context" "fmt" + "maps" "github.com/go-logr/logr" "github.com/pkg/errors" @@ -68,7 +69,7 @@ func (r *ServiceReconciler) Reconcile(ctx context.Context, req kube_ctrl.Request } log.Info("annotating service which is part of the mesh", "annotation", fmt.Sprintf("%s=%s", metadata.IngressServiceUpstream, metadata.AnnotationTrue)) - annotations := metadata.Annotations(svc.Annotations) + annotations := metadata.Annotations(maps.Clone(svc.Annotations)) if annotations == nil { annotations = metadata.Annotations{} }