Controller-runtime: Registering multiple webhooks concurrently will cause a panic

[steven-zou]

The panic logs are:

fatal error: concurrent map writes

goroutine 339 [running]:
runtime.throw(0x2ff20bd, 0x15)
        /Users/zziming/go/go1.14.6/src/runtime/panic.go:1116 +0x72 fp=0xc0005f9990 sp=0xc0005f9960 pc=0x103b062
runtime.mapassign_faststr(0x2d92240, 0xc0001cc390, 0xc0007ce1e0, 0x2c, 0x41fb960)
        /Users/zziming/go/go1.14.6/src/runtime/map_faststr.go:211 +0x529 fp=0xc0005f9a10 sp=0xc0005f9990 pc=0x1018659
sigs.k8s.io/controller-runtime/pkg/webhook.(*Server).Register(0xc000481580, 0xc0007ce1e0, 0x2c, 0x330a680, 0xc000910090)
        /Users/zziming/work/harbor-operator/vendor/sigs.k8s.io/controller-runtime/pkg/webhook/server.go:120 +0x244 fp=0xc0005f9b38 sp=0xc0005f9a10 pc=0x24f4f24
sigs.k8s.io/controller-runtime/pkg/builder.(*WebhookBuilder).registerValidatingWebhook(0xc00050c540)
        /Users/zziming/work/harbor-operator/vendor/sigs.k8s.io/controller-runtime/pkg/builder/webhook.go:127 +0x3bc fp=0xc0005f9c80 sp=0xc0005f9b38 pc=0x262a07c
sigs.k8s.io/controller-runtime/pkg/builder.(*WebhookBuilder).registerWebhooks(0xc00050c540, 0x0, 0x0)
        /Users/zziming/work/harbor-operator/vendor/sigs.k8s.io/controller-runtime/pkg/builder/webhook.go:80 +0x1bf fp=0xc0005f9d50 sp=0xc0005f9c80 pc=0x26296af
sigs.k8s.io/controller-runtime/pkg/builder.(*WebhookBuilder).Complete(0xc00050c540, 0x0, 0x0)

The panic place is at the webhook server register func:

s.webhooks[path] = hook

// Register marks the given webhook as being served at the given path.
// It panics if two hooks are registered on the same path.
func (s *Server) Register(path string, hook http.Handler) {
    s.defaultingOnce.Do(s.setDefaults)
    _, found := s.webhooks[path]
    if found {
        panic(fmt.Errorf("can't register duplicate path: %v", path))
    }
    // TODO(directxman12): call setfields if we've already started the server
    s.webhooks[path] = hook
    s.WebhookMux.Handle(path, instrumentedHook(path, hook))
    log.Info("registering webhook", "path", path)
}

and webhooks is a just standard map.

[alvaroaleman]

/kind bug
/help

[k8s-ci-robot]

@alvaroaleman:
This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/kind bug
/help

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[jay-rob]

Hey there,

I've tried to reproduce this panic locally, and I have not been able to yet. I've noticed, based on the code snippet provided above and in https://github.com/goharbor/harbor-operator/issues/202, that you are using controller-runtime v0.6.1. Have you tried upgrading to v.0.6.4?

It looks like https://github.com/kubernetes-sigs/controller-runtime/commit/0f11ed4ba4b08cc61860a51e21029a5a2b7d7d7d added logic to provide better safer con-currency and you may just need to upgrade your controller-runtime version.

[steven-zou]

@jay-rob

Thanks for the info. I checked the code change in the commit 0f11ed4 you mentioned. It seems the concurrent write issue has been fixed. We'll try to upgrade controller-runtime to v.0.6.4 later for introducing the change.

[alvaroaleman]

Closing, as this was fixed in https://github.com/kubernetes-sigs/controller-runtime/commit/0f11ed4ba4b08cc61860a51e21029a5a2b7d7d7d