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

cli: add "cilium bpf config list" #26105

Merged
merged 3 commits into from Jun 15, 2023
Merged

cli: add "cilium bpf config list" #26105

merged 3 commits into from Jun 15, 2023

Conversation

mhofstetter
Copy link
Member

@mhofstetter mhofstetter commented Jun 12, 2023 •
edited

This PR introduces the possibility to list the content of the BPF map cilium_runtime_config via the cilium cli command cilium bpf config list.

In addition, these information are dumped during sysdump with the bugtool.

@mhofstetter mhofstetter added area/bugtool Impacts gathering of data for debugging purposes. area/cli Impacts the command line interface of any command in the repository. release-note/misc This PR makes changes that have no direct user impact. labels Jun 12, 2023
@mhofstetter mhofstetter marked this pull request as ready for review June 12, 2023 09:02
@mhofstetter mhofstetter requested review from a team as code owners June 12, 2023 09:02
@mhofstetter mhofstetter force-pushed the pr/mhofstetter/cli-bpf-config-map branch from 2500b1d to 9827b47 Compare June 12, 2023 09:39
mhofstetter
mhofstetter commented Jun 12, 2023
View reviewed changes
pkg/maps/configmap/config_map.go
@@ -32,14 +32,21 @@ const (
AgentLiveness
Copy link
Member Author

Choose a reason for hiding this comment

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

@dylandreimerink @jrajahalme it looks like the "marker" entry UsedEntries is unused and lost its meaning. When L2 Announcement has been introduced, a new entry has been added to the end (whereas the intention of the marker entry was to be the last entry).

should we delete the entry and move AgentLiveness to second position?

Copy link
Member

Choose a reason for hiding this comment

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

Moving AgentLiveness is alright with me, as long as we do it before the feature freeze. Moving after that could pose upgrade issues if the map is not recreated. I also have no idea what UsedEntries was supposed to be used for.

Copy link
Member Author

Choose a reason for hiding this comment

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

@dylandreimerink thanks - removed the constant (in agent & common.h)

@mhofstetter mhofstetter removed the request for review from aditighag June 14, 2023 14:54
@mhofstetter
Copy link
Member Author

mhofstetter commented Jun 14, 2023 •
edited

removed review-request for @aditighag - as @dylandreimerink covered sig-datapath. he was willing to do the (full) review, because L2Announcement that he introduced is also using the runtime config map.

@mhofstetter
cli: add "cilium bpf config list"
8e05ca3 
This commit introduces the possibility to list the content of the BPF
map "cilium_runtime_config" via the cilium cli command `cilium bpf
config list`.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
@mhofstetter
bugtool: dump runtime config map information
770c1ac 
This commit introduces that runtime config map related information
are dumped when executing the bugtool / sysdump.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
@mhofstetter mhofstetter force-pushed the pr/mhofstetter/cli-bpf-config-map branch from 9827b47 to 770c1ac Compare June 14, 2023 16:54
@mhofstetter
Copy link
Member Author

rebased to main to get rid of jenkins checks :)

@mhofstetter
config map: remove constant UsedEntries
1207580 
This commit removes the unused constant UsedEntries in the runtime
config map. Therefore, AgentLiveness gets moved to position 1.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
@mhofstetter
Copy link
Member Author

mhofstetter commented Jun 14, 2023 •
edited

removed unused constant UsedEntries and therefore moved AgentLiveness to index 1.

-> added release-blocker/1.14 label

@mhofstetter
Copy link
Member Author

/test

@mhofstetter mhofstetter added kind/enhancement This would improve or streamline existing functionality. release-blocker/1.14 This issue will prevent the release of the next version of Cilium. kind/cleanup This includes no functional changes. labels Jun 14, 2023
asauber
asauber approved these changes Jun 14, 2023
View reviewed changes
Copy link
Member

@asauber asauber left a comment

Choose a reason for hiding this comment

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

Looks great. The de-duplication of the index strings is nice

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 14, 2023
@tklauser tklauser merged commit f0f7d0c into cilium:main Jun 15, 2023
61 checks passed
@mhofstetter mhofstetter deleted the pr/mhofstetter/cli-bpf-config-map branch June 15, 2023 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dylandreimerink dylandreimerink dylandreimerink approved these changes

@asauber asauber asauber approved these changes

Assignees
No one assigned
Labels
area/bugtool Impacts gathering of data for debugging purposes. area/cli Impacts the command line interface of any command in the repository. kind/cleanup This includes no functional changes. kind/enhancement This would improve or streamline existing functionality. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-blocker/1.14 This issue will prevent the release of the next version of Cilium. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@mhofstetter @dylandreimerink @asauber @tklauser