Fix issues for GCE when running CloudBlockStoreServiceTestCase. #121
Conversation
ghost
assigned 
4 similar comments
| # In GCE, the name of the volume is provided by the client when | ||
| # initially creating the resource. The name cannot be changed after | ||
| # the volume is created. | ||
| cb.log.warning("Setting volume name after it is created is not " |
There was a problem hiding this comment.
@baizhang Can we use a label/tag to track the name instead? In cloudbridge, the name is just a user provided friendly name, and is therefore, changeable. So we could do something similar to what's done for description below? (unless it's super inefficient, and requires a new http request)
There was a problem hiding this comment.
Hi @nuwang -
In #44, where we landed was:
@nuwang - your proposal seems reasonable to me:
Use the disk name field for name (with the specified character and inability to rename restrictions)
Use labels for the disk description field. GCE implementation will convert spaces to underscores
Please review the discussion. Do the premises of the discussion in that PR still hold or do we need to revisit?
Thanks,
-Matt
There was a problem hiding this comment.
@mbookman Thanks for pointing to that. Going through that discussion through, I can't find why exactly we decided to go with the GCE name for the 'name' field, instead of using a label, but I think there was some important reason? As I recall, @afgane raised an issue - something to do with potential mismatches between the original name given at creation time and a later rename perhaps? It would be good to record what it is for future reference.
There was a problem hiding this comment.
I believe that the main issue that we landed on was that there would become disconnects between what the user sees in Galaxy vs. what they see in the Cloud console (or gcloud compute ... list).
The user would change the name in Galaxy and when they looked in Google Cloud, they'd see the old name.
There was also a problem of enforcing uniqueness. You could have multiple users create and rename Galaxy hosts to be "my-galaxy". Going to the Cloud console, you wouldn't see two instances named "my-galaxy", you'd have to know how to view the labels.
These same reasons apply to different Cloud resources. The feeling at the time was that it was better to have restrictions on naming such that Galaxy and Cloud were in sync. Only if there was a real clamor for the ability to rename did it seem worth creating this potential disconnect between Galaxy/Cloud.
There was a problem hiding this comment.
And - not all Cloud resource support labels. So we could potentially end up with a hodge podge of resources which did support renames vs. those that did not.
There was a problem hiding this comment.
@mbookman Sorry, I had missed this comment. Right, that was the issue - thanks for penning it down. I guess we still haven't figured out a way to make this consistent across all providers, since, as it stands now, a rename will fail silently for GCE only. Let's leave this as is for now, since it's not a major issue, and hopefully, figure out a way to make things uniform across all providers if and when needed?
| disk=self.name) | ||
| .execute()) | ||
| self._provider.wait_for_operation( | ||
| response, zone=self._provider.default_zone) |
There was a problem hiding this comment.
@baizhang Is there a specific reason to block till the delete is complete?? I think ideally, the delete call shouldn't block. I believe the tests handle this already, and wait for the delete to complete.
There was a problem hiding this comment.
This is the only issue that remains - should we just go ahead and merge it and we can sort this issue out later since it isn't a blocker?
There was a problem hiding this comment.
Thanks for your review, @nuwang !
I agree. Let's go ahead and merge this one first. And I will work on a non-blocking delete method in a separate pull request.
| # the snapshot is created. | ||
| cb.log.warning("Setting snapshot name after it is created is not " | ||
| "supported by this provider.") | ||
|
|
There was a problem hiding this comment.
Same as for Volume name. The name should ideally be editable. In GCE, I guess the 'name' maps more to cloudbridge's id.
Hi @nuwang @afgane @chiniforooshan @mbookman,
This pull request fixes a few issues for GCE when running CloudBlockStoreServiceTestCase. Specifically,
Thank you!