Support for pushing notifications to the Ubuntu ISO Tracker #81

sil2100 · 2022-01-17T13:19:35Z

Sending this out for an initial review. The goal of this PR is to send out notifications to the Ubuntu ISO Tracker about any new WSL builds that are completed, notifying the respective per-series products. This way we'd have Ubuntu WSL images appearing on the tracker automatically. It also plays in nicely with another feature of this related to the ability to re-trigger builds via the tracker.

http://iso.qa.ubuntu.com/

This is the first time I deal with github actions, so that part is mostly written blindly by just looking at other sections and reading documentation. There's also some open questions here.

Open questions:

Version of the resulting WSL build. Right now I went with the simple route of just using build_id, but from what I see it's like a small number right now. Ideas I had was things like DATESTAMP.build_id or something, or does build_id suffice?
Not sure if the ISO Tracker API will work over HTTPS (which we need as it's outside of Canonical network)
I have no idea how github actions are triggered and how long the environment survives, but in the code I used /tmp/all-releases.csv in case that it might 'stick around' from previous job runs. I can switch to just doing it via LP directly, but if it's persistent for long enough I thought it might save a few LP API calls.

Other than that I think it's good enough. I only tested it in dry-run mode on my local machine. I'm open to suggestions!

…O Tracker.

didrocks

I think this is a nice first version, especially from someone not familiar with Github Action. Nice work!

I have suggested some improvements and changes I consider needed for this. Also, to answer your questions:

Version of the resulting WSL build. Right now I went with the simple route of just using build_id, but from what I see it's like a small number right now. Ideas I had was things like DATESTAMP.build_id or something, or does build_id suffice?
You should use the complete version after our replacement during the build. This one takes into account which image we are pushing to (like 2004.4.<build_id>.0) for instance. Note that we don’t have control over the last .0 which is internal to the Microsoft Store. This whole version will be. after the first builds, something like 2204.0.1.0 for instance.

Not sure if the ISO Tracker API will work over HTTPS (which we need as it's outside of Canonical network)*
Yeah, can you give it some tries with curl directly from your machine without the VPN connected?

I have no idea how github actions are triggered and how long the environment survives, but in the code I used /tmp/all-releases.csv in case that it might 'stick around' from previous job runs. I can switch to just doing it via LP directly, but if it's persistent for long enough I thought it might save a few LP API calls.
Those are sticking per job. So, if one step of a job stick a file somewhere, it will be available for every other steps in the job. However, we can also upload/download artefacts to share things between jobs (you have some examples in the build process here). However, I think you should really take in general the version we build in the XML file and use that as a reference to avoid any discrepancies between the 2.

.github/workflows/build-wsl.yaml

didrocks · 2022-01-19T07:14:43Z

.github/workflows/build-wsl.yaml

+          ISOTRACKER_USERNAME: ${{ secrets.ISOTrackerUsername }}
+          ISOTRACKER_PASSWORD: ${{ secrets.ISOTrackerPassword }}
+        run: |
+          perl -pe 's/@([A-Z_]+)@/$ENV{$1}/eg' isotracker.conf.in >~/isotracker.conf


Can’t we do that with sed? Because perl will make it harder to maintain :p
I would like the configuration to be store somewhere not in workdir if possible.

I can look into doing it with sed ;p The perl version was straightforward as it could be written in one line, in case there's more credentials at some point. But that's rather unlikely!
As for the placement of the config file - I think this would require some changes to the isotracker module as by default it hard-codes this path for the config file. But I think this should be configurable, so I'll make the required modifications!

.github/workflows/build-wsl.yaml

didrocks · 2022-01-19T07:17:18Z

.github/workflows/build-wsl.yaml

@@ -252,6 +266,15 @@ jobs:
          name: build-artifacts-${{ matrix.WslID }}
          path: ${{ env.workDir }}/${{ env.buildInfoPath }}/${{ matrix.WslID }}-*

+      - name: Notify ISO Tracker


Same, I would do that after updating the build artefacts.

Actually, thinking about it, we are only interested when we store the version in the "db" (yes, the wiki). So maybe another approach would be to do it in update-build-artifacts job rather, for all new application packages (==images) we uploaded to the store.

didrocks · 2022-01-19T07:19:53Z

isotracker.conf.in

+url=https://iso.qa.ubuntu.com/xmlrpc.php
+username=@ISOTRACKER_USERNAME@
+password=@ISOTRACKER_PASSWORD@
+default_milestone=Jammy Daily


This needs to be reworked. We avoided hardcoding anything (version, name…) in our repo, so that if a new release opened, we just receive an automated PR from our detect-update-releases workflow and we can review + merge.

This makes opening/tracking new releases painless compared to 60 places we need to update the name (and of course, forgot some in the wiki).
Can we retrieve the information from launchpad, or update maybe the release.csv file and add + " Daily" to it?

hm, you're right. Maybe we could actually make this super dynamic. A few ideas is: either generating the isotracker.conf file automatically in notify-tracker before creating the ISOTracker object or, maybe even better: tweaking ISOTracker in ubuntu-archive-tools to offer some functionality to pass the credentials and config data without the need of a file on the filesystem.

Anyway, good idea! I'll get that going.

sil2100

Thank you for your review Didier! I'll be taking care of those issues as soon as I have a moment.

sil2100 · 2022-01-26T11:31:20Z

.github/workflows/build-wsl.yaml

+          ISOTRACKER_USERNAME: ${{ secrets.ISOTrackerUsername }}
+          ISOTRACKER_PASSWORD: ${{ secrets.ISOTrackerPassword }}
+        run: |
+          perl -pe 's/@([A-Z_]+)@/$ENV{$1}/eg' isotracker.conf.in >~/isotracker.conf


I can look into doing it with sed ;p The perl version was straightforward as it could be written in one line, in case there's more credentials at some point. But that's rather unlikely!
As for the placement of the config file - I think this would require some changes to the isotracker module as by default it hard-codes this path for the config file. But I think this should be configurable, so I'll make the required modifications!

sil2100 · 2022-01-26T11:36:41Z

isotracker.conf.in

+url=https://iso.qa.ubuntu.com/xmlrpc.php
+username=@ISOTRACKER_USERNAME@
+password=@ISOTRACKER_PASSWORD@
+default_milestone=Jammy Daily


hm, you're right. Maybe we could actually make this super dynamic. A few ideas is: either generating the isotracker.conf file automatically in notify-tracker before creating the ISOTracker object or, maybe even better: tweaking ISOTracker in ubuntu-archive-tools to offer some functionality to pass the credentials and config data without the need of a file on the filesystem.

Anyway, good idea! I'll get that going.

…racker configuration.

sil2100 · 2022-01-31T14:14:33Z

Pushed a new version, ready for review.

In this version, as per discussion, I have switched to the approach of generating the ISOTracker config on the fly dynamically without actually creating a configuration file (pushed support for that to lp:ubuntu-archive-tools). I reorganized some of the github actions to have a better order too.

I tried a bit to move the notification logic to update-build-artifacts as you suggested and even had an idea how to do it (calling git-tree for HEAD to get the list of changed files to determine which projects got updated), but in the end decided to go with how it was before as I am not as confident in github actions logic. Since this then requires having the WSL repository available, and from the looks of it the update-build-artifacts job was working in the wiki repository directory. And I was like, eh, I don't feel comfortable enough to twiddle with things like these without having any way of testing this.

So yeah, I'd say we can switch to the other approach, but it feels like more invasive then?

didrocks · 2022-02-08T14:58:04Z

Thanks for the changes here and not relying on the hardcoded ISO tracker configuration anymore. It looks good to me :) However, it seems you didn’t remove the hardcoded version from your branch.

On the other big comment, I’m still convinced that the best approach is to do in update-build-artifacts workflow:

there are some use cases for manual builds, without uploading to the store. In that case, the iso tracker should not be notified.
this whole logic of notifying the iso tracker is the same than "should we update our version database"? So, it means that if this job is running and we have a diff here, we had the corresponding desired upload done to the store for that version already. This way, we are sure our logics are aligned.
I think this can be easily done by calling git status in a job just after "Copy modified artifacts to base wiki", determining which file in which folder have been changed, and using this to know what to notify. If you need any additional metadata, I’m happy to give some hands here :)

