Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix runtime panic when trying to update operator controlled resources that don't have annotation or labels #433

Merged
merged 2 commits into from
May 24, 2019

Conversation

slok
Copy link
Contributor

@slok slok commented May 23, 2019

Hi!

First of all, thanks for this awesome software, it helps us a lot (also Jaeger! 😄).

This PR fixes the possibility of having runtime panics due to the operator trying to update annotations or labels on existing Kubernetes resources that don't have labels or annotations.

This happened to us when we upgraded from v1.11.0 to a newer version (v1.11.1 and v1.12.0), the operator listed the operator owned services and tried to update the annotations, it didn't have, so the map wasn't initialized and when updating it hit a panic due to the assignment on the map making the operator enter in crash loopback.

To fix it I initialized the annotation and label maps when they are nil, also I saw that this was on all the resources of the inventory, so I applied the same logic to all of the resources.

In the meantime, we managed to bypass the problem setting a dummy annotation on all the services controlled by the operator.

Here are tracebacks of the panic:

time="2019-05-23T11:48:16Z" level=info msg=Versions arch=amd64 jaeger-operator=1.12.0 operator-sdk=v0.5.0 os=linux version=go1.11.1
time="2019-05-23T11:48:16Z" level=info msg="Auto-detected the platform" platform=kubernetes
time="2019-05-23T11:48:16Z" level=info msg="Automatically adjusted the 'es-provision' flag" es-provision=false
E0523 11:48:18.127847       1 runtime.go:69] Observed a panic: "assignment to entry in nil map" (assignment to entry in nil map)
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:76
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:65
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:51
/home/travis/.gimme/versions/go1.11.1.linux.amd64/src/runtime/asm_amd64.s:522
/home/travis/.gimme/versions/go1.11.1.linux.amd64/src/runtime/panic.go:513
/home/travis/.gimme/versions/go1.11.1.linux.amd64/src/runtime/map_faststr.go:192
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/pkg/inventory/service.go:36
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/pkg/controller/jaeger/service.go:23
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/pkg/controller/jaeger/jaeger_controller.go:205
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/pkg/controller/jaeger/jaeger_controller.go:119
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:215
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:158
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:133
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:134
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:88
/home/travis/.gimme/versions/go1.11.1.linux.amd64/src/runtime/asm_amd64.s:1333
panic: assignment to entry in nil map [recovered]
        panic: assignment to entry in nil map

goroutine 503 [running]:
github.com/jaegertracing/jaeger-operator/vendor/k8s.io/apimachinery/pkg/util/runtime.HandleCrash(0x0, 0x0, 0x0)
        /home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:58 +0x108
panic(0x12ac620, 0x15b6ed0)
        /home/travis/.gimme/versions/go1.11.1.linux.amd64/src/runtime/panic.go:513 +0x1b9
github.com/jaegertracing/jaeger-operator/pkg/inventory.ForServices(0xc0002b4000, 0x3, 0x3, 0xc0002fc000, 0x3, 0x4, 0x0, 0x0, 0x0, 0x0, ...)
        /home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/pkg/inventory/service.go:36 +0x454
github.com/jaegertracing/jaeger-operator/pkg/controller/jaeger.(*ReconcileJaeger).applyServices(0xc0006a6420, 0x1426656, 0x6, 0xc000a6e020, 0x13, 0xc0004657d0, 0x6, 0x0, 0x0, 0xc0004657e0, ...)
        /home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/pkg/controller/jaeger/service.go:23 +0x247
github.com/jaegertracing/jaeger-operator/pkg/controller/jaeger.(*ReconcileJaeger).apply(0xc0006a6420, 0x1426656, 0x6, 0xc000a6e020, 0x13, 0xc0004657d0, 0x6, 0x0, 0x0, 0xc0004657e0, ...)
        /home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/pkg/controller/jaeger/jaeger_controller.go:205 +0x586
github.com/jaegertracing/jaeger-operator/pkg/controller/jaeger.(*ReconcileJaeger).Reconcile(0xc0006a6420, 0xc0004657e0, 0x6, 0xc0004657d0, 0x6, 0x21be880, 0x1, 0xc00032cc00, 0xc000560d88)
        /home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/pkg/controller/jaeger/jaeger_controller.go:119 +0x93a
github.com/jaegertracing/jaeger-operator/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem(0xc000379720, 0x0)
        /home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:215 +0x18f
github.com/jaegertracing/jaeger-operator/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func1()
        /home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:158 +0x36
github.com/jaegertracing/jaeger-operator/vendor/k8s.io/apimachinery/pkg/util/wait.JitterUntil.func1(0xc00039c0b0)
        /home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:133 +0x54
