Skip to content

Conversation

@Gal-Zaidman
Copy link

@Gal-Zaidman Gal-Zaidman commented Sep 6, 2020

We copy /var/run/NetworkManager/resolv.conf to /etc/resolv.conf for the case that /etc/resolv.conf is not present, but if it is present we override it.
On oVirt this causes the api-int to not be resolvable for a short period of time, causing failures and events.
This PR adds a check to override cp only if it is not present.

Notice that at the end of this script we replace it with a new file anyway but that is ok.

** Sent this fix to all the platforms that have similar logic to be consistent

Signed-off-by: Gal-Zaidman [email protected]

- What I did

Added a check to see if /etc/resolv.conf exist before overriding it

- Description for the changelog

Check if /etc/resolv.conf is present before overriding at the beginning of NetworkManager-resolv-prepender

@openshift-ci-robot openshift-ci-robot added bugzilla/severity-unspecified Referenced Bugzilla bug's severity is unspecified for the PR. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Sep 6, 2020
@openshift-ci-robot
Copy link
Contributor

@Gal-Zaidman: This pull request references Bugzilla bug 1876216, which is invalid:

  • expected the bug to target the "4.6.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

Bug 1876216: Check if /etc/resolv.conf is present before overriding

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.

@Gal-Zaidman
Copy link
Author

/cc @mandre

@Gal-Zaidman
Copy link
Author

/bugzilla refresh

@openshift-ci-robot openshift-ci-robot added bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/severity-unspecified Referenced Bugzilla bug's severity is unspecified for the PR. labels Sep 6, 2020
@openshift-ci-robot
Copy link
Contributor

@Gal-Zaidman: This pull request references Bugzilla bug 1876216, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.6.0) matches configured target release for branch (4.6.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
Details

In response to this:

/bugzilla refresh

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.

@openshift-ci-robot openshift-ci-robot removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Sep 6, 2020
@Gal-Zaidman
Copy link
Author

/test e2e-ovirt
/test e2e-openstack
/test e2e-vshpere

@openshift-ci-robot
Copy link
Contributor

@Gal-Zaidman: This pull request references Bugzilla bug 1876216, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.6.0) matches configured target release for branch (4.6.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
Details

In response to this:

Bug 1876216: Check if /etc/resolv.conf is present before overriding

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.

@Gal-Zaidman
Copy link
Author

/test e2e-ovirt

@Gal-Zaidman Gal-Zaidman changed the title Bug 1876216: Check if /etc/resolv.conf is present before overriding WIP: Bug 1876216: Check if /etc/resolv.conf is present before overriding Sep 6, 2020
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 6, 2020
@Gal-Zaidman
Copy link
Author

/test e2e-ovirt

@Gal-Zaidman
Copy link
Author

/test e2e-ovirt

We copy /var/run/NetworkManager/resolv.conf to /etc/resolv.conf for the case that /etc/resolv.conf is not present, but if it is present we override it. On oVirt is causes the api-int to not be resolveable for a short period of time, causing failures and events.
This PR adds a check to override cp only if it is not present.
Notice that at the end of this script we replace it with a new file anyway but that is ok.

Signed-off-by: Gal-Zaidman <[email protected]>
@Gal-Zaidman Gal-Zaidman changed the title WIP: Bug 1876216: Check if /etc/resolv.conf is present before overriding Bug 1876216: Check if /etc/resolv.conf is present before overriding Sep 7, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 7, 2020
@Gal-Zaidman
Copy link
Author

/assign @sinnykumari

@mandre
Copy link
Member

mandre commented Sep 7, 2020

/test e2e-ovirt
/test e2e-openstack
/test e2e-vshpere

@mandre
Copy link
Member

mandre commented Sep 7, 2020

All good for OpenStack 👍

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

20 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Sep 9, 2020

@Gal-Zaidman: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-vsphere 03c8800 link /test e2e-vsphere

Full PR test history. Your PR dashboard.

Details

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.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 40c3cc0 into openshift:master Sep 9, 2020
@openshift-ci-robot
Copy link
Contributor

@Gal-Zaidman: All pull requests linked via external trackers have merged:

Bugzilla bug 1876216 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1876216: Check if /etc/resolv.conf is present before overriding

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.

@hardys
Copy link

hardys commented Sep 15, 2020

It looks like this breaks some baremetal IPI deployments, since it's possible for resolv.conf to exist like:

[core@localhost ~]$ cat /etc/resolv.conf
# Generated by NetworkManager
search local <somedomain>

This won't work for external resolution though, so the podman then fails to pull the image and we reintroduce the bug fixed via #1765

So I think we either need to partially revert this (for the baremetal platform), or do some smarter check to still overwrite the resolv.conf when it only contains a local search domain and no upstream DNS?

/cc @celebdor @bcrochet @stbenjam

@yuqi-zhang
Copy link
Contributor

This appears to break rhel workers: https://bugzilla.redhat.com/show_bug.cgi?id=1879322

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants