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
Extend cilium config
to expose all active configurations. Add subcommand cilium config get
to get configurations from CLI
#16519
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks it's going on the right track. I assume all fields had to be made public otherwise the reflect won't work?
Yes @aanm . All fields need to be public. Though there is a way we can read unexported fileds by using reflect's unsafe pointer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! A few minor comments below.
I've answered to your weekly report to discuss what we want to include in that PR before merging.
ce142a5
to
aa6f8bb
Compare
cilium config
to expose all active configuraitons. Add subcommand cilium config get
to get configurations from CLI
b2035f6
to
63ca380
Compare
3842db4
to
3615599
Compare
Note: All the daemon configurations have to be public for reflections to work Signed-off-by: Gaurav Genani <h3llix.pvt@gmail.com>
cilium config get <config_name> gives configuration value from the cli. conifg_name should be in kebab case. Signed-off-by: Gaurav Genani <h3llix.pvt@gmail.com>
Add -r flag to show read-only configurations while default cilium config show read-write configurations Signed-off-by: Gaurav Genani <h3llix.pvt@gmail.com>
test-me-please Job 'Cilium-PR-K8s-1.16-net-next' hit: #17060 (84.67% similarity) |
All tests are passing except for the one known flake above. All reviews are in except for Joe's but he said before that he was okay to merge without waiting for his review. I'm marking ready to merge. |
Fixes: #13913
For
cilium config get <config_name>
configuration name should be in kebab case. For exampleEncryptNode
, So we can get its value bycilium config get encrypt-node
. Add flag -r to display read-only configurations.Test-image :
docker.io/h3llix/cilium-dev:cilium-config
(if required)