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

daemon: store Cilium's configuration in a file #16017

Merged
merged 1 commit into from May 11, 2021
Merged

daemon: store Cilium's configuration in a file #16017

merged 1 commit into from May 11, 2021

Conversation

aanm
Copy link
Member

@aanm aanm commented May 4, 2021

To make it easier debug or share configurations across users and
developers, Cilium will store its viper.Config as well as agent's
configuration in files under /var/run/cilium. It will store up to
the last 3 previous configurations.

Another use case for it will be to check the previous configuration run
by Cilium in case certain steps need to be executed, for example, to
clean up state left from a previous run that will not be cleaned up by a
new set of flags.

Signed-off-by: André Martins andre@cilium.io

Store the previous Cilium's configuration options in the host

@aanm aanm added release-note/minor This PR changes functionality that users may find relevant to operating Cilium. needs-backport/1.8 labels May 4, 2021
@aanm aanm requested a review from a team May 4, 2021 23:56
@aanm aanm requested a review from a team as a code owner May 4, 2021 23:56
@aanm aanm requested review from borkmann and nathanjsweet May 4, 2021 23:56
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.10.0-rc2 May 4, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.9.7 May 4, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.8.10 May 4, 2021
@aanm
Copy link
Member Author

aanm commented May 5, 2021

test-me-please

christarazi
christarazi requested changes May 5, 2021
View reviewed changes
Copy link
Member

@christarazi christarazi left a comment

Choose a reason for hiding this comment

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

Good idea

daemon/cmd/daemon_main.go Outdated Show resolved Hide resolved
pkg/option/config.go Outdated Show resolved Hide resolved
pkg/option/config.go Outdated
Comment on lines 3104 to 3106
"viper-config.yaml",
"viper-config-1.yaml",
"viper-config-2.yaml",
Copy link
Member

Choose a reason for hiding this comment

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

Since this is probably what Cilium was configured with, maybe we can have a name that's clearer than this to indicate that?

Copy link
Member

Choose a reason for hiding this comment

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

Also, should viper-config*.yaml rather be stored by date, so that we can diff on a timeline when agent was restarted with different configs?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's easier to leave without timestamps in the file names, however, the content of agent-config.json contains an entry with the creation time.

daemon/cmd/daemon_main.go Outdated Show resolved Hide resolved
@pchaigno pchaigno self-requested a review May 5, 2021 08:27
@borkmann
Copy link
Member

borkmann commented May 5, 2021

Please also add support for bugtool, so it will pick up all the config files from there.

@aanm aanm force-pushed the pr/store-config-locally branch from 1d5637f to 75df30d Compare May 5, 2021 18:35
@aanm
Copy link
Member Author

aanm commented May 5, 2021

test-me-please

@aanm aanm force-pushed the pr/store-config-locally branch from 75df30d to 8bfccb1 Compare May 5, 2021 18:37
@aanm
Copy link
Member Author

aanm commented May 5, 2021

test-me-please

@aanm
Copy link
Member Author

aanm commented May 5, 2021

@borkmann it should be picked by the bug tool automatically since it's going to be stored inside the state directory.

@aanm aanm requested a review from anfernee May 5, 2021 18:37
@aanm aanm requested a review from christarazi May 5, 2021 18:37
christarazi
christarazi reviewed May 5, 2021
View reviewed changes
Copy link
Member

@christarazi christarazi left a comment •
edited

Choose a reason for hiding this comment

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

I'm not sure I'm a fan of calling it Viper. That doesn't really mean much. What about "desired config" or "explicit config" or "initial config"? This would require renaming the "agent-config" to "agent-runtime-config" to make more sense. WDYT?

@aanm
Copy link
Member Author

aanm commented May 6, 2021

I'm not sure I'm a fan of calling it Viper. That doesn't really mean much. What about "desired config" or "explicit config" or "initial config"? This would require renaming the "agent-config" to "agent-runtime-config" to make more sense. WDYT?

@christarazi I think it should be prefixed with viper since in the end this is a file created by viper itself. I like the agent-runtime-config, so maybe agent-runtime-config.json and viper-agent-config.yaml?

