Merge ~d0ugal/maas-images:release-notifications into maas-images:master

Note: I'm close to splitting the streams into candidate and stable. candidate replaces daily so I'll do the same below.

A SimpleStream is a collection of product streams, each product stream contains a list of products, and each product contains a list of versions. For example the MAAS stream contains three product streams, one for Ubuntu, one for CentOS, and one for bootloaders. Each product stream contains a collection of products such as 14.04, 16.04, 18.04, and 20.04. And each product contains a list of versions which are based on the date (20200720.0, 20200720.1, 20200724.0, etc).

You are creating a new product stream. SimpleStreams expects this to be a collection of products, even if we only have one product now. As such I think the product stream name should be "com.ubuntu.maas:candidate:1:maas-notifications" and the product name should be "com.ubuntu.com.candidate:1:release-notifications". In the future we can add more products for things like feature-deprecation or image removal.

Streams only contain metadata and the only metadata that changes in a stream is the versions. We can try to change that but the stream needs to be readable by older versions of MAAS. I do think the metadata needs to be consistent with content changing in a specific key. This format may work(I didn't test)

{
    "content_id": "com.ubuntu.maas:candidate:1:maas-notifications",
    "datatype": "notifications",
    "format": "products:1.0",
    "products": {
        "com.ubuntu.com.candidate:1:release-notifications": {
            "name": "release-notifications",
            "label", "candidate",
            "notifications": {
                [
                    {
                        "maas_version": "< 2.8",
                        "message": "<strong>We\u2019ve released MAAS 2.8.</strong> This version supports LXD VM hosts, faster UI and many bug fixes. Find out <a href=\"/docs/release-notes\">what\u2019s new in 2.8</a>.\n",
                        "version": "1.0"
                    }
                ]
            }
        }
    }
}

Your challenge is create a format that works now and in the future while not breaking previous versions of MAAS. This may require you to add fields which don't make sense. For example every product defines arch, arches, and os MAAS may assume those values are always available. What you should do is create a format, merge it into the MAAS stream, point a running MAAS at the stream, and verify MAAS can still download images and no errors/exceptions have been raised in the logs. You can do that as follows

1. Create a mirror of our stream

KEYRING_FILE=/usr/share/keyrings/ubuntu-cloudimage-keyring.gpg
IMAGE_SRC=images.maas.io/ephemeral-v3/daily
IMAGE_DIR=$HOME/maas-v3-images

sstream-mirror --keyring=$KEYRING_FILE \
    http://images.maas.io/ephemeral-v3/daily $IMAGE_DIR \
    'arch=amd64' 'release~(centos70|xenial|bionic|focal)' --max=1 --progress
sstream-mirror --keyring=$KEYRING_FILE \
    http://images.maas.io/ephemeral-v3/daily $IMAGE_DIR \
    'os~(grub*|pxelinux)' --max=1 --progress

# sstream doesn't grab the unsigned streams
rsync -xavzhP --del --exclude .bzr \
    rsync://images.maas.io/maas-images/ephemeral-v3/d...

This looks better but it doesn't work with lp:maas-images due versions missing.

$ ./maas-images/bin/meph2-util merge release-notifications/ candidate/
Traceback (most recent call last):
  File "./maas-images/bin/meph2-util", line 25, in <module>
    call_entry_point("meph2.commands.meph2_util.main")
  File "./maas-images/bin/meph2-util", line 22, in call_entry_point
    sys.exit(getattr(sys.modules[istr], ent)())
  File "/home/lee/maas-images/meph2/commands/meph2_util.py", line 510, in main
    return args.action(args)
  File "/home/lee/maas-images/meph2/commands/meph2_util.py", line 254, in main_merge
    for (version, version_info) in product_info['versions'].items():
KeyError: 'versions'

The important part is making sure the stream works with MAAS unmodified, you can modify lp:maas-images. However it may be difficult to work around versions being missing due to our process.

1. A new product stream is created, it is merged into the candidate stream on images.maas.io. This happens once.
  $ meph2-util merge release-notifications/ candidate/
2. CPC has a job which generates new versions by running MAAS images
  $ meph2-import release-notifications.yaml release-notifications
3. CPC inserts new *versions* into the candidate stream. It does not change any metadata.
  $ meph2-util insert release-notifications/ candidate/
4. CPC has a job which will allow us to promote *versions* from candidate to stable
  $ meph2-util patch cpc-patch.yaml candidate stable

It may be easier to store the content in a separate file and use versions.

Each item requires an ftype. MAAS looks at this field to know if it can process the item. You can actually put the version in the ftype which may simplify things.

If the notification data is being stored in a separate file I would use the standard item format and keep all things like maas_version in that separate YAML/JSON file[1]. One thing to keep in mind is that the BootResourceFile table requires a type. All current types are for images and boot loaders(see BOOT_RESOURCE_FILE_TYPE in src/maassserver/enum.py) so a migration will be required.

The notifications seem to be fairly short, you could also store them directly in the stream[2].

[1] https://pastebin.canonical.com/p/3Q8KHdfpgk/
[2] https://pastebin.canonical.com/p/QmcYtMfM9N/

The "1" in "com.ubuntu.maas:candidate:1:maas-notification" can be used as your version. Its up to you if you want a specific field.

It looks like our existing mirroring tools require the file field, and it must be valid. If I generate a stream and try to mirror it the SimpleStreams mirroring tool chokes. sstream-mirror is Canonical's standard mirroring tool[1] which goes back at least to Precise. Unfortinetly that means this stream must work with the tool as is.

$ ./bin/meph2-import conf/release-notifications.yaml /tmp/test
$ $ sstream-mirror /tmp/test/ /tmp/mirror
+ com.ubuntu.maas:candidate:1:maas-notification 20200921.0 release_notification release-notifications/20200921.0/release-notification.yaml 0 Mb
0 Mb change
Traceback (most recent call last):
  File "/usr/bin/sstream-mirror", line 157, in <module>
    main()
  File "/usr/bin/sstream-mirror", line 153, in main
    tmirror.sync(smirror, initial_path)
  File "/usr/lib/python3/dist-packages/simplestreams/mirrors/__init__.py", line 91, in sync
    return self.sync_index(reader, path, data, content)
  File "/usr/lib/python3/dist-packages/simplestreams/mirrors/__init__.py", line 254, in sync_index
    self.sync(reader, path=epath)
  File "/usr/lib/python3/dist-packages/simplestreams/mirrors/__init__.py", line 89, in sync
    return self.sync_products(reader, path, data, content)
  File "/usr/lib/python3/dist-packages/simplestreams/mirrors/__init__.py", line 355, in sync_products
    self.insert_item(item, src, target, pgree, ipath_cs)
  File "/usr/lib/python3/dist-packages/simplestreams/mirrors/__init__.py", line 490, in insert_item
    self.store.insert(data['path'], contentsource,
  File "/usr/lib/python3/dist-packages/simplestreams/objectstores/__init__.py", line 142, in insert
    buf = reader.read(self.read_size)
  File "/usr/lib/python3/dist-packages/simplestreams/contentsource.py", line 275, in read
    buf = self.cs.read(size)
  File "/usr/lib/python3/dist-packages/simplestreams/contentsource.py", line 135, in read
    self.open()
  File "/usr/lib/python3/dist-packages/simplestreams/contentsource.py", line 131, in open
    self.fd = self._open()
  File "/usr/lib/python3/dist-packages/simplestreams/contentsource.py", line 127, in _open
    raise myerr
OSError: Unable to open /tmp/test/release-notifications/20200921.0/release-notification.yaml. mirrors=[]

[1] https://maas.io/docs/local-image-mirror

> The "1" in "com.ubuntu.maas:candidate:1:maas-notification" can be used as your
> version. Its up to you if you want a specific field.
>
> It looks like our existing mirroring tools require the file field, and it must
> be valid. If I generate a stream and try to mirror it the SimpleStreams
> mirroring tool chokes. sstream-mirror is Canonical's standard mirroring
> tool[1] which goes back at least to Precise. Unfortinetly that means this
> stream must work with the tool as is.

Hrm, interesting. I don't understand why that is the case because as far as I can tell the paths are valid and do exist.

For example:

https://people.canonical.com/~dougal/maas-stream-with-notifications/streams/v1/com.ubuntu.maas:candidate:1:maas-notification.json

path to:

https://people.canonical.com/~dougal/maas-stream-with-notifications/release-notifications/20200922.0/release-notification.yaml

Found it. I wasn't taking into account the args.target when writing the files. wasn't aware of that feature.

ah ok mirroring works now :) Couple more questions below.

I would just use the full version form(2.8.0), even if we aren't going to notify on micro versions. The backend always uses the full version. You can then validate it with a regex(\d+\.\d+\.\d+) to ensure not only is it a string but its in the right form.

Whenever reading data from a remote client and server you should assume the data is bad. It shouldn't matter to MAAS if the version is float(2.8) or str(2.8), the data is the same. Thats why people like strongly typed languages for things like this :)

MAAS should do something like this:

from provisioningserver.utils.version import get_maas_version, get_maas_version_tuple

maas_version = get_version_tuple(get_maas_version())
version_for_notification = get_version_tuple(str(version))

if version_for_notification > maas_version:
    # Show notification

This will work if version is a string or float. Using the existing version code also allows this to work for micro versions(e.g 2.9.1) as well. I did find a bug with get_version_tuple though(LP:1896826) due to the MAAS version containing an epoch now.

> Whenever reading data from a remote client and server you should assume the
> data is bad. It shouldn't matter to MAAS if the version is float(2.8) or
> str(2.8), the data is the same. Thats why people like strongly typed languages
> for things like this :)

