I was reviewing the proposed upload against the admittedly outdated exception. While I highly appreciate that the exception is being reworked, and getting much more complete, and removing old cruft - I agree that in the short term for the current upload we might want to go by the old exception to not stall even more.
To make sense of all of it I sat together with Ernest as by chance due to my sprinting he has been nearby. We have been cross Q&A-ing all kind of details, to ensure the testing today is equal or better (usually it is the latter) to what the original exception required. We also discussed about the general stance of long term stability of behavior, which I gladly learned is part of their team's approach anyway. There can always be issues, but at least they are conceptually far away from any "just add things" thinking. We've dived into the testing that is done and I appreciate outlining so much in the bug description - looking forward to the new exception making this line up well. We identified a few weak spots which he took as action to resolve for the current case as well as for the renewed exception. While for this upload what is missing will be added manually, but there has been great agreement like utilizing the autopkgtest more to get better cross arch coverage and to get better notice of apparmor/system influences. All that will eventually help to make these uploads more smooth. All I've found gave me quite good confidence in the general approach, the coverage they already have and to extend where it is needed to feel truly safe for an SRU of that scale. What was left and will in the future be part of the automation and the new SRU exception is listed in the following section which Ernest will follow up on. As lessons learned from MIR rules, I give them numbers so he can refer to them when providing the related info - this is not an order or priority. Next I checked the state of the archive, all is at 2.66.1 right now which means we do not need to look at 2.66 -> 2.67 but only 2.66.1 -> 2.67. Furthermore together we ensured I understand the sanitazion and mapping from commits to changelog and to release notes. What they do is actually really great but hard to follow if you do not know why/how. In the future their process will provide the artifact (spreadsheet) of that journey so one can more easily follow. Next I made sure that what I compare in git from https://github.com/snapcore/snapd.git was comparable to the upload in the PPA. I've confirmed that the only differences that mattered have been vendoring (in the package, not the repo) and some dotfiles which are stripped when building the package. Furthermore I checked if the further vendored code has brought any changes we also would need to review, but there are none. With that done I was reviewing the 167 commits, which of which the majority 44% are tests for those new things or extending existing tests. What is left is an equal amount of ~14% interface and component code (the actual development) and about 6% bugfixes. The rest have been smaller features which I'd not list individually. Those I was not reviewing with a POV of "is this go code nice" which they have done many times by arriving here. But more as we did in the pro client, would these changes maybe hit/trigger something in Ubuntu they might have missed to think about - because with that POV the sponsoring provides actual value and not just doing the same a third time. The per commit review presented me with - As expected many tests, for which I mostly ensures they really are just tests. - Apparmor changes around interfaces which IMHO are fine - and furthermore do not add general allowance, but are tied to these particular interfaces. - New interface to the debug API, fine not changing the old. - New options like get --pristine, again not changing existing behavior. - ... many more, I won't rewrite the changelog, but AFAICS none changed existing behavior in a negative way (I count e.g. not hanging on snap kill a fix and not a behavior change). - The two big newer set of changes have been around apparmor prompting and supporting components. - A FIPS change made me wonder, but we checked and that is for special builds not relevant to the build done in the Ubuntu archive. - Registry related things added are behind a default off feature flag. - The changed emmc handling is not relevant for snapd on non Ubuntu-core AFAICS. - The factory reset functions changed defaults now using some of the new. mount options, but that again is not relevant on non Ubuntu-core. - Changes to the mount options of gadget snaps is yet again Ubuntu-core only. Areas of potential regressions: I've not seen an issue in these, but in regard to SRUs we always want to identify where regressions most likely might occur so we can have an extra look and identify them quickly: - overlayfs options in general, more mount options like noexec nodev usage, dm-verity and hybrid recovery all are new options added and therefore should not change behavior when used as-is, but they caused enough churn in the code about bootstrap/mounting, thereby this is an area to watch for carefully - Interfaces allowing more is good if it was blocking functions before like the changes for interface daemons. But that will change behavior, hopefully only in the positive intended way but a thing to watch out for seems to be its run in KDE plasma based environments and desktop sessions in general. - Locking was reworked with unblockers for some paths to avoid deadlocks, which is perfectly fine but adds to waitch out for potential concurrency conflicts between snapd, snap-confine and snap-update-ns to the list to watch out for. - The arrival of components change the sideloading (install from local file) quite a bit, gladly plenty of tests got added too. Still install from file is an area to watch out for too. - Refreshing should have no external behavior change, but internally changes the api used - so yet again worth t obe aware in case of regressions. - Better handling of cgroup v2 memory controller is nice, but yet again an area to watch out for unintended behavior change in case regressions get reported Out of all of the above the following details shall be added for this case. I was reviewing this quite some days and told Ernest about findings every now and then, due to that the timing of me asking and him adding sometimes is odd. For example he already clarified which architectures were covered by the automatic and QA tests. There is more which Ernest and I found which will be added to their process and the updated policy - but for now and here just what we need for this upload. Requests to Ernest/snapd: 1. coverage of non-x86/non-arm64. Yes I know that this will be extended properly in the future, but for now I ask for semi manual e.g. run unit tests on s390x/ppc64 and add logs here. When: Does not block sponsoring nor accepting to proposed, but releasing it. 2. Outline a bit more detail of what Cert and QA Deb testing will exactly cover, in particular please ensure it has (and add logs) what the old exception asked for: 2.1 apt install/upgrade snapd packages 2.2 post upgrade reboot and post reboot apt install tests 2.3 interactions with gnome software center 2.4 release upgrade after snapd has been upgraded When: Does not block sponsoring nor accepting to proposed, but releasing it. 3. After our discussion Ernest was already referencing the associated bugs in the changelog. Please update these bugs descriptions with the SRU templates as we discussed to allow their individual verification once in proposed. Ernest already mentioned that this is on his TODO-list. When: Does not block sponsoring, but accepting it to proposed For future proposed uploads you can think about creating the list I've added as "Areas of potential regressions" yourself, as you've by now seen the awareness of potential regression areas is part of the usual template an SRU paperwork would specify. Combined with the great evidence we already have that IMHO pleases all requirements of the old and quite some of the future even better exception. It makes me feel comfortable to sponsor the currently proposed versions. I'll now (after meetings actually) check the new PPA content (since the changelog was updated due to my request) to still match git and if so then sponsor 2.67. Once you have fulfilled one of my requests you could give the bug a ping/update comment as SRU members will likely look out if you have provided all these #3 to accept to -proposed and #2/#1 to release to -updates. P.S. I've also had a look at 2.67..2.67.1 which is fine as well, opening up a few more accesses and restricting "." usage in apparmor rules. The latter would be another potential regression risk, but unlikely. Ernest mentioned that they do not have the testing on 2.67.1 ready, hence I should sponsor 2.67 - but if you have that ready before more people spend more time e.g. while you follow up my requests above please ask to sponsor that instead saving everyone yet another tour for 2.67.1 later :-) ** Changed in: snapd (Ubuntu Focal) Status: New => Triaged ** Changed in: snapd (Ubuntu Jammy) Status: New => Triaged ** Changed in: snapd (Ubuntu Oracular) Status: New => Triaged ** Changed in: snapd (Ubuntu Plucky) Status: New => Triaged ** Changed in: snapd (Ubuntu Noble) Status: New => Triaged -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2089691 Title: [SRU] 2.67 To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/snapd/+bug/2089691/+subscriptions -- ubuntu-bugs mailing list [email protected] https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
