Build succeeded.

• gophercloud-unittest : SUCCESS in 3m 52s
• gophercloud-acceptance-test : SUCCESS in 2h 00m 05s
• gophercloud-acceptance-test-ironic : SUCCESS in 29m 51s

@notandy can you run gofmt -w ./openstack against the code?

@notandy can you run gofmt -w ./openstack against the code?

thx!

looks like the newly used github actions ci is failing... 🤕
@jtopjian Any chance to recheck?

Build succeeded.

• gophercloud-unittest : SUCCESS in 4m 06s
• gophercloud-acceptance-test : SUCCESS in 1h 39m 12s
• gophercloud-acceptance-test-ironic : SUCCESS in 29m 18s

looks like the newly used github actions ci is failing...

My guess is that the error always happened in Travis when acceptance tests were updated but it was just being ignored on failure. I'll see if I can figure out the issue.

@notandy Let's see if #2080 fixes the problem. Can you rebase off of master and do a force-push? Something like this should work:

$ git checkout master
$ git pull
$ git checkout fully-populated-load-balancer
$ git rebase -i master

# make no changes to the list of commits

$ git push -f sapcc fully-populated-load-balancer

Coverage increased (+0.002%) to 79.743% when pulling f61767d on sapcc:fully-populated-load-balancer into 766a46f on gophercloud:master.

@notandy Let's see if #2080 fixes the problem. Can you rebase off of master and do a force-push? Something like this should work:

$ git checkout master
$ git pull
$ git checkout fully-populated-load-balancer
$ git rebase -i master

# make no changes to the list of commits

$ git push -f sapcc fully-populated-load-balancer

thx, it worked. Since I needed to rebase anyway I incorporated the go fmt/import fixes into the inital commits, hope that's fine :)

Build failed.

• gophercloud-unittest : SUCCESS in 3m 51s
• gophercloud-acceptance-test : FAILURE in 19m 15s
• gophercloud-acceptance-test-ironic : SUCCESS in 31m 04s

recheck

Build succeeded.

• gophercloud-unittest : SUCCESS in 4m 14s
• gophercloud-acceptance-test : SUCCESS in 2h 26m 28s
• gophercloud-acceptance-test-ironic : SUCCESS in 47m 50s

@notandy Thank for working on this.

There's a lot going on here. Effectively, these changes reverse the logic of building the various load balancer resources. I understand how this all fits together to create a load balancer, but I'd like to know if these new fields are only applicable when creating a load balancer or if they are able to be used on their own.

For example, to create Rules, an L7 Policy first needs created. Then each rule is given the ID of the new Policy during the l7policies.CreateRule call. With the changes in this PR, a slice of CreateRuleOpts is able to be specified when creating the Policy and now Rules can be created when the Policy is created.

Is this only possible when creating a load balancer or can this be used when creating policies on their own? For example, is it now possible to do:

createOpts := l7policies.CreateOpts{
    Name:        "redirect-example.com",
    ListenerID:  "023f2e34-7806-443b-bfae-16c324569a3d",
    Action:      l7policies.ActionRedirectToURL,
    RedirectURL: "http://www.example.com",

    Rules:       []l7policies.CreateRuleOpts{
        ...
    },
}
l7policy, err := l7policies.Create(lbClient, createOpts).Extract()

Whichever the behaviour is, the inline documentation should be updated in more detail. For example, instead of:

// Create rules opts

This should be something like:

// Rules is a slice of CreateRuleOpts which allows a set of rules 
// to be created at the same time the policy is created.
//
// This is only possible to use when creating a fully populated 
// load balancer.

The new behaviour(s) should also be documented in the various doc.go files where appropriate, too.

Additionally, I noticed you've added fields to the various Update calls, too. Does this mean it's possible to update all of a load balancer's resources in one call? If so, the unit and acceptance tests for the load balancer should be updated to test the update functionality.

Additionally, are the individual resources able to be updated in other places? In other words, focusing on L7 Policies, is it possible to do a bulk update of a Policy's Rules in the l7policies.Update call?

Again, thanks for your work on this. Please let me know if you have any questions.

@jtopjian, thanks for the reply.

