rackspace / gophercloud

A Go SDK for OpenStack. IN FEATURE FREEZE. See Issue #592
http://gophercloud.io
Other
456 stars 181 forks source link

X: Rewrite Server-related code to use mapstructure and extensions. #122

Closed sam-falvo closed 10 years ago

sam-falvo commented 10 years ago

In looking at the problem of extensibility, it looks like the entire Server structure needs to go away. OpenStack extensions can apply new members to ANY JSON structure, anywhere, at any time, as they see fit. Go's struct-based JSON encoding and decoding can't cope with this degree of flexibility.

So, Server needs to be changed to a map[string]interface{}.

Doing that, of course, breaks EVERYTHING.

So, I need to come up with:

1) An API to allow me to query extensions supported by your provider, and, 2) Rework all the APIs to use mapstructure, and, 3) Adjust all the acceptance tests, and, 4) ... I don't know from here. Maybe more, maybe not. Let's hope not.

This topic is for exploration only.

justinsb commented 10 years ago

What Protobuf does is to have a strongly-typed structured, but then have unknown fields in a map, so that they can round-trip. "Best of both worlds".

What I did when I wrote a Java binding (which didn't become the official binding, but let me know and I can probably dig up the code if wanted) was to do this, but also to have some strong-typing for "well-known" extensions. If (for example) security groups are an extension, you could ask the returned server instance for the well-known security groups extension object and it would effectively re-parse the unrecognized JSON against the extension schema object.

You would probably have to hack-up / re-create the Go JSON parser.

Giving up the strong typing in Go would make me sad.

sam-falvo commented 10 years ago

I'm mobile so apologies for spammy response.

I agree Protobuf is the ideal solution.

I'm planning on storing a raw decode as a map[string]interface{}, and encapsulating access to it behind interfaces. This will break everything, but it's at least more future proof. My only question now is how to implement this idiomatically.

jamiehannaford commented 10 years ago

@sam-falvo Why does the entire Server struct needs to be changed? Couldn't you maintain the original/master Server struct, but provide access to the dozens of different Compute extensions? The main Server struct would be immutable, but the individual extension file would contain all the structs required and would be accessed from the main Server struct:

func (server *Server) GetExtension(name string) (*ComputeExtensionInterface, error) {
   // check extension existence
   // return extension interface and/or map it to a Server property, like server.ext
}
sam-falvo commented 10 years ago

(1) OpenStack provides no hard definition that says, 'this is what a Server instance looks like.' (or any other entity for that matter.) I had merely inferred what I did because that's what all the documentation says it looks like, but as soon as you consider extensions, that assumption falls apart. You can already see in the existing Server structure evidence of this: the OS-EXT-DCF extension just introduces its fields willy-nilly into the structure. (2) The Go type-safe JSON decoder expects you to have a well-defined data structure. As you can see, these are completely at odds with each other.

I could define everything into a gigantic data structure with all fields from all extensions flattened into Server's namespace (sub-structures cannot be flattened as far as I can tell), but that's going to be as big a mess as the current directory layout is, and would waste a colossal amount of memory for deployments that don't use many of the extensions. There are 14 extensions presently defined on OpenStack's website, and that covers Nova alone; networking has its own (completely independent!!) extension mechanism too). This means I end up having to write custom unmarshallers for virtually everything, which will always need updating as extensions revise over time. Note that this covers only the Server structure! A wide number of other structures also receive "tweaks" from extensions, including but not limited to AddressSets, etc.

Alternatively, I could keep Server as-is, and strip out all the extensions, and that'd work for getting the least common denominator structure out of OpenStack. But, that now leaves all the extensions out in the cold. To support them, I must retain a full copy, byte-for-byte, of the JSON returned from OpenStack, just so I can reparse it in the context of any desired extensions as the client requests. I might as well just keep a server entity as raw JSON (e.g., map[string]interface{} in Go), and map it as-needed to structures as they're needed from the get-go. I'm going to have to write that code anyway, one way or another.

This becomes particularly poignant when you consider AddressSets, where it's been discussed in another ticket, that Public and Private are not the only pools supported, and indeed, it's perfectly legal OpenStack configuration to not have any pools with those names. So, in this case, Gophercloud's AddressSet structure is actually worse than wrong --- not only does it make a false assumption, but if the assumption breaks even a little, it actually wastes memory for each server you list. :(

I hope this explains my quandary and why I'm looking into a reengineering of the server API in Gophercloud.

sam-falvo commented 10 years ago

This is all happening with the v0.2.0 branch. I'm going to close this ticket as it's not been updated, and stake-holders seem happy with v0.2.0's progress to date. Feel free to re-open if I'm mistaken. Thanks!