dfarrell07 / puppet-opendaylight

Moved: https://git.opendaylight.org/gerrit/gitweb?p=integration/packaging/puppet-opendaylight.git;a=tree
12 stars 15 forks source link

changing custom-properties to ini_setting #125

Closed nikolas-hermanns closed 7 years ago

nikolas-hermanns commented 7 years ago

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

nikolas-hermanns commented 7 years ago

@dfarrell07 here is the pull request

nikolas-hermanns commented 7 years ago

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 commented 7 years ago

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

Maybe the xmlfile Puppet module to abstract Augeas.

nikolas-hermanns commented 7 years ago

What do you think about that?

trozet commented 7 years ago

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

dfarrell07 commented 7 years ago

@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 commented 7 years ago

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

dfarrell07 commented 7 years ago

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 commented 7 years ago

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

shague commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

dfarrell07 commented 7 years ago

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

Thanks all :+1: