Fetch latest image from clouds #204

lucasmoura · 2022-06-08T15:11:03Z

Add logic to fetch daily images directly from the clouds and not through simplestreams.
Right now we are only supporting this behavior for AWS, Azure and GCP.
Additionally, we are also adding support for launching PRO and FIPS PRO tests for those clouds.

PS: We should still add tests for the new methods here, but first I think we need a fixture that is able to create the cloud instances.
However, I think we can add this into a separate PR.

lucasmoura

Also, we should define how the handle the released_images method. On GCP and Azure that method was basically using the daily_image method, so we should be fine. But on AWS, there might be a difference, even though I couldn't find the right owner id for the released images.

So I think we can discuss on how we should support that distinction, daily vs released for AWS.

TheRealFalcon

This looks good to me, but maybe let one more person review.

TheRealFalcon · 2022-06-08T19:03:15Z

pycloudlib/cloud.py

+
+    GENERIC = "generic"
+    PRO = "PRO"
+    PRO_FIPS = "FIPS PRO"


Nit: the words in the value are reversed compared to the key. Also, up 2 lines, 'generic' is lowercase whereas the rest of the values are uppercase.

Good catch on reversed name, but the uppercase is just because we usually refer to PRO instances using all letters in uppercase. And since we will surface that name on the Exceptions, I have kept that it just for consistency. But I can update that if necessary

We should standardize on "Pro" and "Pro FIPS". That is how they're marketed. E.g. https://aws.amazon.com/marketplace/pp/prodview-pfsgbblavhjc4

paride · 2022-06-09T13:28:53Z

I'm going to try using this in some bootspeed jobs and submit my review, but by reading the code it LGTM.

paride · 2022-06-09T16:44:49Z

One issue I see is that with this change pycloudlib is unable to find AMIs for the devel release:

In [4]: ec2.daily_image(release='bionic')
Out[4]: 'ami-0e8597a872ec674ee'

In [5]: ec2.daily_image(release='kinetic')
---------------------------------------------------------------------------
Exception                                 Traceback (most recent call last)
<ipython-input-5-8e5735439f6c> in <module>
----> 1 ec2.daily_image(release='kinetic')

