Attach security group to VM - EOF error despite successful request handling at backend

[pschollbach]

Source:
/compute/v2/extensions/secgroups/requests.go

Method:
AddServer()

Problem:
Method returns EOF error despite the request being perfectly handled by Openstack backend. The attachment of security group succeeded (verified with OS CLI). Gophercloud should return nil instead.

Analysis:
The attachment (request) is done according
http://developer.openstack.org/api-ref/compute/?expanded=add-security-group-to-a-server-addsecuritygroup-action-detail
The http response code is 202 but there is absolutely no body returned (empty body). It looks like gophercloud would have expected {} as body (according test cases).
This will fail in /github.com/gophercloud/gophercloud/provider_client.go:

func (client *ProviderClient) Request(method, url string, options *RequestOpts) (*http.Response, error) {
...
	// Parse the response body as JSON, if requested to do so.
	if options.JSONResponse != nil {
		defer resp.Body.Close()
		if err := json.NewDecoder(resp.Body).Decode(options.JSONResponse); err != nil {

--> seems the json decoder returns EOF error, which is then forwarded to gophercloud consumers

Minor comment:
Shouldn't OpenStack return 204 No contents in first place? But then, defaultOkCodes for method == "POST" would need 204 too.

Question:
We can provide a PR if preferred ways of fixing this are indicated. Would some "empty body check" within provider_client.go might have side effects for other requests / response scenarios?! Any other ideas? Thanks!

[jtopjian]

@pschollbach I'm not sure how this issue ran under the radar for so long - I apologize about that.

I've seen EOF reported from time to time, but I've never been able to reproduce the issue myself. Do you have a specific scenario that would confidently trigger an EOF? Additionally, since this is an older issue, has the problem resolved on its own (whether by a different patch or a newer version of Go)?

[pschollbach]

@jtopjian
Don't mind the delay because likely you were busy with much higher-prioritized tasks.

Anyway, I have moved on to another company (completely different projects) and therefore have no access to this test environment + code base anymore. I'm afraid we need to rely on my rather "shaky" memories. :-/

I think it was:

POST /servers/{server_id}/action 

Adds a security group to a server.

Specify the addSecurityGroup action in the request body.
Normal response codes: 202

(from OS API spec.)

Furthemore the API specification states:

If successful, this method does not return content in the response body.

Likely it's straight-forward to re-produce the problem by simply adding a security group to a server. In other words:

gophercloud / openstack / compute / v2 / extensions / secgroups / requests.go 

// AddServer will associate a server and a security group, enforcing the 
// rules of the group on the server. 
 func AddServer(client *gophercloud.ServiceClient, serverID, groupName string) 

gophercloud/openstack/compute/v2/extensions/secgroups/requests.go

Line 160 in 368deee

// AddServer will associate a server and a security group, enforcing the

OpenStack likely would reply with 202 ACCEPTED and no body at all. Seems to be the correct response to me after all (please ignore my minor comment regarding 204 above).

You could consider removing this unit test line (not {} - body but no body instead).

gophercloud/openstack/compute/v2/extensions/secgroups/testing/fixtures.go

Line 306 in 368deee

fmt.Fprintf(w, `{}`)

I guess this would replicate the problem too.

I think I simply added an extra "EOF error check" into our code 9 months back. Said differently, we translated the EOF error from gophercloud into OK for this particular response handler. Therefore, this issue wasn`t blocking us.

Probably you know best... would it be a big risk to handle this case around this area?
https://github.com/gophercloud/gophercloud/blob/master/provider_client.go#L284
Some 2xx response code (ok = true) but having no / empty body might occur for other request scenarios too.

[jtopjian]

Thank you for the details.

Don't mind the delay because likely you were busy with much higher-prioritized tasks.

Actually this would have been a priority - I really have no idea how I missed this open issue until now!

Likely it's straight-forward to re-produce the problem by simply adding a security group to a server.

That's the problem - I haven't been able to reproduce this. Between the acceptance tests within Gophercloud, the acceptance tests in the Terraform project, and using Gophercloud/Terraform on a daily basis in production across an array of OpenStack releases, I personally haven't run into an EOF error. But people have reported it in the past.

Probably you know best... would it be a big risk to handle this case around this area?

I can't confidently answer that at the moment. I'd really like to be able to reproduce a situation that triggers an EOF to study it in more detail.

I think it might be best to leave this issue open. Maybe someone else will come across it and be able to provide more details.

[colin-nolan]

We have come across this issue. In our case it occurred with this code:

// Create Server
serverCreateOpts := servers.CreateOpts{
    ServiceClient: computeClient,
    Name:          name,
    FlavorName:    m.MasterNodeSize,
    ImageName:     m.OpenStackConfig.ImageName,
    UserData:      masterUserdata.Bytes(),
    Networks: []servers.Network{
        servers.Network{UUID: m.OpenStackConfig.NetworkID},
    },
    Metadata: map[string]string{"Name": m.Name, "Role": "master"},
}
masterServer, err := servers.Create(computeClient, serverCreateOpts).Extract()
if err != nil {
    return err
}

// Set security group
err = secgroups.AddServer(computeClient, masterServer.ID, m.OpenStackConfig.NodeSecurityGroupID).ExtractErr()
if err != nil {
    return err
}

The AddServer method results in a post to the correct OpenStack endpoint, which returns a 202 and an empty response body. The EOF error happens when the Request method attempts to parse the response body:
https://github.com/gophercloud/gophercloud/blob/master/provider_client.go#L346-L351

The workaround here is to set the security group in the server create options (which wasn't been done before as 2 bits of code were stapled together):
https://godoc.org/github.com/gophercloud/gophercloud/openstack/compute/v2/servers#CreateOpts

[jtopjian]

@colin-nolan well I'll be - thank you! In retrospect, your analysis matches @pschollbach, too.

I've opened #815 which should resolve this problem. If you are able to, can you test it?

[colin-nolan]

Done :). Thanks for your work!

Note: the example posted in #206 (comment) can also fail with "Resource not found" as the server create is async too - don't use secgroups.AddServer like this!

[jtopjian]

Thank you for the details and for testing, too :)

Right - the above example you gave won't work out of the box, but it was good enough for me to go off of. The acceptance test included in #815 waits until the server is in an "active" state before attaching the group which prevents the "resource not found" error.

[jrperritt]

closed by #815