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

Octavia: support L7 policy/rule operations #832

Closed
10 tasks done
lingxiankong opened this issue Mar 15, 2018 · 15 comments
Closed
10 tasks done

Octavia: support L7 policy/rule operations #832

lingxiankong opened this issue Mar 15, 2018 · 15 comments

Comments

@lingxiankong
Copy link
Contributor

lingxiankong commented Mar 15, 2018

Please refer to: https://docs.openstack.org/octavia/latest/user/guides/l7.html


  • l7 policy create
  • l7 policy list
  • l7 policy get
  • l7 policy update
  • l7 policy delete

  • l7 rule create
  • l7 rule list
  • l7 rule get
  • l7 rule update
  • l7 rule delete
@jtopjian
Copy link
Contributor

@lingxiankong Thanks for noting this :)

I see the L7 stuff defined here:

https://github.com/openstack/octavia/blob/master/octavia/api/v2/controllers/l7rule.py
https://github.com/openstack/octavia/blob/master/octavia/api/v2/controllers/l7policy.py

But I also see it in the Neutron LBaaS code:

https://github.com/openstack/neutron-lbaas/search?q=l7

Does this mean that L7 is supported in both Octavia and the "deprecated" Neutron LBaaS? Or does Octavia use the neutron-lbaas library? Put another way: Does one need to have Octavia deployed to use L7?

The reason I ask is because, given my comment earlier today (#831 (comment)), I'm wondering if:

  1. Similar to LBs, pools, monitors, etc, L7 is supported by both Neutron LBaaS and Octavia. In which case this implementation can go under the current lbaas_v2 package with no special conditions/checks.
  2. We should add it to lbaas_v2 but with the same kind of service check implemented in add cascade option to loadbalancers.Delete #831.
  3. Copy/paste lbaas_v2 to something like lbaas_v2_octavia or lbaas_v2_neutron or similar and add the L7 stuff to whichever is deemed the current LBaaS v2 API implementation.

I apologize if I'm making this more difficult than it really is.

@lingxiankong
Copy link
Contributor Author

hi, @jtopjian

AFAIK, the octavia l7 functionality is totally back-compatible with neutron-lbaas, octavia just has more features than the deprecated neutron-lbaas. I was a developer in octavia, btw.

@lingxiankong
Copy link
Contributor Author

@jtopjian if you don't mind, i can hand up to help with this issue.

@jtopjian
Copy link
Contributor

jtopjian commented Mar 15, 2018

if you don't mind, i can hand up to help with this issue.

We'd love to get your help on this, whether answering questions, implementing the code, or both 😄

AFAIK, the octavia l7 functionality is totally back-compatible with neutron-lbaas,

OK, that's good to know. So implementing this should be quite simple: it'll go in the lbaas_v2 package and either the network or load-balancer service client can use it.

octavia just has more features than the deprecated neutron-lbaas.

Right, and this is what I'm trying to sort out.

We've been able to introduce Octavia support into Gophercloud by re-using current Neutron LBaaS v2 package. This has worked very well so far.

But #831 introduced a feature that is only available in Octavia and not Neutron LBaaS.

Thinking long-term, I'm wondering if there will be enough Octavia-only features that warrant a dedicated octavia/lbaas_v2 package. Kind of like if Octavia-lbaasv2 was really a v3 API release.

But if L7 works with both neutron and octavia, then that decision doesn't need to be made here. :)

@lingxiankong
Copy link
Contributor Author

@jtopjian in my understanding, the octavia project started by using lbaas v2 api as a baseline and lbaas v2 api has been stopped development since then, so people who are still using lbaas v2 can always use the interface in lbaas_v2 package, and it's safe for us to add more features and new api supports in lbaas_v2 repo without breaking the current implemented apis. However, in case people don't know which features are not supported (or params) in lbaas v2 but in octavia, we should pay more attention to the docstring in the upcoming patches.

@jtopjian
Copy link
Contributor

and it's safe for us to add more features and new api supports in lbaas_v2 repo without breaking the current implemented apis

This is a kind of problem we've run into before. Rather than a v3 API, the Octavia enhancements sound similar to a microversion increase (if you think of the load-balancer client like a microversion increase). #441 lays out the difficulty in implementing microversion enhancements to existing APIs.

I'm sure that most Octavia-only features can be added to the existing lbaas_v2 package without any issue at all (as you say, we can simply document these additions), but there might be some future ones that will cause us to rethink the way they're implemented. These future cases are the ones I'm concerned about.

@jrperritt
Copy link
Contributor

jrperritt commented Mar 15, 2018

@jtopjian what do you think about having a top-level loadbalancer package, and delegating calls in it to lbass_v2 unless a change is needed?

Edit: Delegating calls may not get us much (I think someone looked into that for block storage). In that case, just copy/paste I guess.

@jtopjian
Copy link
Contributor

I see similarity between lbaasv2/Octavia and microversions and I think it'll be interesting to keep an eye on this to see if it presents a solution we haven't thought of. I'm kind of hoping some serendipitous solution pops up that we're all in awe over :)

Until then, I think we can proceed as normal and just be mindful.