didrocks

Moving it back as we need some changes, the overall changes looks like it’s going in the right direction though :)

…d-artifacts stage.

sil2100 · 2022-02-11T13:04:58Z

Could you take a look if this is something that would work? Thanks!

didrocks

Some small change, but it seems we are getting there! Thanks for the discussion :)

didrocks · 2022-02-11T13:13:00Z

.github/workflows/build-wsl.yaml

+            WSLId=$(basename $build);
+            WSLId=${WSLId%-buildid.md};
+            buildid=$(cat $build)
+            PYTHONPATH=${{ env.codeDir }}/uat ${{ env.codeDir }}/notify-isotracker $WslID $buildid


Can you move notify-isotracker under wsl-buidler/? This will make buid-related stuff outside of WSL.

didrocks · 2022-02-11T13:14:00Z

.github/workflows/build-wsl.yaml

@@ -286,3 +288,29 @@ jobs:
          git add .
          git commit -m "Auto-update build info"
          git push origin master
+
+      - name: Checkout WSL repo


Maybe add the if: ${{ steps.modified-artifacts.outputs.needs-wiki-update == 'true' }} in there and do the 2 clones in a single step, as both is to get the code to notify the tracker.

sil2100 · 2022-02-11T14:20:49Z

Almost there! But ready for re-review once again!

