changing custom-properties to ini_setting

[nikolas-hermanns]

There is no need to have the template of the whole custom.properties. It is enough to change only the lines wanted.

[nikolas-hermanns]

@dfarrell07 here is the pull request

[trozet]

This actually doesnt do anything anymore with new netvirt. We can remove the enable_l3 parameter. We should actually cut a stable/boron branch, and you can remove this on master.

[trozet]

Also consider using str2bool() for cheking values that can be yes or true

[nikolas-hermanns]

ah ok so odl will always be the l3 router now?

[trozet]

iirc this also does nothing anymore. Need to double check with Sam Hague.

[trozet]

I like the idea behind the change. Do you think we could implement a library to do configuration settings similar to puppet-openstacklib?

[nikolas-hermanns]

I think that is a good idea. do we have a list of possible configuration for odl so that we can catch most of the things?

Some things are configured in xml files. I have not yet found out how to configure xml files with puppet.

[dfarrell07]

I have not yet found out how to configure xml files with puppet.

Maybe the xmlfile Puppet module to abstract Augeas.

[nikolas-hermanns]

What do you think about that?

[trozet]

@nikolas-hermanns this is just failing now because there are leftover spec tests failing. Can you also remove the spec tests?

[dfarrell07]

@nikolas-hermanns this is just failing now because there are leftover spec tests failing. Can you also remove the spec tests?

+1. I wish there was a good way to amend someone else's patch with GitHub like you can with Gerrit, I'd remove them for you.

[dfarrell07]

I fixed the Beaker tests in #126, so we can use them to verify this change.

[dfarrell07]

This actually doesnt do anything anymore with new netvirt. We can remove the enable_l3 parameter. - @trozet

ah ok so odl will always be the l3 router now? - @nikolas-hermanns

So L3 is enabled by default in the new NetVirt? If so, we need to fix the docs.

Latest stable/boron (pre-SR3) etc/custom.properties:

# ovsdb can be configured with ml2 to perform l3 forwarding. The config below enables that functionality, which is
# disabled by default.
# ovsdb.l3.fwd.enabled=yes

Latest master (pre-Carbon) etc/custom.properties:

# ovsdb can be configured with ml2 to perform l3 forwarding. The config below enables that functionality, which is
# disabled by default.
# ovsdb.l3.fwd.enabled=yes

Do we still want this knob at all? Does new NetVirt respect it? Can you disable L3 with it, and is that useful enough to justify keeping the param?

[dfarrell07]

I have a finished version of this patch in #129. Beaker and rspec tests are passing.

[shague]

new netvirt doesn't look at any of the params in the custom.properties. There is no way to disable l3 in the new netvirt. I thought Tim wanted to keep the params around though so that both netvirt versions would be used?

[dfarrell07]

new netvirt doesn't look at any of the params in the custom.properties. There is no way to disable l3 in the new netvirt.

Awesome, very good to know, thanks @shague.

thought Tim wanted to keep the params around though so that both netvirt versions would be used?

Tim cut a stable/boron branch that will still have this param. I'm guessing he doesn't want it for Carbon, based on above. @trozet?

[trozet]

@dfarrell07 Correct. This patch should only go into master.

[dfarrell07]

@trozet Okay, cool. So I'll go ahead and merge #129 and abandon this.

Thanks all 👍