Skip to content

Commit

Permalink
fix(k8s): clone annotations before modyfing (#12338)
Browse files Browse the repository at this point in the history
## 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
#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

<!-- Is there a MADR? An Issue? A related PR? -->

Fix #XX

<!--
> Changelog: skip
-->
<!--
Uncomment the above section to explicitly set a [`> Changelog:` entry
here](https://github.com/kumahq/kuma/blob/master/CONTRIBUTING.md#submitting-a-patch)?
-->

Signed-off-by: Lukasz Dziedziak <lukidzi@gmail.com>
  • Loading branch information
lukidzi authored Dec 19, 2024
1 parent c0ac858 commit 8248765
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion pkg/plugins/runtime/k8s/controllers/service_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package controllers
import (
"context"
"fmt"
"maps"

"github.com/go-logr/logr"
"github.com/pkg/errors"
Expand Down Expand Up @@ -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{}
}
Expand Down

0 comments on commit 8248765

Please sign in to comment.