Sure, that is reasonable but I think it isn't a bad idea to have a quick check in here to check that things are done roughly correctly. It is very easy to make the typo in YAML, I have done it several times already.

I can remove it if you prefer.

>
> > Whenever reading data from a remote client and server you should assume the
> > data is bad. It shouldn't matter to MAAS if the version is float(2.8) or
> > str(2.8), the data is the same. Thats why people like strongly typed
> languages
> > for things like this :)
>
> Sure, that is reasonable but I think it isn't a bad idea to have a quick check
> in here to check that things are done roughly correctly. It is very easy to
> make the typo in YAML, I have done it several times already.
>
> I can remove it if you prefer.

Although, thinking about it a bit more MAAS doesn't do this in general yet with SimpleStreams. Errors will quickly happen if the stream doesn't match the expected structure and/or have the expected keys.

lp:maas-images doesn't do much checking I'm not opposed to adding it but we should validate the format as well. Using

re.search('\d+\.\d+\.\d+', release_notification["maas_version"])

is a bit better because not only does it validate that its a string but a string in the format you expect. Without the regex you could set the version to "Dougal" :)

> lp:maas-images doesn't do much checking I'm not opposed to adding it but we
> should validate the format as well. Using
>
> re.search('\d+\.\d+\.\d+', release_notification["maas_version"])
>
> is a bit better because not only does it validate that its a string but a
> string in the format you expect. Without the regex you could set the version
> to "Dougal" :)

Sure, I can add that. I was mostly aiming to catch my own typos and YAML so often has strings without quotes that it is easy to do. This forces a bit more structure which is nice.

Thanks for the fixes. This looks good but I would wait on landing this until you land the MAAS patches incase you have to change something.