github.com/jaegertracing/jaeger-operator/vendor/k8s.io/apimachinery/pkg/util/wait.JitterUntil(0xc00039c0b0, 0x3b9aca00, 0x0, 0x1, 0xc000702360)
        /home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:134 +0xbe
github.com/jaegertracing/jaeger-operator/vendor/k8s.io/apimachinery/pkg/util/wait.Until(0xc00039c0b0, 0x3b9aca00, 0xc000702360)
        /home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:88 +0x4d
created by github.com/jaegertracing/jaeger-operator/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start
        /home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:157 +0x32a

Also happens on v1.11.1

time="2019-05-23T11:32:03Z" level=info msg=Versions arch=amd64 jaeger-operator=1.11.1 operator-sdk=v0.5.0 os=linux version=go1.11.1
time="2019-05-23T11:32:03Z" level=info msg=Versions arch=amd64 jaeger-operator=1.11.1 operator-sdk=v0.5.0 os=linux version=go1.11.1                                                                                  
time="2019-05-23T11:32:03Z" level=info msg="Auto-detected the platform" platform=kubernetes                                                                                                                          
time="2019-05-23T11:32:03Z" level=info msg="Automatically adjusted the 'es-provision' flag" es-provision=false                                                                                                       
E0523 11:32:04.510526       1 runtime.go:69] Observed a panic: "assignment to entry in nil map" (assignment to entry in nil map)                                                                                     
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:76                                                                                           
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:65                                                                                           
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:51                                                                                           
/home/travis/.gimme/versions/go1.11.1.linux.amd64/src/runtime/asm_amd64.s:522                                                                                                                                        
/home/travis/.gimme/versions/go1.11.1.linux.amd64/src/runtime/panic.go:513                                                                                                                                           
/home/travis/.gimme/versions/go1.11.1.linux.amd64/src/runtime/map_faststr.go:192                                                                                                                                     
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/pkg/inventory/service.go:36                                                                                                                         
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/pkg/controller/jaeger/service.go:23                                                                                                                 
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/pkg/controller/jaeger/jaeger_controller.go:205                                                                                                      
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/pkg/controller/jaeger/jaeger_controller.go:119
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:215
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:158
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:133
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:134
/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:88
/home/travis/.gimme/versions/go1.11.1.linux.amd64/src/runtime/asm_amd64.s:1333
panic: assignment to entry in nil map [recovered]
        panic: assignment to entry in nil map

...

Thanks!

@codecov
Copy link

codecov bot commented May 23, 2019

Codecov Report

Merging #433 into master will increase coverage by 1.32%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #433      +/-   ##
==========================================
+ Coverage   89.79%   91.11%   +1.32%     
==========================================
  Files          64       64              
  Lines        3115     3130      +15     
==========================================
+ Hits         2797     2852      +55     
+ Misses        216      196      -20     
+ Partials      102       82      -20
Impacted Files Coverage Δ
pkg/inventory/secret.go 100% <100%> (+12.9%) ⬆️
pkg/inventory/route.go 100% <100%> (+13.33%) ⬆️
pkg/inventory/deployment.go 100% <100%> (+13.33%) ⬆️
pkg/inventory/account.go 100% <100%> (+12.5%) ⬆️
pkg/util/util.go 100% <100%> (ø) ⬆️
pkg/inventory/elasticsearch.go 100% <100%> (+13.33%) ⬆️
pkg/inventory/ingress.go 100% <100%> (+13.33%) ⬆️
pkg/inventory/configmap.go 100% <100%> (+12.9%) ⬆️
pkg/inventory/daemonset.go 100% <100%> (+13.33%) ⬆️
pkg/inventory/service.go 100% <100%> (+12.5%) ⬆️
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3765a39...9a3b2e5. Read the comment docs.

Copy link
Contributor

@objectiser objectiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for finding this and submitting a PR.


// initK8sObjectMeta will set the required default settings to
// kubernetes objects metadata if is required.
func initK8sObjectMeta(obj metav1.Object) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move this to pkg/util/util.go?

Also would be good to have a test for this.

@slok
Copy link
Contributor Author

slok commented May 24, 2019

Ready for a new check!

…ethod to an utility method

Signed-off-by: Xabier Larrakoetxea <[email protected]>
@slok slok force-pushed the inventory-nil-maps branch from c557107 to 9a3b2e5 Compare May 24, 2019 07:01
@objectiser objectiser merged commit 3469391 into jaegertracing:master May 24, 2019
@objectiser
Copy link
Contributor

@slok Thanks!

@slok slok deleted the inventory-nil-maps branch May 24, 2019 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants