sap-oc / crowbar-openstack

Openstack deployment for Crowbar
3 stars 1 forks source link

neutron: lbaasv2: optional haproxy when using f5 #58

Closed matelakat closed 6 years ago

matelakat commented 6 years ago

When people use F5 they might still want to have haproxy as a non-default. This change will add a raw parameter add_haproxy_as_non_default to the neutron barclamp/f5. When user sets that value to true, HAProxy will be added as a non-default lbaasv2 driver to the neutron_lbaas.conf

matelakat commented 6 years ago

Configuration of multiple LBaaS v2 providers

vuntz commented 6 years ago

So if we only consider this branch, this approach would work for me. But:

I'm slightly concerned that we would not forward port this patch to upstream because it's slightly too hacky. But I think addressing the need is more important than this concern in the short term; we'll find a way to upstream this properly later on.

matelakat commented 6 years ago

So if we only consider this branch, this approach would work for me. But:

I would make the attribute not required in the schema (helps to not diverge from upstream) I would drop the migration I would possibly just make the attribute (in the cookbook) default to true (more debatable as this would definitely cause restart of neutron by default; on the other hand, I guess this would be wanted anyhow?) I'm slightly concerned that we would not forward port this patch to upstream because it's slightly too hacky. But I think addressing the need is more important than this concern in the short term; we'll find a way to upstream this properly later on.

Regarding to moving this to upstream, that is not the scope of this change. If we at any point have the same functionality implemented upstream in a different way, we can update this branch to be aligned with that solution / provide a straight upgrade path from this codebase to upstream.

I think leaving out the migration would just make it easier for this change to disappear, and would make this change really a hack.

Regarding to defaulting to having haproxy as a non-default driver, I am thinking that it would mean that only the deployment of this change would result in a configuration file change, which is not expected. If people install this patch as it is, no change will happen to the cloud (configuration files). When they change a value in the proposal, that's when the modifications kick off.

I'd like to leave it as it is and not address your concerns. Hopefully the explanations make it clear why I prefer this way. On the other hand if you still think we need to make those changes, please give it a -1 and I will address those immediately.

matelakat commented 6 years ago

This patch is not working as the original code is expecting that F5 and haproxy are mutually exclusive. More code changes needed here.