snapcraft: Fix *_ORIG variables

[Chris-Peterson444]

Ensure subiquity-server and ubuntu-desktop-bootstrap apps have PATH_ORIG, PYTHON_ORIG, and PYTHONPATH_ORIG environment variables. Lacking these causes usage of subiquity's system_scripts to fail to find the right python outside of the snap. This is currently causing installs to fail on daily desktop-builds.

Also remove LD_LIBRARY_PATH_ORIG since it's not needed anymore.

The rational for removing LD_LIBRARY_PATH is mostly that we don't need it. However, after injecting a new snap to test these changes I found subiquity struggling to replace LD_LIBRARY_PATH_ORIG because LD_LIBRARY_PATH wasn't set. LD_LIBRARY_PATH_ORIG is empty, which means the control flow in the environment cleaner function executes this line which cause a KeyError because LD_LIBRARY_PATH is unset (trying to delete a non-existent key). Strangely this error isn't present in an unmodified ISO. This leads me to believe that the way I'm building the snap is slightly different than the launchpad setup. (I can download the snap and inject it without encountering the issue.) Regardless, we can remove LD_LIBRARY_PATH_ORIG because we're always going to remove it from the cleaned environment anyways.

LP: #2082851

[dbungert]

Does APPORT_DATA_DIR need to be set?

[Chris-Peterson444]

I didn't run into that issue in particular this time around but we might as well. @d-loose will this affect any of the apport reporting u-d-b does, if any?

[Chris-Peterson444]

I also want to mention on the LD_LIBRARY_PATH issue that I was able to reproduce the problem with no changes to the snapcraft yaml and just building the snap locally. My bet right now is that the variable exists empty in the launchpad build environment, purposefully or not, where my local builder doesn't. So to call this fully resolved I'd like to download the version that the launchpad builder generates and test with that.

[d-loose]

I didn't run into that issue in particular this time around but we might as well. @d-loose will this affect any of the apport reporting u-d-b does, if any?

It currently doesn't do any apport reporting, due to the problems described in #770. Instead, we added a custom error message with instructions for the user to manually run ubuntu-bug (#789). See also UDENG-3172 on JIRA.
So I think this doesn't have any consequences for the UI.

[d-loose]

I also want to mention on the LD_LIBRARY_PATH issue that I was able to reproduce the problem with no changes to the snapcraft yaml and just building the snap locally. My bet right now is that the variable exists empty in the launchpad build environment, purposefully or not, where my local builder doesn't. So to call this fully resolved I'd like to download the version that the launchpad builder generates and test with that.

Which snapcraft version did you use for your local build? There is a known issue with snaps built using snapcraft 8.x (which is why snapcraft is pinned to 7.x/stable here and here). I think @seb128 was looking into this, but I'm not sure if he had the time to make any progress on this. In any case we should document this more clearly.

[Chris-Peterson444]

Ahh, I was using 8.4.1. Thanks for pointing that out.

So using the version from 7.x/stable (I got 7.5.6) I was able to get around the difference in behavior. However, it also broke my proposed solution. $PATH_ORIG now becomes:

/snap/ubuntu-desktop-bootstrap/<revision>/usr/sbin:/snap/ubuntu-desktop-bootstrap/<revision>/usr/bin:/snap/ubuntu-desktop-bootstrap/<revision>/usr/sbin:/snap/ubuntu-desktop-bootstrap/<revision>/usr/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/usr/games:/usr/local/games:/snap/bin

Note the doubling of the $SNAP/usr/sbin:$SNAP/usr/bin prefix, even though it's only defined as PATH: $SNAP/usr/bin:$SNAP/bin:$PATH in the ubuntu-desktop-bootstrap definition.

I got to thinking that maybe the ubuntu-server app's environment variables are set in the context of ubuntu-desktop-bootstrap's already being set, but removing the $PATH_ORIG definition in subiquity-server results in the variable not being set at all. So it seems that $PATH is already set to the snap path when that environment gets defined. Usage of $PATH_ORIG isn't going to help us here. I'm getting convinced this _ORIG trick isn't worth it anymore. Going forward:

1. I would like to fix the cloud-init bug in Subiquity instead of here. I think we should eventually move away from _ORIG usage entirely and do some proper environment handling in the code. I have a solution in mind for the short term so this is fixed for release, but Subiquity should come up with other ways to getting a system environment.
2. We could optionally still land this PR. Or at the very least, I would still like to propose adding the APPORT_DATA_DIR variable. I didn't experience it with this bug, but the lack of APPORT_DATA_DIR has broken some bug reporting in subiquity and it may do so in the future. As for the others: PYTHONPATH_ORIG and PYTHON_ORIG do get set correctly and would bump us in the right direction in the short term until we do some proper environment handling. Removing the LD_LIBRARY_PATH_ORIG is a no-op for now but will stop the bug from presenting once u-d-b starts building with snapcraft 8.x/stable.

[Chris-Peterson444]

Actually I just found a bug report in the wild with the APPORT_DATA_DIR issue. Let's definitely fix that: https://bugs.launchpad.net/ubuntu/+source/subiquity/+bug/2083067

[Chris-Peterson444]

So I did a bunch of testing and I still can't figure out exactly why or how $PATH is set to include the /snap paths by the time the subiquity-server environment variables are defined. Instead of doing some hacking in subiquity to fix this, I have a renewed proposal:

1. Set PATH_ORIG manually to what we expect it to be (the documented default for classic confinement snaps[0]) in both apps: /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games

2. Set PYTHON_ORIG and PYTHONPATH_ORIG to their original values before changing them.

3. Set APPORT_DATA_DIR and APPORT_DATA_DIR_ORIG to fix bug reports like LP: #2083067

4. Unset LD_LIBRARY_PATH_ORIG to preempt the bug when moving to snapcraft 8+

[0] https://snapcraft.io/docs/environment-variables#heading--path

[d-loose]

Great, thanks a lot! I've tested this with snapcraft 7.5.6 and 8.4.1 - the snap builds without errors in both cases and works in the live session.

@seb128 I'd appreciate it if you could have a quick look as well :)

[seb128]

Seems reasonable to me, thanks for the work Chris!