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
UPSTREAM: 121124: Remove problematic Dbus restart test #1782
UPSTREAM: 121124: Remove problematic Dbus restart test #1782
Conversation
/sig node |
@kwilczynski: the contents of this pull request could not be automatically validated. The following commits could not be validated and must be approved by a top-level approver:
Comment |
@kwilczynski: The label(s) In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/test pull-kubernetes-node-e2e-containerd |
@kwilczynski: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/test pull-kubernetes-node-crio-e2e |
@kwilczynski: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/retest |
Commit message needs to be updated to be:
|
/retest-required |
@kwilczynski: the contents of this pull request could not be automatically validated. The following commits could not be validated and must be approved by a top-level approver:
Comment |
Hi @rphillips,
Done. |
/retest-required |
/approve |
/lgtm |
@kwilczynski: you cannot LGTM your own PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we wait for this change to land in the next rebase?
/lgtm I would leave to @rphillips , @soltysh for the approval !! |
Signed-off-by: Krzysztof Wilczyński <kwilczynski@redhat.com>
@kwilczynski: the contents of this pull request could not be automatically validated. The following commits could not be validated and must be approved by a top-level approver:
Comment |
/lgtm |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: kwilczynski, rphillips, sairameshv The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest-required |
@kwilczynski: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
What needs to be done here to help it through the finish line? @mrunalp, anything left I need to do? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see this test being executed nowhere in origin (our main test repository), nor anywhere here (https://github.com/openshift/origin/blob/master/test/extended/util/annotate/generated/zz_generated.annotations.go) so unless I see an explicit failure this is trying to fix, rather than proactively picking upstream PRs I don't see any value in bringing this down to our fork.
/hold
@soltysh, I see the following there (from test/extended/util/annotate/generated/zz_generated.annotations.go#L1426):
Which should include the test we want to remove? Or, would this not match the annotation? |
This will be included in 1.29 bump in #1810 |
What type of PR is this?
/kind cleanup
/kind failing-test
What this PR does / why we need it:
Restarting the Dbus broker service at runtime is generally not recommended or supported according to the feedback from the systemd maintainers.
Many systemd components (but also other software and services) that rely on Dbus wouldn't reconnect to a new Unix socket following the Dbus broker restart. This can lead to anything from authentication timeouts (since the systemd-logind and the polkitd services would be impacted) to the system generally broken following the Dbus broker restart.
Different software might handle this differently and choose to reconnect on a socket write or read timeout. However, systemd at large is not one of these.
Given that tests are nowadays commonly run on either Googe Container-optimised OS (COS) or Fedora CoreOS, the addition of the Dbus broker restart has unwelcome side effects on these platforms.
Thus, remove a problematic test that performs Dbus broker restart and, in doing so, fix several currently failing tests on platforms that do not handle Dbus service restart gracefully.
Which issue(s) this PR fixes:
Fixes kubernetes#121124
Special notes for your reviewer:
NONE
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: