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

Renew dbus connection #2862

Closed
wants to merge 3 commits into from
Closed

Renew dbus connection #2862

wants to merge 3 commits into from

Conversation

wzshiming
Copy link
Contributor

@wzshiming wzshiming commented Mar 18, 2021 •
edited
Loading

After the current dbus restarts, legacyManager will always return an error 'dbus: connection closed by user'
This PR fixes when dbus returns this error, try to reconnect to ensure that subsequent calls are normal
xref kubernetes/kubernetes#100328

Please review the changes, if there is no problem, I will to add a test.

@wzshiming wzshiming mentioned this pull request Mar 18, 2021
Closed
@wzshiming wzshiming force-pushed the fix/renew-dbus branch 2 times, most recently from dd7cc8f to 56ffd42 Compare March 18, 2021 16:05
cyphar
cyphar requested changes Mar 19, 2021
View reviewed changes
libcontainer/cgroups/systemd/common.go Outdated Show resolved Hide resolved
libcontainer/cgroups/systemd/common.go Show resolved Hide resolved
libcontainer/cgroups/systemd/common.go Outdated Show resolved Hide resolved
@wzshiming wzshiming requested review from cyphar and kolyshkin March 19, 2021 07:08
@wzshiming
Copy link
Contributor Author

@cyphar @kolyshkin
Thanks, updated.

@wzshiming wzshiming requested a review from kolyshkin March 24, 2021 06:41
@wzshiming wzshiming force-pushed the fix/renew-dbus branch 4 times, most recently from b9357d6 to 471ea99 Compare March 25, 2021 02:36
@wzshiming
Copy link
Contributor Author

wzshiming commented Mar 25, 2021 •
edited
Loading

@kolyshkin
Maybe I can also move Dbus-related functions to dbusConnManager as a method?

@wzshiming wzshiming requested a review from kolyshkin March 25, 2021 02:45
@wzshiming
Copy link
Contributor Author

wzshiming commented Apr 7, 2021 •
edited
Loading

Hi @kolyshkin, could you have time to take a look this PR.

xref kubernetes/kubernetes#100328 (comment)

@wzshiming wzshiming mentioned this pull request Apr 8, 2021
Closed
@wzshiming
Copy link
Contributor Author

PING @kolyshkin

@wzshiming wzshiming force-pushed the fix/renew-dbus branch 2 times, most recently from 4a2f7d6 to a4e640f Compare April 20, 2021 03:03
@wzshiming
Add dbus manager
ca4e572 
Signed-off-by: Shiming Zhang <[email protected]>
@wzshiming
Add renew dbus connection
514a26e 
Signed-off-by: Shiming Zhang <[email protected]>
@wzshiming
Privatize NewUserSystemDbus
0983ca1 
Signed-off-by: Shiming Zhang <[email protected]>
@wzshiming
Copy link
Contributor Author

How can I continue to advance this PR?

@kolyshkin kolyshkin mentioned this pull request Apr 27, 2021
Merged
@kolyshkin
Copy link
Contributor

@wzshiming I described the idea of retry in place above (#2862 (comment) and #2862 (comment)), and I guess my description was not clear enough.

I have took your patches and added retry-in-place on top, please review #2923

@wzshiming
Copy link
Contributor Author

@kolyshkin
Thank you for taking over

@wzshiming wzshiming closed this Apr 28, 2021
@wzshiming wzshiming deleted the fix/renew-dbus branch April 28, 2021 01:46
This was referenced Apr 29, 2021
Closed
Closed
Closed
Closed
@kolyshkin kolyshkin mentioned this pull request Aug 19, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cyphar cyphar Awaiting requested review from cyphar

@kolyshkin kolyshkin Awaiting requested review from kolyshkin

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wzshiming @kolyshkin @cyphar