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

Support node[:dnsmasq][:enable_hostsfile] #8

Closed
wants to merge 1 commit into from
Closed

Support node[:dnsmasq][:enable_hostsfile] #8

wants to merge 1 commit into from

Conversation

philpennock
Copy link

In some environments, dnsmasq might be wanted but not the addition of
items to /etc/hosts based on dnsmasq. Support such use-cases by
letting node[:dnsmasq][:enable_hostsfile] be set false.

This is a variation of something carried locally, ported forward to the current dnsmasq code-base.

In some environments, dnsmasq might be wanted but not the addition of
items to `/etc/hosts` based on dnsmasq.  Support such use-cases by
letting `node[:dnsmasq][:enable_hostsfile]` be set false.
@chrisroberts
Copy link
Contributor

👍

@mdhelgen
Copy link

mdhelgen commented Feb 5, 2015

👍 we are running this recipe but aren't using /etc/hosts and this causes

[2015-02-04T19:37:28-06:00] ERROR: Failed to load data bag item: "dnsmasq" "managed_hosts"

in the chef logs. It's getting rescued, but causing some angst among people scanning their chef logs or automated tools reporting ERROR entries in CI.

Was going to suggest this same approach so that we could turn it off.


if(node[:dnsmasq][:enable_hostsfile])
include_recipe 'dnsmasq::manage_hostsfile'
end

This comment was marked as outdated.

This comment was marked as outdated.

@jtimberman
Copy link

I think manage_hostsfile might be a more clear attribute name than enable_hostsfile. Especially since it coincides with the recipe name.

@philpennock
Copy link
Author

The attribute name chosen is consistent with enable_dns and enable_dhcp in default.rb and I'm inclined to push back a little here and argue for consistency.

We no longer use dnsmasq in the environment for which I wrote this changed cookbook, so I can't test any changes to detect typo problems; this PR has been noted as useful as-is by others, but my employer no longer has a need for this improvement so I'm not going to argue for accepting the PR if there are changes needed to be accepted upstream.

Should I close this PR?

@jtimberman
Copy link

re, the include_recipe comment: I've made the inline conditional in this PR (#15), as those need to be moved above the service resource anyway. If that PR is accepted, then my comment would be in line with consistency here.

re, the attribute: I can dig it.

In #15, the main body of work is to update the test harness so changes can be tested easier. If you don't have time to work on contributing to this cookbook and want to close this PR, that's cool. I can incorporate it an updated one. I'm not a maintainer of this cookbook, but it does seem that perhaps @chrisroberts doesn't have time or interest either, since the last commit on master was Dec 23, 2013, and the last activity from him on this PR is Jan 24, 2014.

@philpennock
Copy link
Author

I was coming to close this, but then saw that the easiest UX for discovering what's already known and has fixes is seeing the open PRs; since this is a solution helping others, I'll leave it open for now. If you get progress in having your changes accepted and have a replacement ready for this one, then I'll close it.

@jtimberman
Copy link

Cool 😺

@damacus
Copy link
Member

damacus commented May 6, 2019

Closing due to inactivity.

If this is still an issue please reopen or open another issue. Alternatively drop by the #sous-chefs channel on the Chef Community Slack and we'll be happy to help!

Thanks,
Sous-Chefs

@damacus damacus closed this May 6, 2019
@lock
Copy link

lock bot commented May 5, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators May 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants