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

daemon: Create RuntimePath if not equal to StateDir #15711

Merged
merged 1 commit into from
Apr 16, 2021
Merged

Conversation

oblazek
Copy link
Contributor

@oblazek oblazek commented Apr 15, 2021

With this change, state-dir can be set to anything and code makes sure that /var/run/cilium is always present with correct rights since /var/run is clean after reboot.

PidfilePath is set here https://github.com/cilium/cilium/blob/master/daemon/cmd/daemon_main.go#L1143
always set to RuntimePath + "/cilium.pid". By default StateDir is set to defaults.RuntimePath == /var/run/cilium but when changed to something else, /var/run/cilium won't be created as in here:
https://github.com/cilium/cilium/blob/master/daemon/cmd/daemon_main.go#L1119 because https://github.com/cilium/cilium/blob/master/pkg/option/config.go#L2449.

This change fixes an issue if StateDir is specified and is
different from defaults.RuntimePath which is used to store
pidfile in.

When host is rebooted /var/run is clean. In case StateDir
is set to something else than "/var/run/cilium" the daemon
fails to create pidfile under "/var/run/cilium" path since
that is not created:

level=fatal msg="Failed to create Pidfile" error="open
/var/run/cilium/cilium.pid: no such file or directory"
file-path=/var/run/cilium/cilium.pid subsys=daemon

Signed-off-by: Ondrej Blazek <ondra.blazkuj@gmail.com>
@oblazek oblazek requested a review from a team April 15, 2021 13:26
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Apr 15, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.10.0 Apr 15, 2021
@oblazek oblazek requested a review from jibi April 15, 2021 13:26
Copy link
Member

@aanm aanm left a comment

Choose a reason for hiding this comment

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

Nice catch. Thank you!

@aanm aanm added the release-note/misc This PR makes changes that have no direct user impact. label Apr 15, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Apr 15, 2021
@aanm
Copy link
Member

aanm commented Apr 15, 2021

test-me-please

1 similar comment
@jibi
Copy link
Member

jibi commented Apr 16, 2021

test-me-please

@aanm
Copy link
Member

aanm commented Apr 16, 2021

hit #15620, merging

@aanm aanm merged commit 6149fa1 into cilium:master Apr 16, 2021
1.10.0 automation moved this from In progress to Done Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants