Skip to content

Renew dbus connection#2862

Closed
wzshiming wants to merge 3 commits intoopencontainers:masterfrom
wzshiming:fix/renew-dbus
Closed

Renew dbus connection#2862
wzshiming wants to merge 3 commits intoopencontainers:masterfrom
wzshiming:fix/renew-dbus

Conversation

@wzshiming
Copy link
Copy Markdown
Contributor

@wzshiming wzshiming commented Mar 18, 2021

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 force-pushed the fix/renew-dbus branch 2 times, most recently from dd7cc8f to 56ffd42 Compare March 18, 2021 16:05
@wzshiming
Copy link
Copy Markdown
Contributor Author

@cyphar @kolyshkin
Thanks, updated.

@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
Copy Markdown
Contributor Author

wzshiming commented Mar 25, 2021

@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
Copy Markdown
Contributor Author

wzshiming commented Apr 7, 2021

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

xref kubernetes/kubernetes#100328 (comment)

@wzshiming
Copy link
Copy Markdown
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
Signed-off-by: Shiming Zhang <wzshiming@foxmail.com>
Signed-off-by: Shiming Zhang <wzshiming@foxmail.com>
Signed-off-by: Shiming Zhang <wzshiming@foxmail.com>
@wzshiming
Copy link
Copy Markdown
Contributor Author

How can I continue to advance this PR?

@kolyshkin
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

@kolyshkin
Thank you for taking over

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants