Coverage increased (+0.01%) to 79.556% when pulling 6259bcd on alexeymyltsev:l3-agents-by-router into b1ee3a6 on gophercloud:master.

Build failed.

• gophercloud-unittest : SUCCESS in 3m 52s
• gophercloud-acceptance-test : FAILURE in 29m 39s
• gophercloud-acceptance-test-ironic : SUCCESS in 29m 40s

@jtopjian Could you help me?
A have a doubt about acceptance tests for ListL3Agents. For testing ListL3Agents needed create HA router, but it is impossible with existing type

type CreateOpts struct {
	Name                  string       `json:"name,omitempty"`
	Description           string       `json:"description,omitempty"`
	AdminStateUp          *bool        `json:"admin_state_up,omitempty"`
	Distributed           *bool        `json:"distributed,omitempty"`
	TenantID              string       `json:"tenant_id,omitempty"`
	ProjectID             string       `json:"project_id,omitempty"`
	GatewayInfo           *GatewayInfo `json:"external_gateway_info,omitempty"`
	AvailabilityZoneHints []string     `json:"availability_zone_hints,omitempty"`
}

For creating need add optional field ha
https://docs.openstack.org/api-ref/network/v2/index.html?expanded=create-router-detail#create-router
If add this field to CreateOpts than this value should be fielded, that wrong.
I can create separate type CreateHAOpts where this field will be mandatory for filling, but it is mean that will be needed create new function CreateHA and tests for all this things.
Could you advice how will be better add listing L3-agents for router?

Build failed.

• gophercloud-unittest : SUCCESS in 3m 12s
• gophercloud-acceptance-test : FAILURE in 41m 43s
• gophercloud-acceptance-test-ironic : SUCCESS in 31m 38s

Build failed.

• gophercloud-unittest : SUCCESS in 3m 52s
• gophercloud-acceptance-test : FAILURE in 1h 26m 54s
• gophercloud-acceptance-test-ironic : SUCCESS in 29m 29s

@jtopjian Could you advise what needed for merge this PR?

@alexeymyltsev

I'm sorry for the late reply.

If add this field to CreateOpts than this value should be fielded, that wrong.

I'm sorry, I don't understand what you mean here.

Source code:
https://github.com/openstack/neutron/blob/fefd9a6bb1e5b685d56c75e7a538b0dfc0a959ec/neutron/db/l3_hascheduler_db.py#L44-L55

Is this the only block of code that's needed to understand how a "List" call is implemented? Where are the fields in the L3Agent struct defined? Should this code be added to the agents package instead (https://github.com/gophercloud/gophercloud/tree/master/openstack/networking/v2/extensions/agents)?

@jtopjian

Is this the only block of code that's needed to understand how a "List" call is implemented? Where are the fields in the L3Agent struct defined?

Unfortunately I didn't find code in neutron where this struct is defined. As far as I understood neutron just checks fields in db and if needed provides answer with ha attribute.

