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

Proposed by Dougal Matthews
Status: Merged
Approved by: Dougal Matthews
Approved revision: 6533444328d018f6f557966d9b185f1ae69f5872
Merged at revision: 4ae63c7c2530030e10bed0f019b2ad807db3785e
Proposed branch: ~d0ugal/maas-images:release-notifications
Merge into: maas-images:master
Diff against target: 155 lines (+92/-4)
3 files modified
README (+21/-3)
conf/release-notifications.yaml (+9/-0)
meph2/commands/mimport.py (+62/-1)
Reviewer Review Type Date Requested Status
Lee Trager (community) Approve
Review via email: mp+387927@code.launchpad.net

Commit message

Add Release Notification stream

This stream will be used by MAAS to create notifications for users that
are not running the latest version.
This stream can be imported with the following command;

    meph2-util import conf/release-notifications.yaml release-notifications.d

The --no-sign flag can be added to the above command to disable gpg
signing, which may be needed for local testing.

To post a comment you must log in.
Revision history for this message
Lee Trager (ltrager) wrote :
Download full text (4.1 KiB)

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...

Read more...

Revision history for this message
Lee Trager (ltrager) wrote :

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.

review: Needs Fixing
Revision history for this message
Lee Trager (ltrager) wrote :

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/

Revision history for this message
Lee Trager (ltrager) wrote :

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

review: Needs Fixing
Revision history for this message
Dougal Matthews (d0ugal) wrote :

> 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

Revision history for this message
Dougal Matthews (d0ugal) wrote :

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

Revision history for this message
Dougal Matthews (d0ugal) :
Revision history for this message
Lee Trager (ltrager) wrote :

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

Revision history for this message
Dougal Matthews (d0ugal) :
Revision history for this message
Lee Trager (ltrager) wrote :

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.

Revision history for this message
Dougal Matthews (d0ugal) wrote :

> 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.

Revision history for this message
Dougal Matthews (d0ugal) wrote :

>
> > 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.

Revision history for this message
Lee Trager (ltrager) wrote :

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" :)

Revision history for this message
Dougal Matthews (d0ugal) wrote :

> 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.

Revision history for this message
Lee Trager (ltrager) wrote :

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.

