Add labelling for newly created GCP disks and snapshots #933
Conversation
|
@AmineChikhaoui do you think this change is okay or if anything in particular should be changed ? This would be very useful for billing and usage reports, prod vs dev resources separation,.. etc. Thanks. |
amemni
changed the title
nixops/backends/gce.py
Outdated
| self.log("updating labels of snapshot {0}".format(snapshot_name)) | ||
| self.connect().connection.request( | ||
| '/global/snapshots/%s/setLabels' %(snapshot_name), | ||
| method = 'POST', data = { |
There was a problem hiding this comment.
indentation seems off here ?
nixops/backends/gce.py
Outdated
| @@ -769,6 +780,25 @@ def backup(self, defn, backup_id): | |||
| .format(volume.name, self.machine_name) | |||
| }) | |||
|
|
|||
| # Apply labels to snapshot just created | |||
| def check_snapshot_initiated(): | |||
There was a problem hiding this comment.
should be probably a wait until expected state e.g snapshot initiated ?
There was a problem hiding this comment.
i did that @AmineChikhaoui , I guess the expected state for when a snap was initiated (present) should be one of "UPLOADING", "CREATING" or "READY" (https://cloud.google.com/compute/docs/reference/rest/beta/snapshots). That being said, I think it takes only 1 or 2 seconds for that so the output would be something like this:
$ nixops backup -d macys-dev-1 frontend.> backing up GCE machine 'n-b963de1933f911e89f780a29f7962a18-frontend' using ID '20180522045235' frontend.> initiating snapshotting of disk 'n-b963de1933f911e89f780a29f7962a18-frontend-root': 'backup-20180522045235-e89f780a29f7962a18-frontend-root' frontend.> . done frontend.> updating labels of snapshot backup-20180522045235-e89f780a29f7962a18-frontend-root frontend.> initiating snapshotting of disk 'n-b963de1933f911e89f780a29f7962a18-frontend-data': 'backup-20180522045235-e89f780a29f7962a18-frontend-data' frontend.> . done frontend.> updating labels of snapshot backup-20180522045235-e89f780a29f7962a18-frontend-data 20180522045235
Let me know if you think that should be changed some other way, thanks !!
nixops/backends/gce.py
Outdated
| def wait_for_snapshot_initiated(self, snapshot_name): | ||
| while True: | ||
| try: | ||
| snapshot_status = self.connect().connection.request("/global/snapshots/{0}".format(snapshot_name), method='GET').object['status'] |
There was a problem hiding this comment.
Any reason for switching from self.connect().ex_get_snapshot() to a raw http request ?
I think we'd rather stick with libcloud api whenever possible.
There was a problem hiding this comment.
No reason, that's actually better, I changed it that way and re-tested, thanks for pointing that out.
Also, would you think the same should be done here or that's okay (line 794) ?
self.connect().connection.request("/global/snapshots/{0}".format(snapshot_name), method='GET').object['labelFingerprint']
There was a problem hiding this comment.
right, maybe wait_for_snapshot_initiated() can return the object so that you re-use that to get the labelFingerprint instead of re-doing the api call.
There was a problem hiding this comment.
Actually it turns out the GCESnapshot object returned by libcloud doesn't have an attribute 'labelFingerprint'. Here's an example:
<VolumeSnapshot id=542163224484327809 size=0 driver=Google Compute Engine state=None>
I did also checked that here, the attributes being returned are in the function _to_snapshot(): http://libcloud.readthedocs.io/en/v0.14.0/_modules/libcloud/compute/drivers/gce.html
So I guess we're gonna need to use a raw http request then, no ? Thanks.
| @@ -769,10 +782,35 @@ def backup(self, defn, backup_id): | |||
| .format(volume.name, self.machine_name) | |||
| }) | |||
|
|
|||
| # Apply labels to snapshot just created | |||
There was a problem hiding this comment.
@amemni this seems to run regardless of whether the defn.labels is set or not, I guess it would be better to check if the labels aren't empty and do the call only if there is something set there
There was a problem hiding this comment.
Agreed, I've added a control over defn.labels, thanks.
GCE labels are very useful for grouping together resources from the same development stage, easily searching for and separatly treating resources intended for production from the ones of development .. Among use cases are Usage reports, Billing export ..
This was done the same way as in: #866
Also, looking in gce.py, Libcloud seem to yet only support labelling of nodes or images (none for disks and snapshots).