Should this code be added to the agents package instead (https://github.com/gophercloud/gophercloud/tree/master/openstack/networking/v2/extensions/agents)?

I think this code should be in routers package because endpoint provides only L3-agents list, not all agents.

Is there something wrong with openlab/check or I should fix something in code?

I think this code should be in routers package because endpoint provides only L3-agents list, not all agents.

Good point. I agree.

Is there something wrong with openlab/check or I should fix something in code?

There's something wrong with OpenLab. Please ignore 🙂

There's something wrong with OpenLab. Please ignore 🙂

Then maybe PR can be merged ?

Then maybe PR can be merged ?

Not yet. We need to confirm the fields that you've defined in the structure. Where are you seeing that ha_state is a field that is being returned in the JSON response?

Looking for these things in the Neutron code can sometimes be difficult. Let me know if you need some help. It might take me a few days to get enough time to research it, but it's something I can help with.

Where are you seeing that ha_state is a field that is being returned in the JSON response?

This field described in doc and additionally I check it with my stand with Ussuri.

DEBUG: keystoneauth.session REQ: curl -g -i -X GET https://api-dev.cloud.lmru.tech:9696/v2.0/routers/efc06386-d9ff-40ab-b556-87ecf27fca68/l3-agents -H "Accept: application/json" -H "User-Agent: python-neutronclient" -H "X-Auth-Token: {SHA256}bd3d4c127f268bc4f2daefcfe20873d0938158fb6f2f626c92634f8ff05badb4"
DEBUG: keystoneauth.session RESP: [200] Content-Encoding: gzip Content-Length: 562 Content-Type: application/json Date: Thu, 24 Sep 2020 20:49:07 GMT Vary: Accept-Encoding X-Openstack-Request-Id: req-d84464c6-6b54-40b0-a653-1397da23d13d
DEBUG: keystoneauth.session RESP BODY: {"agents": [{"id": "4541cc6c-87bc-4cee-bad2-36ca78836c91", "agent_type": "L3 agent", "binary": "neutron-l3-agent", "topic": "l3_agent", "host": "msk3-dev-os-ctrl-03", "admin_state_up": true, "created_at": "2020-08-28 15:43:11.066068", "started_at": "2020-09-24 14:09:10.523083", "heartbeat_timestamp": "2020-09-24 20:48:41.411041", "description": null, "resources_synced": null, "availability_zone": "nova", "alive": true, "configurations": {"agent_mode": "legacy", "ex_gw_ports": 22, "floating_ips": 20, "handle_internal_only_routers": true, "interface_driver": "linuxbridge", "interfaces": 21, "log_agent_heartbeats": false, "routers": 22}, "resource_versions": {}, "ha_state": "active"}, {"id": "ddbf087c-e38f-4a73-bcb3-c38f2a719a03", "agent_type": "L3 agent", "binary": "neutron-l3-agent", "topic": "l3_agent", "host": "msk3-dev-os-ctrl-02", "admin_state_up": true, "created_at": "2020-08-28 15:43:15.216697", "started_at": "2020-09-24 14:09:23.650726", "heartbeat_timestamp": "2020-09-24 20:48:54.640741", "description": null, "resources_synced": null, "availability_zone": "nova", "alive": true, "configurations": {"agent_mode": "legacy", "ex_gw_ports": 22, "floating_ips": 20, "handle_internal_only_routers": true, "interface_driver": "linuxbridge", "interfaces": 21, "log_agent_heartbeats": false, "routers": 22}, "resource_versions": {}, "ha_state": "standby"}, {"id": "f98bd36e-847a-48e8-a670-f021f14d5cce", "agent_type": "L3 agent", "binary": "neutron-l3-agent", "topic": "l3_agent", "host": "msk3-dev-os-ctrl-01", "admin_state_up": true, "created_at": "2020-08-28 15:43:31.686402", "started_at": "2020-09-24 14:09:05.006501", "heartbeat_timestamp": "2020-09-24 20:49:06.220488", "description": null, "resources_synced": null, "availability_zone": "nova", "alive": true, "configurations": {"agent_mode": "legacy", "ex_gw_ports": 22, "floating_ips": 20, "handle_internal_only_routers": true, "interface_driver": "linuxbridge", "interfaces": 21, "log_agent_heartbeats": false, "routers": 22}, "resource_versions": {}, "ha_state": "standby"}]}

but if it mandatory I will try to find out ha_state in neutron code.

This field described in doc

Unfortunately we don't use API documentation to confirm fields. The API documentation might be out of date and we wouldn't know.

additionally I check it with my stand with Ussuri

But this is OK. The best source is from the Neutron service code directly, but if it's too hard to figure out, then debug output from the client is acceptable.

One other question: earlier, you had concerns about adding an ha field in the router CreateOpts. Adding this field would be a separate PR, but do you need help with this or had any questions about it?

I had concerns about creating HA routers for acceptance test. For implementation of creating HA router needed create separate PR and unfortunately I do not have enough time for it.

For this moment would be ok just check routers.ListL3Agents with non-HA router. This function return agents array with one agent and "ha_state": null, as you can see below.

DEBUG: keystoneauth.session REQ: curl -g -i -X GET https://api-dev.cloud.lmru.tech:9696/v2.0/routers/0de890e1-eb70-4a2f-bf1c-1622a0cb26ae/l3-agents -H "Accept: application/json" -H "User-Agent: python-neutronclient" -H "X-Auth-Token: {SHA256}8967337adbea186459feb3f70e6f773404600dc30f3b17f326dc3eb53548aaec"
DEBUG: keystoneauth.session RESP: [200] Content-Encoding: gzip Content-Length: 413 Content-Type: application/json Date: Fri, 25 Sep 2020 10:16:14 GMT Vary: Accept-Encoding X-Openstack-Request-Id: req-028df6f8-897d-4c7b-8711-3ee6c72d8e30
DEBUG: keystoneauth.session RESP BODY: {"agents": [{"id": "f98bd36e-847a-48e8-a670-f021f14d5cce", "agent_type": "L3 agent", "binary": "neutron-l3-agent", "topic": "l3_agent", "host": "msk3-dev-os-ctrl-01", "admin_state_up": true, "created_at": "2020-08-28 15:43:31.686402", "started_at": "2020-09-25 10:03:42.314935", "heartbeat_timestamp": "2020-09-25 10:16:12.339417", "description": null, "resources_synced": null, "availability_zone": "nova", "alive": true, "configurations": {"agent_mode": "legacy", "ex_gw_ports": 21, "floating_ips": 0, "handle_internal_only_routers": true, "interface_driver": "linuxbridge", "interfaces": 21, "log_agent_heartbeats": false, "routers": 24}, "resource_versions": {}, "ha_state": null}]}

For acceptance tests I have added routers.ListL3Agents to TestLayer3RouterCreateDelete

Build failed.

• gophercloud-unittest : SUCCESS in 4m 03s
• gophercloud-acceptance-test : FAILURE in 1h 53m 59s
• gophercloud-acceptance-test-ironic : SUCCESS in 28m 38s

Build failed.

• gophercloud-unittest : SUCCESS in 3m 50s
• gophercloud-acceptance-test : FAILURE in 1h 42m 29s
• gophercloud-acceptance-test-ironic : SUCCESS in 28m 01s

@jtopjian Could you tell me what is blocking this PR now?

Could you tell me what is blocking this PR now?

My real life schedule and me finding time to set aside and do a final review of everything 🙂

I'll take a look at this as soon as I can. I apologize for the inconvenience.

@alexeymyltsev Sorry again for the delay.

I looked everything over and, to help expedite this, made some changes directly instead of through a review.

The first change I made was to convert the ListL3Agents function to a paged function instead of a "standard" result request. Even though the results should always be a single page (which is accounted for), having this mirrors the existing agents functionality.

I then updated the documentation in doc.go accordingly.

The second change I made was to update the acceptance tests. It appears that OpenLab tests are now working and the last failure reported was a real failure - there was a typo in the test you submitted. In addition, this API call requires admin permissions, so I had to create a new test that ensures an admin user executes the test.

Let me know if you have any questions.

Build succeeded.

• gophercloud-unittest : SUCCESS in 3m 58s
• gophercloud-acceptance-test : SUCCESS in 1h 40m 22s
• gophercloud-acceptance-test-ironic : SUCCESS in 29m 08s

@jtopjian I only not sure that this needed in acceptance test for TestLayer3RouterAgents, looks like duplication with TestLayer3RouterCreateDelete.

	newName := tools.RandomString("TESTACC-", 8)
	newDescription := ""
	updateOpts := routers.UpdateOpts{
		Name:        newName,
		Description: &newDescription,
	}

	_, err = routers.Update(client, router.ID, updateOpts).Extract()
	th.AssertNoErr(t, err)

	_, err = routers.Get(client, router.ID).Extract()
	th.AssertNoErr(t, err)

Build succeeded.

• gophercloud-unittest : SUCCESS in 3m 57s
• gophercloud-acceptance-test : SUCCESS in 1h 51m 43s
• gophercloud-acceptance-test-ironic : SUCCESS in 29m 20s

I only not sure that this needed in acceptance test for TestLayer3RouterAgents, looks like duplication with TestLayer3RouterCreateDelete

I agree, but in my testing environment, no agents would be reported until I did an update on the router.

@alexeymyltsev Thank you for your work and patience with this.