~/ubuntu/git/pycloudlib/pycloudlib/ec2/cloud.py in daily_image(self, release, arch, image_type)
    174         """
    175         self._log.debug("finding daily Ubuntu image for %s", release)
--> 176         image = self._find_latest_image(
    177             release=release, arch=arch, image_type=image_type
    178         )

~/ubuntu/git/pycloudlib/pycloudlib/ec2/cloud.py in _find_latest_image(self, release, arch, image_type)
    152 
    153         if not images.get("Images"):
--> 154             raise Exception(
    155                 "Could not find {} image for {} release".format(
    156                     image_type.value, release

Exception: Could not find generic image for kinetic release

lucasmoura · 2022-06-09T17:37:47Z

Good point, I will double check if we have a specific owner for launching the kinetic image on AWS

lucasmoura · 2022-06-09T19:06:09Z

@paride @TheRealFalcon code updated

paride · 2022-06-10T13:37:40Z

@lucasmoura are you sure you pushed the latest changes? I still see 18ff0ca on the tip of your branch.

lucasmoura · 2022-06-10T13:45:06Z

I have pushed to the wrong branch 🤦
Now it is updated @paride

paride · 2022-06-10T17:01:29Z

I think I found two more issues:

EC2 daily_image() used to accept amd64 as the arch, while now it wants x86_64. Reproducer: ec2.daily_image('bionic', arch='x86_64'). I think specifying the Ubuntu arch name (amd64) makes sense for pycloudlib.
The image serial lookup (still based on streams) doesn't work:

In [11]: image = ec2.daily_image('bionic')

In [12]: ec2.image_serial(image)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-12-c36dc0d66013> in <module>
----> 1 ec2.image_serial(image)

~/ubuntu/git/pycloudlib/pycloudlib/ec2/cloud.py in image_serial(self, image_id)
    198         )
    199         filters = ["id=%s" % image_id]
--> 200         image_info = self._streams_query(filters, daily=True)
    201         if not image_info:
    202             image_info = self._streams_query(filters, daily=False)

~/ubuntu/git/pycloudlib/pycloudlib/cloud.py in _streams_query(filters, daily)
    219         result = stream.query(filters)
    220         if not result:
--> 221             raise ValueError(f"No images found matching filters: {filters}")
    222 
    223         return result

ValueError: No images found matching filters: ['id=ami-0e8597a872ec674ee']

lucasmoura · 2022-06-13T14:58:44Z

@paride okay, just addressing the points you made:

Yes, I agree that keeping amd64 would be better, but on aws we cannot use it directly as an architecture filter, as can be seen here:

https://docs.aws.amazon.com/cli/latest/reference/ec2/describe-images.html

We could map amd64into the x86_64 string if you think it is needed.

That one is a little odd. I have identified the image location simplestreams is using and applied it for the new aws image query. The problem is that by querying aws directly, we can fetch images that are newer than the ones on simplestreams, which will cause the problem we are seeing. This can be seen when we try to use xenial using the new name filter I have commented about.

Also, I realized that the image_serial function is also not working for the released images, which were not modified in this PR.
With that said, @paride what do you think should be the best plan forward ? I could try to also adapt image_serial to fetch the cloud directly, by I need to double check if the information is surfaced through the clouds.

PS: I have pushed the new daily name filter on this PR, so you can check the Xenial issue I have described.

paride · 2022-06-14T17:42:25Z

@lucasmoura apparently the image serial is exposed via the image Name, see for example ami-03d0d5c3a4731a9dd (region: us-west-1), which has serial 20210329.1:

aws --region us-west-1 ec2 describe-images --image-ids ami-03d0d5c3a4731a9dd                                                                                                                
{
    "Images": [
        {
            "Architecture": "x86_64",
            "CreationDate": "2021-03-30T17:42:49.000Z",
            "ImageId": "ami-03d0d5c3a4731a9dd",
            "ImageLocation": "ubuntu-images-us-west-1-release/xenial/20210329.1/hvm/instance-store/image.img.manifest.xml",
            "ImageType": "machine",
            "Public": true,
            "OwnerId": "099720109477",
            "PlatformDetails": "Linux/UNIX",
            "UsageOperation": "RunInstances",
            "State": "available",
            "BlockDeviceMappings": [],
            "Description": "Canonical, Ubuntu, 16.04 LTS, amd64 xenial image build on 2021-03-29",
            "EnaSupport": true,
            "Hypervisor": "xen",
            "Name": "ubuntu/images/hvm-instance/ubuntu-xenial-16.04-amd64-server-20210329.1",
            "RootDeviceType": "instance-store",
            "SriovNetSupport": "simple",
            "VirtualizationType": "hvm",
            "DeprecationTime": "2023-03-30T17:42:49.000Z"
        }
    ]
}

If that's stable (let's ask CPC?) maybe we can extract the serial from there.

paride · 2022-06-14T17:54:47Z

On the arch name (amd64 vs x86_64): I am not sure. From one side I see pycloudlib as Ubuntu-centric, so I'd like too keep the Ubuntu arch names. OTOH the amd64 <-> x86_64 mapping works well only until there's a simple bijective mapping. Things may become ugly if Ubuntu amd64v2 is released (compiled a with higher baseline for the arch, this is in the talks), or even worse if ec2 starts offering x86_64v2 before Ubuntu releases amd64v2 (e.g. because CPC prepares higher performance builds for some instance types - completely hypothetical not impossible I think). So maybe in the end it's better to keep it simple and use the cloud arch names with no mappings.

paride · 2022-06-15T12:37:15Z

On daily_image() vs released_image(), note that for a released image we have:

$ aws --region us-east-2 ec2 describe-images --image-id ami-05a2de8ec6f3e7b61
[...]
            "Name": "ubuntu/images/hvm-ssd/ubuntu-jammy-22.04-amd64-server-20220609",

while for a daily image:

$ aws --region us-east-2 ec2 describe-images --image-id ami-02f3416038bdb17fb
[...]
            "Name": "ubuntu/images-testing/hvm-ssd/ubuntu-jammy-daily-amd64-server-20220609",

so maybe the trick is to search for images by Name (given that the format is fixed), and use /images-testing/ for dailies, and /images/ for released images.

lucasmoura · 2022-06-20T15:09:04Z

@paride I have updated the code for ec2 to fetch released images directly from AWS while also fixing the image_serial issue that was raised

paride

Nice! But it doesn't seem to be working yet for released images:

In [13]: ec2 = pycloudlib.EC2(tag='foo', region='us-east-1')

In [14]: ec2.released_image('focal')
Out[14]: 'ami-031cf125b681ca3e0'

In [15]: ec2.image_serial('ami-031cf125b681ca3e0')
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-15-3b7825534dd7> in <module>
----> 1 ec2.image_serial('ami-031cf125b681ca3e0')

~/ubuntu/git/pycloudlib/pycloudlib/ec2/cloud.py in image_serial(self, image_id)
    241         )
    242         filters = ["id=%s" % image_id]
--> 243         image_info = self._streams_query(filters, daily=True)
    244         if not image_info:
    245             image_info = self._streams_query(filters, daily=False)

~/ubuntu/git/pycloudlib/pycloudlib/cloud.py in _streams_query(filters, daily)
    219         result = stream.query(filters)
    220         if not result:
--> 221             raise ValueError(f"No images found matching filters: {filters}")
    222 
    223         return result

ValueError: No images found matching filters: ['id=ami-031cf125b681ca3e0']

For this AMI the serial is there:

$ aws --profile cloud-init --region us-east-1 ec2 describe-images --image-id ami-031cf125b681ca3e0
[...]
            "Name": "ubuntu/images/hvm-ssd/ubuntu-focal-20.04-amd64-server-20220615",

paride · 2022-06-20T17:25:18Z

Also: can you please rebase on main? The change that just landed there will make it easier to test your branch with the bootspeed jobs. Thanks!

lucasmoura · 2022-06-20T20:06:49Z

@paride I forgot one last bit 🤦, but it is fixed now.
And I have also rebased on main

paride

LGTM, also tested in Jenkins with the bootspeed jobs and it works fine. We don't have image_serial() for non-EC2 clouds, but I think that's fine for now, we can add it if/once needed. Thanks for addressing all the review comments!

TheRealFalcon · 2022-07-01T19:50:30Z

@lucasmoura , is there anything else to do here?

lucasmoura · 2022-07-04T20:12:12Z

@TheRealFalcon I was just waiting for a review from someone on the UA team for the PRO machines. But I think we can merge and if we find a bug I can come up with a follow up PR

orndorffgrant

@lucasmoura

Just a nitpick on the naming: We should standardize on "Pro" and "Pro FIPS". That is how they're marketed. E.g. https://aws.amazon.com/marketplace/pp/prodview-pfsgbblavhjc4

lucasmoura · 2022-07-05T18:17:48Z

@orndorffgrant done

orndorffgrant

Thanks @lucasmoura and sorry for the nitpick :)

lucasmoura · 2022-07-05T19:15:10Z

@orndorffgrant no worries at all

paride

looks good, +1 again!

paride · 2022-07-12T16:18:19Z

@lucasmoura do you think this is good to land?

lucasmoura · 2022-07-12T16:48:12Z

Yes, merging it right now

lucasmoura commented Jun 8, 2022

View reviewed changes

lucasmoura force-pushed the fetch-latest-image-from-clouds branch from ec27cab to 18ff0ca Compare June 8, 2022 18:10

TheRealFalcon approved these changes Jun 8, 2022

View reviewed changes

lucasmoura force-pushed the fetch-latest-image-from-clouds branch from 18ff0ca to fe17ce2 Compare June 10, 2022 13:44

lucasmoura force-pushed the fetch-latest-image-from-clouds branch from fe17ce2 to be14641 Compare June 10, 2022 14:14

paride requested changes Jun 20, 2022

View reviewed changes

lucasmoura force-pushed the fetch-latest-image-from-clouds branch from e0bf197 to 8e48360 Compare June 20, 2022 20:05

Lucas Moura added 7 commits June 20, 2022 17:06

azure: add support for launching pro images

246f48e

ec2: add support for launching pro images

d60aed8

gce: add support for launching pro instances

958bd8f

rename FIPS PRO image type variable

f3e967e

ec2: new name filter for daily images

a7d71de

ec2: fetch images from aws for released images

d303110

ec2: fetch image serial directly from aws

289f2c7

lucasmoura force-pushed the fetch-latest-image-from-clouds branch from 8e48360 to 289f2c7 Compare June 20, 2022 20:06

paride approved these changes Jun 21, 2022

View reviewed changes

orndorffgrant suggested changes Jul 5, 2022

View reviewed changes

Rename FIPS_PRO image id to PRO_FIPS

f318222

orndorffgrant approved these changes Jul 5, 2022

View reviewed changes

paride approved these changes Jul 6, 2022

View reviewed changes

orndorffgrant mentioned this pull request Jul 7, 2022

prevent unneeded daemon activation and mitigate stalled daemon bug canonical/ubuntu-pro-client#2151

Merged

6 tasks

lucasmoura merged commit 0a55726 into canonical:main Jul 12, 2022

lucasmoura deleted the fetch-latest-image-from-clouds branch July 12, 2022 16:48

Fetch latest image from clouds #204

Fetch latest image from clouds #204

Uh oh!

Conversation

lucasmoura commented Jun 8, 2022

Uh oh!

lucasmoura left a comment

Choose a reason for hiding this comment

Uh oh!

TheRealFalcon left a comment

Choose a reason for hiding this comment

Uh oh!

TheRealFalcon Jun 8, 2022

Choose a reason for hiding this comment

Uh oh!

lucasmoura Jun 8, 2022

Choose a reason for hiding this comment

Uh oh!

orndorffgrant Jul 1, 2022

Choose a reason for hiding this comment

Uh oh!

paride commented Jun 9, 2022

Uh oh!

paride commented Jun 9, 2022

Uh oh!

lucasmoura commented Jun 9, 2022

Uh oh!

lucasmoura commented Jun 9, 2022

Uh oh!

paride commented Jun 10, 2022

Uh oh!

lucasmoura commented Jun 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

paride commented Jun 10, 2022

Uh oh!

lucasmoura commented Jun 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

paride commented Jun 14, 2022

Uh oh!

paride commented Jun 14, 2022

Uh oh!

paride commented Jun 15, 2022

Uh oh!

lucasmoura commented Jun 20, 2022

Uh oh!

paride left a comment

Choose a reason for hiding this comment

Uh oh!

paride commented Jun 20, 2022

Uh oh!

lucasmoura commented Jun 20, 2022

Uh oh!

paride left a comment

Choose a reason for hiding this comment

Uh oh!

TheRealFalcon commented Jul 1, 2022

Uh oh!

lucasmoura commented Jul 4, 2022

Uh oh!

orndorffgrant left a comment

Choose a reason for hiding this comment

Uh oh!

lucasmoura commented Jul 5, 2022

Uh oh!

orndorffgrant left a comment

Choose a reason for hiding this comment

Uh oh!

lucasmoura commented Jul 5, 2022

Uh oh!

paride left a comment

Choose a reason for hiding this comment

Uh oh!

paride commented Jul 12, 2022

Uh oh!

lucasmoura commented Jul 12, 2022

Uh oh!

Reviewers

Assignees

lucasmoura commented Jun 10, 2022 •

edited

Loading

lucasmoura commented Jun 13, 2022 •

edited

Loading