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

Separate systemd dbus connection initialization from running check #2203

Merged
merged 2 commits into from
Mar 31, 2020

Conversation

mrunalp
Copy link
Contributor

@mrunalp mrunalp commented Jan 11, 2020

This allows us to catch and report any failures in initializing the systemd dbus connection.

@mrunalp mrunalp force-pushed the systemd_conn_cleanup branch 2 times, most recently from 610df46 to 33771c7 Compare January 14, 2020 16:30
@mrunalp
Copy link
Contributor Author

mrunalp commented Jan 14, 2020

@opencontainers/runc-maintainers ptal

@AkihiroSuda
Copy link
Member

@cyphar @dqminh @hqhq PTAL?

@hqhq
Copy link
Contributor

hqhq commented Feb 1, 2020

LGTM

Approved with PullApprove

@AkihiroSuda
Copy link
Member

@cyphar @dqminh mergeable?

@AkihiroSuda
Copy link
Member

Needs rebase. Also, could you consider melding #2241 into this?

@mrunalp
Copy link
Contributor Author

mrunalp commented Mar 4, 2020

@AkihiroSuda Yeah, I can merge #2241 into this.

@AkihiroSuda
Copy link
Member

ping

@AkihiroSuda
Copy link
Member

ping @mrunalp

@mrunalp
Copy link
Contributor Author

mrunalp commented Mar 27, 2020

okay, I am working on rebasing this with the other PR today.

@mrunalp mrunalp force-pushed the systemd_conn_cleanup branch from 33771c7 to 89351b2 Compare March 27, 2020 22:04
@mrunalp
Copy link
Contributor Author

mrunalp commented Mar 27, 2020

okay, rebased. @AkihiroSuda Do you feel strongly about returning error as you did in your PR for UseSystemd? I have split that into IsSystemdRunning() and a separate function to get the dbus connection so if there are any failures in getting the connection they get propagated and returned up the stack. Also, we compile systemd support on Linux by default so just returning false in apply_nosystemd is sufficient I think.

@AkihiroSuda
Copy link
Member

AkihiroSuda commented Mar 27, 2020

LGTM

Approved with PullApprove

mrunalp added 2 commits March 30, 2020 15:24
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
@mrunalp mrunalp force-pushed the systemd_conn_cleanup branch from 89351b2 to d05e572 Compare March 30, 2020 22:24
@AkihiroSuda
Copy link
Member

AkihiroSuda commented Mar 31, 2020

LGTM (still)

Approved with PullApprove

@mrunalp
Copy link
Contributor Author

mrunalp commented Mar 31, 2020

@opencontainers/runc-maintainers ptal

@crosbymichael
Copy link
Member

crosbymichael commented Mar 31, 2020

LGTM

Approved with PullApprove

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants