Skip to content
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

plugin/host: don't append the names #3045

Merged
merged 1 commit into from
Jul 25, 2019
Merged

plugin/host: don't append the names #3045

merged 1 commit into from
Jul 25, 2019

Conversation

miekg
Copy link
Member

@miekg miekg commented Jul 25, 2019

The host plugin kept on adding entries instead of overwriting.

A bunch of other cleanup as well. Use functions defined in the plugin
package, don't re-parse strings if you don't have to and use To4() to
check the family for IP addresses.

Fixes: #3014

@corbot corbot bot requested a review from johnbelamaric July 25, 2019 06:57
@corbot
Copy link

corbot bot commented Jul 25, 2019

Thank you for your contribution. I've just checked the OWNERS files to find a suitable reviewer. This search was successful and I've asked johnbelamaric (via plugin/hosts/OWNERS) for a review.

If you have questions or suggestions for this bot, please file an issue against the miekg/dreck repository.

The bot understands the commands that are listed here.

@miekg
Copy link
Member Author

miekg commented Jul 25, 2019

ok, the host unit test don't capture this test failure

The host plugin kept on adding entries instead of overwriting. Split the
inline cache off from the /etc/hosts file cache and clear /etc/hosts
file cache and re-parsing.

A bunch of other cleanup as well. Use functions defined in the plugin
package, don't re-parse strings if you don't have to and use To4() to
check the family for IP addresses. Fix all test cases a carried entries
are always fqdn-ed. Various smaller cleanup in unnessacry constants.

Fixes: #3014

Signed-off-by: Miek Gieben <miek@miek.nl>
@codecov-io
Copy link

Codecov Report

Merging #3045 into master will increase coverage by 0.04%.
The diff coverage is 88.52%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3045      +/-   ##
==========================================
+ Coverage    55.9%   55.94%   +0.04%     
==========================================
  Files         205      205              
  Lines       10116    10105      -11     
==========================================
- Hits         5655     5653       -2     
+ Misses       4046     4041       -5     
+ Partials      415      411       -4
Impacted Files Coverage Δ
plugin/hosts/hosts.go 77.14% <100%> (-0.94%) ⬇️
plugin/hosts/setup.go 46.22% <33.33%> (ø) ⬆️
plugin/hosts/hostsfile.go 67.25% <89.13%> (+5.27%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a41b9a...207a2e3. Read the comment docs.

Copy link
Member

@yongtang yongtang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks for the fix!

@yongtang yongtang merged commit 89fa9bc into master Jul 25, 2019
@corbot corbot bot deleted the host-fix branch July 25, 2019 18:53
russellb pushed a commit to openshift-kni/coredns that referenced this pull request Jan 17, 2020
…s#3045)

The host plugin kept on adding entries instead of overwriting. Split the
inline cache off from the /etc/hosts file cache and clear /etc/hosts
file cache and re-parsing.

A bunch of other cleanup as well. Use functions defined in the plugin
package, don't re-parse strings if you don't have to and use To4() to
check the family for IP addresses. Fix all test cases a carried entries
are always fqdn-ed. Various smaller cleanup in unnessacry constants.

Fixes: coredns#3014

Signed-off-by: Miek Gieben <miek@miek.nl>
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.

plugin/hosts: Hostsfile.parse unexpected append old result
4 participants