Merge lp:~kbringard/nova/lp742578 into lp:~hudson-openstack/nova/trunk

[Kevin Bringard]

Add a flag to allow the user to specify a dnsmasq configuration file for nova-network to use when starting dnsmasq. Currently the command line option is set to "--config-fil=" with nothing specified. This branch will leave it as it is if the user does not specify a config file, but will utilize the specific file if they do.

If there is a conflict with the options specified on the command line and those in the configuration file, dnsmasq will use the config file (overriding the command line options) if the option is only allowed to be specified once, or it will use both if the option is allowed to be specified multiple times.

I have found this personally useful for adding things such as "domain=" to set the domain hostname on VMs as well as to more finely tune DNS delegation and upstream resolution.

[John Dewey (retr0h)]

Looks like the '--conf-file=' flag is always passed. Prior to your change it is just always empty, so your change looks reasonable.

Bit of information needed:

Add yourself to Authors and http://wiki.openstack.org/Contributors.

See the prerequisites section of the How to Contribute Guide:
  http://wiki.openstack.org/HowToContribute

review: Needs Information

[termie (termie)]

seems fine, I don't have a use case for it but I can see how people with very specific configs might want to mess with the config directly. If it is a common issue, however, it may make more sense to add that config option as a flag.

Anyway, a couple nits:

- no need for the extra newline before your flag def, and you need an extra space after it
- no need for the extra space before the command-line option

review: Needs Fixing

[Thierry Carrez (ttx)]

Feature is a bit late to the party, as we hit FeatureFreeze last Thursday.

If you want this into Cactus rather than wait for Diablo, could you provide a quick benefit vs. risk of regression rationale, then request a specific FFe review from me ?

review: Needs Information (ffe)

[Kevin Bringard (kbringard)]

John: Done and done.

Termie: I pushed a new version removing the extraneous whitespace you mentioned

Thierry: I'm OK with this not going out with Cactus; unless you know about some pent up demand for this feature, in which case I'm happy to write up a cost benefit analysis.

[Thierry Carrez (ttx)]

OK, please set it back to work in progress until Diablo opens for merging (April 14) so that it doesn't clog the review queue. We are trying to concentrate the nova-core review resources on the stuff that has to land in Cactus, like all those bugfixes :) Thanks.

[Kevin Bringard (kbringard)]

Reproposed for merging into diablo

[termie (termie)]

lgtm

review: Approve

[John Dewey (retr0h)]

A useful feature. Implementation looks good.

+lgtm

review: Approve

[Josh Kearney (jk0)]

lgtm

review: Approve

[OpenStack Infra (hudson-openstack)]

The attempt to merge lp:~kbringard/nova/lp742578 into lp:nova failed. Below is the output from the failed tests.

AccountsTest
    test_account_create OK
    test_account_delete OK
    test_account_update OK
    test_get_account OK
AdminAPITest
    test_admin_disabled OK
    test_admin_enabled OK
APITest
    test_exceptions_are_converted_to_faults OK
Test
    test_authorize_token OK
    test_authorize_user OK
    test_bad_token OK
    test_bad_user_bad_key OK
    test_bad_user_good_key OK
    test_no_user OK
    test_token_expiry OK
TestFunctional
    test_token_doesnotexist OK
    test_token_expiry OK
TestLimiter
    test_authorize_token OK
LimiterTest
    test_limiter_custom_max_limit OK
    test_limiter_limit_and_offset OK
    test_limiter_limit_medium OK
    test_limiter_limit_over_max OK
    test_limiter_limit_zero OK
    test_limiter_negative_limit OK
    test_limiter_negative_offset OK
    test_limiter_nothing OK
    test_limiter_offset_bad OK
    test_limiter_offset_blank OK
    test_limiter_offset_medium OK
    test_limiter_offset_over_max OK
    test_limiter_offset_zero OK
ActionExtensionTest
    test_extended_action OK
    test_invalid_action OK
    test_invalid_action_body OK
ExtensionControllerTest
    test_get_by_alias OK
    test_index OK
ExtensionManagerTest
    test_get_resources OK
ResourceExtensionTest
    test_get_resources OK
    test_get_resources_with_controller OK
    test_no_extension_present OK
ResponseExtensionTest
    test_get_resources_with_mgr OK
    test_get_resources_with_stub_mgr OK
TestFaults
    test_400_fault_json OK
    test_400_fault_xml OK
    tes...

[Kevin Bringard (kbringard)]

I'm updating the long line at the moment to be < 79 characters. I'll push a new version of the branch momentarily.

[lp:~kbringard/nova/lp742578]

895. By Kevin Bringard on 2011-05-02

Fixed 2 lines to allow pep8 check to pass

=== modified file 'Authors'
--- Authors	2011-05-02 19:21:40 +0000
+++ Authors	2011-05-02 19:24:31 +0000
@@ -46,6 +46,7 @@
 Justin Santa Barbara <[e-mail]>
 Kei Masumoto <[e-mail]>
 Ken Pepple <[e-mail]>
+Kevin Bringard <[e-mail]>
 Kevin L. Mitchell <[e-mail]>
 Koji Iida <[e-mail]>
 Lorin Hochstein <[e-mail]>

=== modified file 'nova/network/linux_net.py'
--- nova/network/linux_net.py	2011-04-08 01:44:41 +0000
+++ nova/network/linux_net.py	2011-05-02 19:24:31 +0000
@@ -56,13 +56,12 @@
                     'chain to add nova_input to')
 flags.DEFINE_integer('dhcp_lease_time', 120,
                      'Lifetime of a DHCP lease')
-
 flags.DEFINE_string('dns_server', None,
                     'if set, uses specific dns server for dnsmasq')
 flags.DEFINE_string('dmz_cidr', '10.128.0.0/24',
                     'dmz range that should be accepted')
-
-
+flags.DEFINE_string('dnsmasq_config_file', "",
+                    'Override the default dnsmasq settings with this file')
 binary_name = os.path.basename(inspect.stack()[-1][1])
 
 
@@ -678,7 +677,7 @@
     cmd = ['sudo', '-E', 'dnsmasq',
            '--strict-order',
            '--bind-interfaces',
-           '--conf-file=',
+           '--conf-file=%s' % FLAGS.dnsmasq_config_file,
            '--domain=%s' % FLAGS.dhcp_domain,
            '--pid-file=%s' % _dhcp_file(net['bridge'], 'pid'),
            '--listen-address=%s' % net['gateway'],