Revision history for this message
Lee Trager (ltrager) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/README b/README
2index df2a9ab..ba8feed 100644
3--- a/README
4+++ b/README
5@@ -29,7 +29,7 @@ example:
6
7 This will build from source-image into output.d for 'arch' and 'release'.
8 The 'version' provided is a serial (YYYYMMDD such as 20160101).
9-source-image can be either a url or a local file of type
10+source-image can be either a url or a local file of type
11 '.tar.gz', '-root.tar.gz', or '-root.tar.xz' or '.img'
12
13 Note, using conf/meph-v2.yaml will build the v2 format stream
14@@ -149,7 +149,7 @@ Likely that label is 'release'.
15 Cleaning is done in 2 parts:
16 * cleaning meta-data (removing entries from the products files)
17 * finding orphans: identify which files in the tree are no longer referenced.
18- * purging orphans: removing files that are known to have been orpaned for
19+ * purging orphans: removing files that are known to have been orphaned for
20 a given amount of time (3 days).
21
22 === cleaning meta-data ===
23@@ -210,7 +210,7 @@ More complex usage:
24 === cleaning / reaping orphans ===
25 Reaping orphans is what actually removes files. There is a '--dry-run' to
26 indicate what would be done.
27-
28+
29 its usage is fairly simple.
30
31 $ meph2-util reap-orphans --older 7d my-orphans.json ./data/
32@@ -246,3 +246,21 @@ Trusty images are built like this:
33 - v3:
34 meph2-cloudimg-sync --config=conf/meph-v3.yaml -vvv out.d \
35 arch=$arch release=trusty
36+
37+
38+=== Release Notifications ===
39+
40+The release notification stream is used to let MAAS know when new versions have
41+been released so users can then be informed.
42+
43+This is a bit of a non-standard way of using simplestreams as we only want to
44+deliver the metadata and no files. There is a requirement for a file however,
45+so we do include a small token file with no real content.
46+
47+There will only ever be one release notification, as there is only ever one
48+latest version of MAAS. To insert a new release notification into the stream
49+the config file in `conf/release-notifications.yaml` should be updated to
50+reflect the new MAAS release and the message we want to show users running a
51+older version. The `version: 1.0` field should stay the same and should only
52+be incremented if we changes to the release notifications and want older MAAS
53+versions to ignore it.
54diff --git a/conf/release-notifications.yaml b/conf/release-notifications.yaml
55new file mode 100644
56index 0000000..35f209f
57--- /dev/null
58+++ b/conf/release-notifications.yaml
59@@ -0,0 +1,9 @@
60+product_id: "com.ubuntu.maas:candidate:1:maas-notification"
61+content_id: "com.ubuntu.maas:candidate:1:maas-notification"
62+
63+release-notification:
64+ version: 1.0
65+ maas_version: "2.8.0"
66+ message: >
67+ <strong>We’ve released MAAS 2.8.</strong> This version supports LXD VM hosts, faster UI and many bug fixes.
68+ Find out <a href="/docs/release-notes">what’s new in 2.8</a>.
69diff --git a/meph2/commands/mimport.py b/meph2/commands/mimport.py
70index 6344d3f..e7c5c0f 100644
71--- a/meph2/commands/mimport.py
72+++ b/meph2/commands/mimport.py
73@@ -203,6 +203,64 @@ def import_bootloaders(args, product_tree, cfgdata):
74 'items': items
75 }
76
77+def import_release_notifications(args, product_tree, cfgdata):
78+ product_id = cfgdata["product_id"]
79+ release_notification = cfgdata['release-notification']
80+
81+ # Simple check to ensure the maas_version is a string. It is very easy to
82+ # typo and write it as a float.
83+ if (not isinstance(release_notification["maas_version"], str)
84+ or not re.match('\d+\.\d+\.\d+', release_notification["maas_version"])):
85+ raise ValueError(
86+ "maas_version should be a string with the full SemVer version. for example: '2.9.1'")
87+
88+ if product_id in product_tree["products"]:
89+ versions = product_tree['products'][product_id]['versions']
90+ if release_notification == versions[max(versions.keys())]:
91+ return
92+ else:
93+ versions = {}
94+ product_tree["products"][product_id] = {
95+ "name": "release-notifications",
96+ "label": "candidate",
97+ "versions": versions,
98+ "arch": "all",
99+ "release": "notifications",
100+ }
101+
102+ today = datetime.utcnow().strftime('%Y%m%d')
103+ point = 0
104+ while True:
105+ version = "%s.%d" % (today, point)
106+ if version not in versions:
107+ break
108+ else:
109+ point += 1
110+
111+ notification_dir = os.path.abspath(os.path.join(
112+ os.path.dirname(__file__), '..', '..', 'release-notifications'))
113+
114+ version_dir = os.path.join(os.path.realpath(args.target), "release-notifications/{}/".format(version))
115+ versioned_notification = os.path.join(version_dir, "release-notification.yaml")
116+ os.makedirs(version_dir, exist_ok=True)
117+
118+ with open(versioned_notification, "w") as f:
119+ f.write("This file is unused.")
120+ with open(versioned_notification,"rb") as f:
121+ hash = hashlib.sha256(f.read()).hexdigest();
122+
123+ path = "release-notifications/{}/release-notification.yaml".format(version)
124+ item = {
125+ "ftype": "notifications",
126+ "path": path,
127+ "sha256": hash,
128+ "size": os.stat(versioned_notification).st_size,
129+ "release_notification": release_notification,
130+ }
131+
132+ product_tree['products'][product_id]["versions"][version] = {"items": {
133+ "release_notification": item
134+ }}
135
136 def get_image_index_images(url):
137 """ Given a URL to an image-index config file return a dictionary of
138@@ -476,7 +534,7 @@ def import_packer_maas(args, cfgdata):
139 with open(os.path.join(args.target, target_product_stream), 'wb') as fp:
140 fp.write(util.dump_data(product_tree))
141
142-
143+
144 def main_import(args):
145 cfg_path = os.path.join(
146 os.path.dirname(__file__), "..", "..", "conf", args.import_cfg)
147@@ -505,6 +563,9 @@ def main_import(args):
148 import_remote_config(args, product_tree, cfgdata)
149 elif cfgdata.get('bootloaders') is not None:
150 import_bootloaders(args, product_tree, cfgdata)
151+ elif cfgdata.get('release-notification') is not None:
152+ product_tree["datatype"] = "release-notifications"
153+ import_release_notifications(args, product_tree, cfgdata)
154 else:
155 sys.exit('Unsupported import yaml!\n')
156

Subscribers

People subscribed via source and target branches