@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.8 in 1.8.10 May 11, 2021
@aanm aanm mentioned this pull request May 11, 2021
Merged
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.10 in 1.10.0-rc2 May 12, 2021
@glibsm glibsm mentioned this pull request May 12, 2021
Merged
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.9 to Backport done to v1.9 in 1.9.7 May 12, 2021
@joestringer joestringer added this to Backport pending to v1.8 in 1.8.11 May 13, 2021
@joestringer joestringer removed this from Backport pending to v1.8 in 1.8.10 May 13, 2021
This was referenced May 13, 2021
Merged
Merged
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Backport done to v1.8 in 1.8.10 May 13, 2021
@joestringer joestringer removed this from Backport pending to v1.8 in 1.8.11 May 13, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.10 to Backport done to v1.10 in 1.10.0-rc2 May 13, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.10 to Backport done to v1.10 in 1.10.0-rc2 May 13, 2021
@aanm aanm mentioned this pull request May 17, 2021
Merged
gandro added a commit to cilium/cilium-cli that referenced this pull request Aug 8, 2022
@gandro
connectivity: Use agent-runtime-config.json for feature detection
f2ccd76 
Previously, we have been relying on the Cilium ConfigMap to detect the
presence of features. Relying on the ConfigMap has however some
downsides, as one has to know the default value of a certain feature if
the option is absent from the ConfigMap. To make things worse, this
default value is also version dependent in some cases.

This PR therefore instead tries to extract the feature status from the
agent-runtine-config.json, which is a JSON representation of the
`option.Config` struct used in Cilium Agent. It contains the actual
values determined at run-time, which is more robust than the ConfigMap
equivalent.

The agent-runtime-config.json is present since Cilium v1.10 and has been
backported to v1.8.10 and v1.9.7. See the PR for more details:
cilium/cilium#16017

A downside of this approach is that the `DaemonConfig` struct is not
guaranteed to be stable. If there are changes to it in the future, we
will likely have to maintain version-specific copies of the struct in
the Cilium-CLI.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
@gandro gandro mentioned this pull request Aug 8, 2022
Merged
gandro added a commit to cilium/cilium-cli that referenced this pull request Aug 8, 2022
@gandro
connectivity: Use agent-runtime-config.json for feature detection
22b6f98 
Previously, we have been relying on the Cilium ConfigMap to detect the
presence of features. Relying on the ConfigMap has however some
downsides, as one has to know the default value of a certain feature if
the option is absent from the ConfigMap. To make things worse, this
default value is also version dependent in some cases.

This PR therefore instead tries to extract the feature status from the
agent-runtine-config.json, which is a JSON representation of the
`option.Config` struct used in Cilium Agent. It contains the actual
values determined at run-time, which is more robust than the ConfigMap
equivalent.

The agent-runtime-config.json is present since Cilium v1.10 and has been
backported to v1.8.10 and v1.9.7. See the PR for more details:
cilium/cilium#16017

A downside of this approach is that the `DaemonConfig` struct is not
guaranteed to be stable. If there are changes to it in the future, we
will likely have to maintain version-specific copies of the struct in
the Cilium-CLI.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
gandro added a commit to cilium/cilium-cli that referenced this pull request Aug 8, 2022
@gandro
connectivity: Use agent-runtime-config.json for feature detection
8232fe0 
Previously, we have been relying on the Cilium ConfigMap to detect the
presence of features. Relying on the ConfigMap has however some
downsides, as one has to know the default value of a certain feature if
the option is absent from the ConfigMap. To make things worse, this
default value is also version dependent in some cases.

This PR therefore instead tries to extract the feature status from the
agent-runtime-config.json, which is a JSON representation of the
`option.Config` struct used in Cilium Agent. It contains the actual
values determined at run-time, which is more robust than the ConfigMap
equivalent.

The agent-runtime-config.json is present since Cilium v1.10 and has been
backported to v1.8.10 and v1.9.7. See the PR for more details:
cilium/cilium#16017