I suppose one final thing to think about (poked JB a while ago about that as well) is actually how we should identify new builds in the ISO Tracker. Right now it's the buildid, but the ISOTracker would have to somehow construct a nice way of offering something to download using that ID.

didrocks · 2022-02-11T15:25:54Z

Agreed, do you want that we resolve this next week before merging?

sil2100 · 2022-02-18T10:53:22Z

hm, okay, I feel that I'd need help getting a better understanding here. So from what JB told me a while ago, one way of 'accessing' WSL downloads is via pointing to the sideload download urls from build jobs, e.g.:
https://github.com/ubuntu/WSL/actions/runs/1852204709

The ISO Tracker url creations is rather simple and 'stupid'. It basically needs to construct the url using simple substitution only via the build_id, project, arch etc. Nothing complex can be done, just substituting variables in the url. In this case, it feels to me we'd have to send the WSL action run ID to the ISOTracker, then we could at least link to the action url for people to fetch the sideload archives.

Is it possible to get the run ID somehow? Or do we need to move this to some different stage then?

sil2100 · 2022-02-22T16:21:04Z

Should now be again good for review. As per our earlier discussion, switched to using the GITHUB_RUN_ID. Hope this is correct! Thank you @didrocks for helping out on all the stages of this PR!

didrocks · 2022-02-23T10:48:43Z

That should be it! If you don’t mind, let’s merge that after .4 and the multiple renaming we have to ensure we don’t add another point of failure :) Otherwise, looking good for me, thanks sil2100!

didrocks · 2022-03-18T10:33:03Z

Naming transitions are now completed. Let’s merge that. Thanks for working on it!

Initial version of support for pushing notifications to the Ubuntu IS…

c533665

…O Tracker.

didrocks requested changes Jan 19, 2022

View reviewed changes

sil2100 commented Jan 26, 2022

View reviewed changes

Some tweaks to the github actions + support dynamically creating ISOT…

a89f67f

…racker configuration.

sil2100 requested a review from didrocks February 7, 2022 11:03

didrocks requested changes Feb 8, 2022

View reviewed changes

sil2100 added 2 commits February 10, 2022 13:50

Ooops, remove leftover file of the tracker config.

e64e79a

First attempt on doing the ISOTracker notification at the update-buil…

c52c628

…d-artifacts stage.

didrocks requested changes Feb 11, 2022

View reviewed changes

Move notify-isotracker under wsl-builder/, combine two steps into one.

b3856de

Use the github runid for identifying the WSL builds.

3d02827

sil2100 requested a review from didrocks February 23, 2022 09:47

didrocks approved these changes Feb 23, 2022

View reviewed changes

didrocks merged commit b9056ff into ubuntu:main Mar 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for pushing notifications to the Ubuntu ISO Tracker #81

Support for pushing notifications to the Ubuntu ISO Tracker #81

sil2100 commented Jan 17, 2022

didrocks left a comment

didrocks Jan 19, 2022

sil2100 Jan 26, 2022

didrocks Jan 19, 2022

didrocks Jan 19, 2022

sil2100 Jan 26, 2022

sil2100 left a comment

sil2100 Jan 26, 2022

sil2100 Jan 26, 2022

sil2100 commented Jan 31, 2022

didrocks commented Feb 8, 2022 •

edited

didrocks left a comment

sil2100 commented Feb 11, 2022

didrocks left a comment

didrocks Feb 11, 2022

didrocks Feb 11, 2022

sil2100 commented Feb 11, 2022 •

edited

didrocks commented Feb 11, 2022

sil2100 commented Feb 18, 2022

sil2100 commented Feb 22, 2022

didrocks commented Feb 23, 2022

didrocks commented Mar 18, 2022

Support for pushing notifications to the Ubuntu ISO Tracker #81

Support for pushing notifications to the Ubuntu ISO Tracker #81

Conversation

sil2100 commented Jan 17, 2022

didrocks left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sil2100 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sil2100 commented Jan 31, 2022

didrocks commented Feb 8, 2022 • edited

didrocks left a comment

Choose a reason for hiding this comment

sil2100 commented Feb 11, 2022

didrocks left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sil2100 commented Feb 11, 2022 • edited

didrocks commented Feb 11, 2022

sil2100 commented Feb 18, 2022

sil2100 commented Feb 22, 2022

didrocks commented Feb 23, 2022

didrocks commented Mar 18, 2022

didrocks commented Feb 8, 2022 •

edited

sil2100 commented Feb 11, 2022 •

edited