Bug 2039541: Fix duplicate prepender entries in resolv.conf#2908
Bug 2039541: Fix duplicate prepender entries in resolv.conf#2908openshift-merge-robot merged 1 commit intoopenshift:masterfrom
Conversation
In openshift#2835, we changed how the prepender finds the location to insert the prepended nameserver. Unfortunately, this caused it to prepend the nameserver before each existing nameserver line, which results in duplicate entries for the local nameserver. For example: search ostest.test.metalkube.org nameserver 192.168.111.23 nameserver 8.8.8.8 nameserver 192.168.111.23 This is mildly problematic since it pushes the second nameserver out of the list of 3 that are allowed. In general this probably won't cause problems because only the local nameserver is actually used, but it looks weird and will almost certainly result in a bug report from customers at some point. The replacement sed line is stolen from [0] and is specific to GNU sed, but that shouldn't be a problem for us since we only use that version of sed. 0: https://stackoverflow.com/questions/9970124/sed-to-insert-on-first-match-only
|
/cc @mandre |
|
The formatting of the description is a bit weird. The last nameserver that shows up as a title is actually commented out. |
mandre
left a comment
mandre
left a comment
There was a problem hiding this comment.
Tried it locally, does what it's supposed to do :)
/lgtm
|
@cybertron do you have a BZ you can reference? |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cgwalters, cybertron, mandre The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
cybertron
changed the title
openshift-ci
Bot
added
the
bugzilla/severity-high
|
@cybertron: This pull request references Bugzilla bug 2039541, which is invalid:
Comment DetailsIn 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. |
openshift-ci
Bot
added
the
bugzilla/invalid-bug
|
/bugzilla refresh |
|
@cybertron: This pull request references Bugzilla bug 2039541, 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
No GitHub users were found matching the public email listed for the QA contact in Bugzilla (vvoronko@redhat.com), skipping review request. DetailsIn 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. |
openshift-ci
Bot
added
bugzilla/valid-bug
Oops, thanks for the reminder. I opened one but forgot to add it here. |
|
@cybertron: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
4 similar comments
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
@cybertron: All pull requests linked via external trackers have merged: Bugzilla bug 2039541 has been moved to the MODIFIED state. DetailsIn 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.9 |
|
@creydr: #2908 failed to apply on top of branch "release-4.9": DetailsIn 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. |
7 participants
In #2835, we changed how the prepender finds the location to insert
the prepended nameserver. Unfortunately, this caused it to prepend
the nameserver before each existing nameserver line, which results
in duplicate entries for the local nameserver. For example:
search ostest.test.metalkube.org
nameserver 192.168.111.23
nameserver 8.8.8.8
nameserver 192.168.111.23
nameserver 192.168.111.1
This is mildly problematic since it pushes the second nameserver out
of the list of 3 that are allowed. In general this probably won't
cause problems because only the local nameserver is actually used,
but it looks weird and will almost certainly result in a bug report
from customers at some point.
The replacement sed line is stolen from [0] and is specific to GNU
sed, but that shouldn't be a problem for us since we only use that
version of sed.
0: https://stackoverflow.com/questions/9970124/sed-to-insert-on-first-match-only
- What I did
- How to verify it
- Description for the changelog
Fix resolv-prepender logic to avoid duplicate entries.