There's a lot going on here. Effectively, these changes reverse the logic of building the various load balancer resources. I understand how this all fits together to create a load balancer, but I'd like to know if these new fields are only applicable when creating a load balancer or if they are able to be used on their own.

As pointed out in the documentation (https://docs.openstack.org/api-ref/load-balancer/v2/?expanded=create-a-load-balancer-detail#creating-a-fully-populated-load-balancer), specifying related hierarchically related entities without their UUID is only applicable for POST (create) calls agains the /loabalancers endpoint.

The logic behind that resides in the _graph_create functions inside the API controller:

/octavia/octavia/api/v2/controllers $ git grep -p '_graph_create'
health_monitor.py=class HealthMonitorController(base.BaseController):
health_monitor.py:    def _graph_create(self, lock_session, hm_dict):
l7policy.py=class L7PolicyController(base.BaseController):
l7policy.py:    def _graph_create(self, lock_session, policy_dict):
l7policy.py:                l7rule.L7RuleController(db_policy.id)._graph_create(
l7rule.py=class L7RuleController(base.BaseController):
l7rule.py:    def _graph_create(self, lock_session, rule_dict):
listener.py=class ListenersController(base.BaseController):
listener.py:    def _graph_create(self, lock_session, listener_dict,
listener.py:            new_l7ps.append(l7policy.L7PolicyController()._graph_create(
load_balancer.py=class LoadBalancersController(base.BaseController):
load_balancer.py:                db_pools, db_lists = self._graph_create(
load_balancer.py:    def _graph_create(self, session, lock_session, db_lb, listeners, pools):
load_balancer.py:            new_pool = (pool.PoolsController()._graph_create(
load_balancer.py:            new_lists.append(listener.ListenersController()._graph_create(
member.py=class MemberController(base.BaseController):
member.py:    def _graph_create(self, lock_session, member_dict):
member.py=class MembersController(MemberController):
member.py:                created_member = self._graph_create(lock_session, m)
pool.py=class PoolsController(base.BaseController):
pool.py:    def _graph_create(self, session, lock_session, pool_dict):
pool.py:            new_hm = health_monitor.HealthMonitorController()._graph_create(
pool.py:                member.MembersController(db_pool.id)._graph_create(

The only call from API side that supports the _graph_create call (and all subsequent entities) is the post() call in load_balancer.py: https://github.com/openstack/octavia/blob/master/octavia/api/v2/controllers/load_balancer.py#L501

Thanks for the hint, I will add inline documentation that clarifies the constraints imposed by Octavia.

Additionally, I noticed you've added fields to the various Update calls, too. Does this mean it's possible to update all of a load balancer's resources in one call? If so, the unit and acceptance tests for the load balancer should be updated to test the update functionality.

Thanks again, it looks for me like a false assumption that this could be possible (since the fully-populated call looks like a declarative API, which is not the case).

For example for l7policies, according to upstream code, the PUT (update) hook sanitizes the input params and removes any l7rule references from the PUT body
https://github.com/openstack/octavia/blob/master/octavia/api/v2/controllers/l7policy.py#L220
https://github.com/openstack/octavia/blob/master/octavia/api/v2/types/l7policy.py#L108

and therefore only supports params as documented:
https://docs.openstack.org/api-ref/load-balancer/v2/?expanded=create-a-load-balancer-detail,update-a-l7-rule-detail,update-a-l7-policy-detail#id136

I will remove the wrong UpdateRuleOpts attributes so it matches the specification, thanks!

Additionally, are the individual resources able to be updated in other places? In other words, focusing on L7 Policies, is it possible to do a bulk update of a Policy's Rules in the l7policies.Update call?

Nope, not according to API or implementation, except of the already supported pool-members batch update.

Build succeeded.

• gophercloud-unittest : SUCCESS in 4m 06s
• gophercloud-acceptance-test : SUCCESS in 2h 33m 59s
• gophercloud-acceptance-test-ironic : SUCCESS in 50m 37s

@notandy Nice - thank you! Is this ready for review/merge?

@notandy Nice - thank you! Is this ready for review/merge?

Yes, please :)