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.

Sign up for GitHub

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

Jump to bottom

Add more user-friendly log messages on map creation #15629

Closed
aanm opened this issue Apr 9, 2021 · 8 comments · Fixed by #15832
Closed

Add more user-friendly log messages on map creation #15629

aanm opened this issue Apr 9, 2021 · 8 comments · Fixed by #15832
Assignees
@jibi
Labels
kind/bug This is a bug in the Cilium logic.
Milestone
1.10.0

Comments

@aanm
Copy link
Member

aanm commented Apr 9, 2021 

Apr 09 18:21:00 k8s2 cilium-agent[18719]: level=error msg="Error while opening/creating BPF maps" error="unable to create map: use pinned map cilium_metrics: expected flags 1, got 0" subsys=daemon
Apr 09 18:21:00 k8s2 cilium-agent[18719]: level=fatal msg="Error while creating daemon" error="unable to create map: use pinned map cilium_metrics: expected flags 1, got 0" subsys=daemon

From Joe:
There is a cilium_metrics map already with flags configured with the value 0, looks like on startup the daemon attempted to reuse the map but since the flags value was different from what it expected the flags to be (1), it took exception to this and failed out.
Fataling out here seems like a bit extreme, normally I would expect such maps just to be recreated with the new map flags and log a big angry warning
@aanm aanm added kind/bug This is a bug in the Cilium logic. release-blocker/1.10 labels Apr 9, 2021
@christarazi
Copy link
Member

If we recreate maps, then we lose the contents. In this case, it's the metrics map so not a big deal if it's gone, but other maps, this might be a more sensitive situation. So it might make sense to make the metrics map not fatal on this error, but others probably keep the fatal.

@aanm aanm added this to the 1.10.0 milestone Apr 16, 2021
@jibi jibi mentioned this issue Apr 20, 2021
Closed
@jibi jibi self-assigned this Apr 20, 2021
@pchaigno
Copy link
Member

When does this happen? On upgrade from v1.9 to master? Why didn't the K8sUpdates test catch it?

@aanm
Copy link
Member Author

aanm commented Apr 20, 2021

IIRC was when switching the preallocate-bpf-maps flag from true to false or vice-versa.

@jibi
Copy link
Member

jibi commented Apr 20, 2021

When does this happen? On upgrade from v1.9 to master? Why didn't the K8sUpdates test catch it?

There is small window of commits (after the switch to cilium/ebpf for the metrics map #14582 and before this fix #14853) where the metrics map would be created without the BPF_F_NO_PREALLOC flag, so I think that could explain why this happened

@pchaigno
Copy link
Member

IIRC was when switching the preallocate-bpf-maps flag from true to false or vice-versa.

If that's all there is to it, maybe we can just delete the map when it happens? I doubt it's worth a full map migration.

@jibi
Copy link
Member

jibi commented Apr 20, 2021

I doubt it's worth a full map migration.

Not sure what you are referring to 🤔

@pchaigno
Copy link
Member

I thought I saw that discussed as a potential solution last time we discussed it on Slack, but can't find it again 😞

@jibi
Copy link
Member

jibi commented Apr 20, 2021

Ah, I think I know what you are referring to now 👍 (different issue 🙂)

@jibi jibi mentioned this issue Apr 21, 2021
Merged
@jibi jibi mentioned this issue Apr 22, 2021
Merged
jibi added a commit that referenced this issue Apr 22, 2021
@jibi
ebpf: delete existing pinned map if incompatible with the spec
6ae9714 
This commit updates the OpenOrCreate() method of the ebpf package to
gracefully handle the case of a pinned map that is incompatible with the
map's spec passed to the method.

Rather than just returning the error returned by NewMapWithOptions(), we
now log a warning and try to delete and recreate the pinned map.

This mimics the behaviour of the old OpenOrCreate() method of the bpf
package.

Fixes: #15629

Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
jibi added a commit that referenced this issue Apr 28, 2021
@jibi
ebpf: delete existing pinned map if incompatible with the spec
400de83 
This commit updates the OpenOrCreate() method of the ebpf package to
gracefully handle the case of a pinned map that is incompatible with the
map's spec passed to the method.

Rather than just returning the error returned by NewMapWithOptions(), we
now log a warning and try to delete and recreate the pinned map.

This mimics the behaviour of the old OpenOrCreate() method of the bpf
package.

Fixes: #15629

Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
jibi added a commit that referenced this issue May 3, 2021
@jibi
ebpf: delete existing pinned map if incompatible with the spec
a5cdb90 
This commit updates the OpenOrCreate() method of the ebpf package to
gracefully handle the case of a pinned map that is incompatible with the
map's spec passed to the method.

Rather than just returning the error returned by NewMapWithOptions(), we
now log a warning and try to delete and recreate the pinned map.

This mimics the behaviour of the old OpenOrCreate() method of the bpf
package.

Fixes: #15629

Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
jibi added a commit that referenced this issue May 4, 2021
@jibi
ebpf: delete existing pinned map if incompatible with the spec
3d18bb2 
This commit updates the OpenOrCreate() method of the ebpf package to
gracefully handle the case of a pinned map that is incompatible with the
map's spec passed to the method.

Rather than just returning the error returned by NewMapWithOptions(), we
now log a warning and try to delete and recreate the pinned map.

This mimics the behaviour of the old OpenOrCreate() method of the bpf
package.

Fixes: #15629

Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
ti-mo pushed a commit that referenced this issue May 4, 2021
@jibi @ti-mo
ebpf: delete existing pinned map if incompatible with the spec
f56cc5c 
This commit updates the OpenOrCreate() method of the ebpf package to
gracefully handle the case of a pinned map that is incompatible with the
map's spec passed to the method.

Rather than just returning the error returned by NewMapWithOptions(), we
now log a warning and try to delete and recreate the pinned map.

This mimics the behaviour of the old OpenOrCreate() method of the bpf
package.

Fixes: #15629

Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
brb pushed a commit that referenced this issue May 7, 2021
@jibi @brb
ebpf: delete existing pinned map if incompatible with the spec
7673dbe 
[ upstream commit f56cc5c ]

This commit updates the OpenOrCreate() method of the ebpf package to
gracefully handle the case of a pinned map that is incompatible with the
map's spec passed to the method.

Rather than just returning the error returned by NewMapWithOptions(), we
now log a warning and try to delete and recreate the pinned map.

This mimics the behaviour of the old OpenOrCreate() method of the bpf
package.

Fixes: #15629

Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
Signed-off-by: Martynas Pumputis <m@lambda.lt>
aanm pushed a commit that referenced this issue May 11, 2021
@jibi @aanm
ebpf: delete existing pinned map if incompatible with the spec
b0ee122 
[ upstream commit f56cc5c ]

This commit updates the OpenOrCreate() method of the ebpf package to
gracefully handle the case of a pinned map that is incompatible with the
map's spec passed to the method.

Rather than just returning the error returned by NewMapWithOptions(), we
now log a warning and try to delete and recreate the pinned map.

This mimics the behaviour of the old OpenOrCreate() method of the bpf
package.

Fixes: #15629

Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
Signed-off-by: Martynas Pumputis <m@lambda.lt>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jibi jibi

Labels
kind/bug This is a bug in the Cilium logic.
Projects
None yet
Milestone
1.10.0
Development

Successfully merging a pull request may close this issue.

ebpf: delete existing pinned map if incompatible with the spec cilium/cilium
4 participants
@jibi @pchaigno @christarazi @aanm