A downside of this approach is that the `DaemonConfig` struct is not
guaranteed to be stable. If there are changes to it in the future, we
will likely have to maintain version-specific copies of the struct in
the Cilium-CLI.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
tklauser pushed a commit to cilium/cilium-cli that referenced this pull request Aug 8, 2022
@gandro @tklauser
connectivity: Use agent-runtime-config.json for feature detection
b8de119 
Previously, we have been relying on the Cilium ConfigMap to detect the
presence of features. Relying on the ConfigMap has however some
downsides, as one has to know the default value of a certain feature if
the option is absent from the ConfigMap. To make things worse, this
default value is also version dependent in some cases.

This PR therefore instead tries to extract the feature status from the
agent-runtime-config.json, which is a JSON representation of the
`option.Config` struct used in Cilium Agent. It contains the actual
values determined at run-time, which is more robust than the ConfigMap
equivalent.

The agent-runtime-config.json is present since Cilium v1.10 and has been
backported to v1.8.10 and v1.9.7. See the PR for more details:
cilium/cilium#16017

A downside of this approach is that the `DaemonConfig` struct is not
guaranteed to be stable. If there are changes to it in the future, we
will likely have to maintain version-specific copies of the struct in
the Cilium-CLI.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
aditighag pushed a commit to aditighag/cilium-cli that referenced this pull request Apr 21, 2023
@gandro @aditighag
connectivity: Use agent-runtime-config.json for feature detection
34923be 
Previously, we have been relying on the Cilium ConfigMap to detect the
presence of features. Relying on the ConfigMap has however some
downsides, as one has to know the default value of a certain feature if
the option is absent from the ConfigMap. To make things worse, this
default value is also version dependent in some cases.

This PR therefore instead tries to extract the feature status from the
agent-runtime-config.json, which is a JSON representation of the
`option.Config` struct used in Cilium Agent. It contains the actual
values determined at run-time, which is more robust than the ConfigMap
equivalent.

The agent-runtime-config.json is present since Cilium v1.10 and has been
backported to v1.8.10 and v1.9.7. See the PR for more details:
cilium/cilium#16017

A downside of this approach is that the `DaemonConfig` struct is not
guaranteed to be stable. If there are changes to it in the future, we
will likely have to maintain version-specific copies of the struct in
the Cilium-CLI.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
michi-covalent pushed a commit to michi-covalent/cilium that referenced this pull request May 30, 2023
@gandro @michi-covalent
connectivity: Use agent-runtime-config.json for feature detection
7048ce5 
Previously, we have been relying on the Cilium ConfigMap to detect the
presence of features. Relying on the ConfigMap has however some
downsides, as one has to know the default value of a certain feature if
the option is absent from the ConfigMap. To make things worse, this
default value is also version dependent in some cases.

This PR therefore instead tries to extract the feature status from the
agent-runtime-config.json, which is a JSON representation of the
`option.Config` struct used in Cilium Agent. It contains the actual
values determined at run-time, which is more robust than the ConfigMap
equivalent.

The agent-runtime-config.json is present since Cilium v1.10 and has been
backported to v1.8.10 and v1.9.7. See the PR for more details:
cilium#16017

A downside of this approach is that the `DaemonConfig` struct is not
guaranteed to be stable. If there are changes to it in the future, we
will likely have to maintain version-specific copies of the struct in
the Cilium-CLI.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@borkmann borkmann borkmann left review comments

@joestringer joestringer joestringer approved these changes

@christarazi christarazi christarazi approved these changes

@nathanjsweet nathanjsweet Awaiting requested review from nathanjsweet

@pchaigno pchaigno Awaiting requested review from pchaigno

@anfernee anfernee Awaiting requested review from anfernee

Assignees

@anfernee anfernee

@nathanjsweet nathanjsweet

@pchaigno pchaigno

Labels
release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
No open projects
1.10.0-rc2
Backport done to v1.10
1.8.10
Backport done to v1.8
1.9.7
Backport done to v1.9
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@aanm @borkmann @anfernee @joestringer @christarazi @glibsm @nathanjsweet @pchaigno