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
OCPBUGS-16733: on-prem: run resolv-prepender on NM reapply event #3827
OCPBUGS-16733: on-prem: run resolv-prepender on NM reapply event #3827
Conversation
@mkowalski: This pull request references Jira Issue OCPBUGS-16733, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
/cherry-pick release-4.13 |
@mkowalski: once the present PR merges, I will cherry-pick it on top of release-4.13 in a new PR and assign it to you. 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. |
@mkowalski: This pull request references Jira Issue OCPBUGS-16733, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: 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 ? |
@mkowalski: The following commands are available to trigger required jobs:
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 e2e-metal-ipi |
/override ci/prow/e2e-aws-ovn-upgrade This is on-prem job and e2e-metal-ipi did pass |
@mkowalski: mkowalski unauthorized: /override is restricted to Repo administrators, approvers in top level OWNERS file, and the following github teams:openshift: openshift-release-oversight. 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. |
>&2 echo "NM resolv-prepender triggered by ${IFACE} ${STATUS}." | ||
if ! timeout 30s bash -c resolv_prepender; then | ||
if ! timeout 20s bash -c resolv_prepender; then |
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 wonder if we need to do this. Can you get both an up and reapply status in the same activation? It seems like those should be mutually exclusive.
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.
Yeah this make sense, no need to modify this timeout here. I just rewritten the comment so that it's clear why numbers don't match (i.e. 90 divided by # of statuses)
With this change we are adding NetworkManager's "reapply" event to the list of events that trigger resolv-prepender. As per documentation[1], reapply attempts to update the configuration of a device without deactivating it what is a valid scenario for us to trigger the script. [1] https://developer-old.gnome.org/NetworkManager/stable/gdbus-org.freedesktop.NetworkManager.Device.html#gdbus-method-org-freedesktop-NetworkManager-Device.Reapply Fixes: OCPBUGS-16733
4eb7a54
to
48f157a
Compare
/test e2e-metal-ipi |
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.
/lgtm
/retest-required Neither of those jobs are affected by this change. |
/assign @jkyros |
/cc @sinnykumari |
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.
LGTM, have been reviewed by on-prem team
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cybertron, mkowalski, sinnykumari The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@mkowalski: 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. |
@mkowalski: Jira Issue OCPBUGS-16733: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-16733 has been moved to the MODIFIED state. 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. |
@mkowalski: new pull request created: #3880 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. |
With this change we are adding NetworkManager's "reapply" event to the list of events that trigger resolv-prepender. As per documentation[1], reapply attempts to update the configuration of a device without deactivating it what is a valid scenario for us to trigger the script.
[1] https://developer-old.gnome.org/NetworkManager/stable/gdbus-org.freedesktop.NetworkManager.Device.html#gdbus-method-org-freedesktop-NetworkManager-Device.Reapply
Fixes: OCPBUGS-16733