Comment 4 for bug 2011477

The attach operation doesn't work on non-LTS (tested kinetic, probably affects lunar too). The team is on it and a bug will be filed still.

Other comments I have are:

a) There is a change that says that unattached status sends virt type to the contract server now. Is that something we should consider updating some GPDR document about? Specially since it's in the unattached state

b) The logging change to json. Any particular reasoning for this? To help a developer story? Or a user story? The way this is now, an existing ubuntu-advantage.log file will end up with a mix post-upgrade: first portion will be normal text, and after the upgrade, it will be json. I'm not aware of log parsers looking at this, but maybe it's something to consider. I wondered about forcing a logrotate in postinst, so at least you would not have a mix of formats in the same log. Thoughts?

c) In esm-counts.cc, line 135, there is a cleanup of the apt config except if the config key is Acquire. In other places in the project, this check was replaced with an anchored regexp, so maybe this is something you will want to do here too in a later release.

d) Use of dpkg --compare-versions "lt" versus "lt-nl". The current postinst uses "lt" in a few places where the intention is to catch versions earlier than $now. When you use "lt", an empty "$2" will be treated as an earlier version, so a check like "$2" lt "27.14~" will always be true on fresh installs, since "$2" will be empty. I didn't review all such calls, only the new one for 27.14, and the functions it calls seem to behave fine in either case. In some other places in the postinst you do use lt-nl, so maybe it's on purpose that you are using "lt" this time, for some scenario I didn't see?

e) There is a migration of "notices" going on in postinst, but only of reboot notices. I wonder which other notices could be there that we are not migrating?

f) In postinst, in case the migration of the config file fails, there is a grep for "ua_config:". If it's still there in the config, then the migration failed, and additional instructions are given (besides what the python script might have printed already). Perhaps that grep should be anchored, like "^ua_config:", but I think this is minor. I was thinking maybe there could be a line like "#ua_config:", and this grep would catch it, but if the migration fails, there will definitely be *also* a ^"ua_config:", if I understood this correctly.