Add glance-simplestreams-sync --set-latest-property functest

[guimaluf]

When creating Openstack VMs the user has to specify the image it wants
to use. sstream-mirror-glance adds a date to the image name, so they
always have to recheck which is the current latest image.

This commit tests the usage of the set-latest-property configuration.

When set-latest-property is given sstream-mirror-glance will set the
recently synced image with the latest=true property and then remove
the latest property from all the os_version/architecture matching
images.

Closes-bug: LP #1933130

[ajkavanagh]

Thanks for the patch; it looks like it is starting to get there. A couple of minor nitpicks, but also a significant issue with the async nature of the models and the code driving them. e.g. config changed -> "config-changed" hook, but that runs asynchronously to the code that starts it. Please take a look at the comments. Thanks.

[ajkavanagh]

So I've taken a step back, and instead of just concentrating on the code in this review, I looked at the associated bug. I'm pretty sure that the code that this might be testing doesn't do what the bug reporter wants: a named alias for the 'latest' version of a particular synced image type.

This code appears to be testing 'adding a property to every image and calling that latest' which doesn't seem that useful to be honest.

I think this solution should be revisited - perhaps email the opendev mailing list, or contact the team via a channel, and see if we can move this forward. Thanks.

[guimaluf]

I've discussed with @wolsen and @freyes about this topic. it was suggested that instead of changing the name of the image I should add the latest attribute.

I'm probably not doing in the best way, but if you look at my proposal for simplestreams I'm removing the latest property from all other images while letting only the last one synced.

diff --git a/simplestreams/mirrors/glance.py b/simplestreams/mirrors/glance.py
index 9181e9c..b96f8eb 100644
--- a/simplestreams/mirrors/glance.py
+++ b/simplestreams/mirrors/glance.py
@@ -517,6 +523,20 @@ class GlanceMirror(mirrors.BasicMirrorWriter):
 
             print("created %s: %s" % (glance_image.id, full_image_name))
 
+            # TODO(guimalufb) use self.loaded_content or
+            # self.load_products() instead
+            if self.set_latest_property:
+                # Search all images with the same target attributtes
+                _filter_properties = {'filters': {
+                    'latest': 'true',
+                    'os_version': glance_props['os_version'],
+                    'architecture': glance_props['architecture']}}
+                images = self.gclient.images.list(**_filter_properties)
+                for image in images:
+                    if image.id != glance_image.id:
+                        self.gclient.images.update(image.id,
+                                                   remove_props=['latest'])
+

I'd like to test having an image with the latest and then a new image on top. but I have no idea how to force that scenario.

thanks for you careful review!

[ajkavanagh]

I've discussed with @wolsen and @freyes about this topic. it was suggested that instead of changing the name of the image I should add the latest attribute.

Okay, at the very least, please go back to the bug and outline the solution so that the bug submitter can take a look at agree/disagree that it solves their problem. Otherwise, whilst it may be a solution, it simply may not work for the bug submitter and their tools.

I'm probably not doing in the best way, but if you look at my proposal for simplestreams I'm removing the latest property from all other images while letting only the last one synced.

That's why I couldn't find the change. This change needs to be on gerrit (review.opendev.org), NOT on launchpad. If you take a look at the code page (https://code.launchpad.net/charm-glance-simplestreams-sync) you'll see that it syncs the code from https://opendev.org/openstack/charm-glance-simplestreams-sync.git - any changes you attempt to make on LP will a) not be merged, and b) if they were, overwritten by a sync. Please re-do your PR on opendev.

diff --git a/simplestreams/mirrors/glance.py b/simplestreams/mirrors/glance.py
index 9181e9c..b96f8eb 100644
--- a/simplestreams/mirrors/glance.py
+++ b/simplestreams/mirrors/glance.py
@@ -517,6 +523,20 @@ class GlanceMirror(mirrors.BasicMirrorWriter):
 
             print("created %s: %s" % (glance_image.id, full_image_name))
 
+            # TODO(guimalufb) use self.loaded_content or
+            # self.load_products() instead
+            if self.set_latest_property:
+                # Search all images with the same target attributtes
+                _filter_properties = {'filters': {
+                    'latest': 'true',
+                    'os_version': glance_props['os_version'],
+                    'architecture': glance_props['architecture']}}
+                images = self.gclient.images.list(**_filter_properties)
+                for image in images:
+                    if image.id != glance_image.id:
+                        self.gclient.images.update(image.id,
+                                                   remove_props=['latest'])
+

I'd like to test having an image with the latest and then a new image on top. but I have no idea how to force that scenario.

Yes, that's tricky from a functional/integration test scenario. Best approach would be to verify that functionality using unit tests and mocking out simplestreams (or even in the g-sss (glance-simplestreams-sync) charm itself).

thanks for you careful review!

No problem. :)

[guimaluf]

Okay, at the very least, please go back to the bug and outline the solution so that the bug submitter can take a look at agree/disagree that it solves their problem. Otherwise, whilst it may be a solution, it simply may not work for the bug submitter and their tools.

done!

That's why I couldn't find the change. This change needs to be on gerrit (review.opendev.org), NOT on launchpad. If you take a look at the code page (https://code.launchpad.net/charm-glance-simplestreams-sync) you'll see that it syncs the code from https://opendev.org/openstack/charm-glance-simplestreams-sync.git - any changes you attempt to make on LP will a) not be merged, and b) if they were, overwritten by a sync. Please re-do your PR on opendev.

my bad. I ran git review for the simplestreams repository and it didn't work, thus I assumed it would work for g-s-s.
done!

Yes, that's tricky from a functional/integration test scenario. Best approach would be to verify that functionality using unit tests and mocking out simplestreams (or even in the g-sss (glance-simplestreams-sync) charm itself).

That has been done on simplestreams repository

[ChrisMacNaughton]

Landing this change but only after raising #947 to resolve the strangely placed config setting