@jrperritt
Copy link
Contributor

This seems different to me. It sounds like microversions are [potentially-backwards-incompatible] changes to a service. Octavia isn't a part of the networking service; it's a service of its own. It seems odd to call a subpackage of networking to call load balancer operations when a dedicated service exists for it.

@jtopjian
Copy link
Contributor

I agree. The reason I am comparing it to a microversion is because I'm anticipating a backwards incompatible change (and by backwards incompatible I mean a more Go-ish breaking change such as the removal of a request body parameter or something like that). So at the moment the two situations are a bit different. My spidey sense is being cautious, though.

It seems odd to call a subpackage of networking to call load balancer operations when a dedicated service exists for it.

Agreed. On one hand we get a big win by piggybacking off of the existing lbaas_v2 package, but yes, this could/should be a first-class service peer of networking.

@jtopjian
Copy link
Contributor

It seems odd to call a subpackage of networking to call load balancer operations when a dedicated service exists for it.

I should also mention that if we did make a new first-class loadbalancer service directory/package, then problem solved. Octavia-only features would be added there and missing LBaaS v2 features would go in both networking/v2/extensions/lbaas_v2 and loadbalancer/lbaas_v2.

The problems I'm anticipating all stem from implementing the LBaaS v2 API in a single place.

@jrperritt
Copy link
Contributor

Right, I'm suggesting a new top-level loadbalancer package to be implemented as you've described.

@jtopjian
Copy link
Contributor

Sounds like loadbalancer/v2/lbaas (or whichever directory structure is best) it is then 😄

lingxiankong added a commit to lingxiankong/gophercloud that referenced this issue Mar 16, 2018
For gophercloud#832

L7policy functionality in Octavia is backward compatible with
Neutron-LBaaS.

Octavia L7 policy create API:
https://developer.openstack.org/api-ref/load-balancer/v2/index.html#create-an-l7-policy
lingxiankong added a commit to lingxiankong/gophercloud that referenced this issue Mar 16, 2018
For gophercloud#832

L7policy functionality in Octavia is backward compatible with
Neutron-LBaaS.

Octavia L7 policy create API:
https://developer.openstack.org/api-ref/load-balancer/v2/index.html#create-an-l7-policy
lingxiankong added a commit to lingxiankong/gophercloud that referenced this issue Mar 17, 2018
For gophercloud#832

L7policy functionality in Octavia is backward compatible with
Neutron-LBaaS.

Octavia L7 policy create API:
https://developer.openstack.org/api-ref/load-balancer/v2/index.html#create-an-l7-policy
lingxiankong added a commit to lingxiankong/gophercloud that referenced this issue Mar 17, 2018
For gophercloud#832

L7policy functionality in Octavia is backward compatible with
Neutron-LBaaS.

Octavia L7 policy create API:
https://developer.openstack.org/api-ref/load-balancer/v2/index.html#create-an-l7-policy
lingxiankong added a commit to lingxiankong/gophercloud that referenced this issue Mar 18, 2018
lingxiankong added a commit to lingxiankong/gophercloud that referenced this issue Apr 3, 2018
lingxiankong added a commit to lingxiankong/gophercloud that referenced this issue Apr 3, 2018
jtopjian pushed a commit that referenced this issue Apr 4, 2018
* LBaaS v2 l7 policy support [Part 1]: create l7policy

For #832

L7policy functionality in Octavia is backward compatible with
Neutron-LBaaS.

Octavia L7 policy create API implementation:
https://github.com/openstack/octavia/blob/master/octavia/api/v2/controllers/l7policy.py#L140
Neutron-LBaaS L7 policy create API implementation:
https://github.com/openstack/neutron-lbaas/blob/8e8a6c47c7d38fa7b61850ff9ea2cf130718ded3/neutron_lbaas/services/loadbalancer/plugin.py#L913
Octavia L7 policy create API doc:
https://developer.openstack.org/api-ref/load-balancer/v2/index.html#create-an-l7-policy

* Use original type for the response fields

* Change to use lbClient for octavia acceptance test
jtopjian pushed a commit that referenced this issue Apr 10, 2018
jtopjian pushed a commit that referenced this issue Apr 12, 2018
* Octavia l7 rule support - part 1: create

For #832

Octavia l7 rule create API implementation:
https://github.com/openstack/octavia/blob/06bf5c58d5845f684fcaf933605ed112586eefc3/octavia/api/v2/controllers/l7rule.py#L146

* Use project_id instead of tenant_id
jtopjian pushed a commit that referenced this issue Apr 17, 2018
* Octavia l7 rule support - part 5(final): update

For #832

Octavia l7 rule PUT API implementation:

- https://github.com/openstack/octavia/blob/69a45c254be854dbdbff946dbe2564711cdecec3/octavia/api/v2/controllers/l7rule.py#L189

* Catch error when constructing update request body
@lingxiankong
Copy link
Contributor Author

hi, @jtopjian this issue could be closed :-) I will submit follow up patches for improvement or potential bugfix, thanks a lot for your review so far!

@jtopjian
Copy link
Contributor

Sounds good!

As well, thank you for your work implementing this -- it's very much appreciated!

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants