Skip to content
This repository was archived by the owner on Aug 1, 2023. It is now read-only.

Boot from volume#259

Merged
smashwilson merged 10 commits intorackspace:v0.2.0from
jrperritt:boot-from-volume
Oct 24, 2014
Merged

Boot from volume#259
smashwilson merged 10 commits intorackspace:v0.2.0from
jrperritt:boot-from-volume

Conversation

@jrperritt
Copy link
Copy Markdown
Contributor

PR corresponding to #258

@jrperritt
Copy link
Copy Markdown
Contributor Author

hmm... I've looked at that JSON 10 times and don't see anything different between them. @smashwilson or @jamiehannaford : Any ideas as to why the isJSONEquals function in testhelper/convenience.go would be saying these are different? Or maybe I'm just missing something obviously different between them.
https://travis-ci.org/rackspace/gophercloud/jobs/38892705#L292

@jamiehannaford
Copy link
Copy Markdown
Contributor

I think it's happening because of a type mismatch. The first value is a deserialised JSON string that comes out of json.Unmarshal; its type for the "block_device_mapping_v2" key is:

[]interface {}{map[string]interface {}{"boot_index":"0", "delete_on_termination":"false", "destination_type":"volume", "source_type":"image", "uuid":"123456", "volume_size":"10"}}

whereas the second value is different because ToServerCreateMap casts it as a more specific slice:

[]map[string]interface {}{map[string]interface {}{"destination_type":"volume", "source_type":"image", "boot_index":"0", "delete_on_termination":"false", "volume_size":"10", "uuid":"123456"}}

If you can think of a way of casting one so that it's consistent with the other, it should be fine

@smashwilson
Copy link
Copy Markdown
Contributor

👍 what @jamiehannaford said. isJSONEquals performs its check by doing a reflect.DeepEquals check on the deserialized JSON string (expected) and the actual value.

I consider that a bug in isJSONEquals - all that you should care about is what the JSON looks like, not the type signatures. Maybe we should round-trip actual through the JSON parser to a fresh interface{} to ensure that the types are consistent?

@jrperritt
Copy link
Copy Markdown
Contributor Author

Good catch @jamiehannaford

👍 on making actual an interface{}.

@jrperritt jrperritt changed the title [wip] Boot from volume Boot from volume Oct 24, 2014
@jrperritt
Copy link
Copy Markdown
Contributor Author

ok @smashwilson , I think this one's ready for review.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hah. I just did this in my branch, too.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Er, this is going to overwrite the res map that the key name was added to up above ☝️. You'll need to move this above the serverMap := res["server"].(map[string]interface{}) bit on line 82, in place of the drive.ToServerCreateMap() call - manual map manipulation has to come last.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll get it one of these times. The question is will you still have any patience when that time comes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was this time 😁 Looks good to me!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants