provider/azure: creating instances now actually works

[skinny]

Based on the 'long' Azure image names (eg. b39f27a8b8c64d52b05eac6a62ebad85__Ubuntu-14_10-amd64-server-20141022.3-en-us-30GB) the provider now actually finds the correct image and creates the VM.

Also prettified the logging a bit, to report the actual image names and to no longer hide the exceptions produced in the two 'retrieve*' functions.

[phinze]

Thanks for this! Is there an acceptance test case we could add to cover the image name behavior?

/cc @aznashwan for review

[skinny]

Will try to add this later today!

[aznashwan]

@skinny thanks for the fix; and hope you enjoyed your first rundown with go 😉

The code looks great; and the tests should work fine after the names in the tested configs are updated and the test conditions verifying those names are changed.

I'm unfortunately [nowhere near] the right person to pass judgement on the change itself, as @svanharmelen was the original implementer of the instance resource.
Personally, I'd argue in favour of the current implementation, as those image names are both very cumbersome and change regularly with every update of the image on Azure, and that referring to the images by labels is a bit more handy (I do, however, fully agree that the way it currently is, both the image parameter's name and the documentation are very confusing and should be brought up to speck).

[skinny]

Hi, getting to know Go is a bit weird but eventually good experience. Regarding the labels: this was my first attempt to "just get it working". Actually I'm now trying to get it both working for Labels and Image ID's. When you supply a Label (Ubuntu 14.04 LTS) it will by default use the latest image found when sorted by publication date.

Let me know if that would work for you too

Mark

[svanharmelen]

@skinny just a quick message to let you know I'm on holiday without any kind of computing device (apart from my iPhone) and an extremely crappy internet connection (my wife pulled me away from any/all kind of tech for 10 days 😉).

So will follow up, but probably in about a week or so...

[skinny]

@svanharmelen no problem, enjoy your holiday! We can have a talk when you are back, in the mean time Ill experiment with the other Azure resources as well.

Mark

[svanharmelen]

@skinny having thought about this a little I really don't think this would be a good approach. Like @aznashwan I really favour to use the labels instead of the names as I feel it makes using the Azure provider al lot more pleasant.

What we could do to improve the current implementation, is change/update the currently available options. If we would remove the image parameter and add a vmimage_label and osimage_label parameter instead.

The verifyInstanceParameters should then be updated so it makes sure one (and only one) of these new values is supplied and if the osimage_label is supplied that the storage_service_name is also supplied.

Of course the docs should then also be updated accordingly and the retrieveVMImageDetails and retrieveOSImageDetails should be tweaked just a little as well. But with that done I think we should have fixed the parts that now a little unclear, right?

@aznashwan what do you think about this approach?

[skinny]

Sounds great and also in the direction I was working. Will work on this a bit more and update the PR

[svanharmelen]

@skinny any progress? Need any help?

[josephholsten]

@skinny how's this coming?

[phinze]

Reviewing some old PRs and this seems like this one has petered out. Going to close for now.

/cc @svanharmelen / @aznashwan in case there's any outstanding work remaining here that should get represented elsewhere.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.