Bug 4885 - Confusing package structure/separation in server
Summary: Confusing package structure/separation in server
Status: CLOSED FIXED
Alias: None
Product: ThinLinc
Classification: Unclassified
Component: Other (show other bugs)
Version: trunk
Hardware: PC Unknown
: P2 Normal
Target Milestone: 4.15.0
Assignee: Pierre Ossman
URL:
Keywords: linma_tester, relnotes, samuel_tester, wilsj_tester
Depends on: 5042
Blocks: 7667 3826 4014 4878 4879 5064 5084
  Show dependency treegraph
 
Reported: 2013-11-05 11:20 CET by Pierre Ossman
Modified: 2023-08-16 14:46 CEST (History)
3 users (show)

See Also:
Acceptance Criteria:
* Common Makefile boilerplate should be clearly separated out so that it is easy to keep this up to date and consistent across the tree * All server module Makefiles should follow a common pattern so things are familiar between them * The new server RPM/deb should have identical contents to the previous setup * The new server RPM/deb should have identical behaviour in trigger script to the previous setup * Upgrading ThinLinc to this new version should not require any extra manual steps from the user


Attachments

Description Pierre Ossman cendio 2013-11-05 11:20:54 CET
The current separation of packages for the ThinLinc server has arisen somewhat organically and there has been some discussion that it is time for a cleanup.

As a first step, we need to decide what the goals of the structure should be. Based on that, suggestions for a solution can be made.

Bugs related to this issue have been set as being blocked by this bug. It is probably not a good idea to work on those bugs until we know what structure we want in the end.
Comment 1 Pierre Ossman cendio 2013-11-05 12:36:03 CET
Goals that have been mentioned (without any regard to importance or support):

a) Structure of packages mimics structure of development in order to avoid a convoluted build system. Also makes it easier for developers to know what goes where.

b) Releasing hot fixes should be easy. I.e. we want to be able to replace small components without having to upgrade unrelated things.

c) The ability to run components independently. E.g. installing just our rdesktop.

d) Easy to install, no matter the method. E.g. by our installer, by hand, by automated systems such as puppet.

e) Familiar structure. I.e. the old "do what everyone else does" so that administrators easily understand how to use our packages.
Comment 2 Pierre Ossman cendio 2013-11-05 12:38:24 CET
Some rough plans that have been mentioned:

1. One monolithic package.

Probably satisfies e) the best, but all the other goals go out the window.
Comment 3 Pierre Ossman cendio 2013-11-05 12:45:34 CET
2. A cleaned up modular design with sane dependencies

So far only ideas have been thrown around:

 - No cyclical dependencies. They screw up puppet and they generally indicate poor modularity anyway.

 - Components should be bundled together based on their strongest dependency. I.e. ship things together that are most likely to have to be updated at the same time in the event of a hot fix.

 - Good modularity in code. Think in the terms of a large company where different teams work on different parts. I.e. minimise dependencies and construct stable APIs

 - Better base package(s). Perhaps a filesystem package is needed.
Comment 5 Pierre Ossman cendio 2022-02-16 13:00:36 CET
We'll go ahead with the suggestion above for this bug. The main goals are listed in acceptance criteria.
Comment 31 Pierre Ossman cendio 2022-02-23 13:35:06 CET
Something is amiss when trying to upgrade to this new unified package on .deb systems.

I've put the following in the new spec file:

> # Pre-merging packages
> Obsoletes: thinlinc-tladm < 4.15.0
> Obsoletes: thinlinc-tlmisc < 4.15.0
> Obsoletes: thinlinc-tlmisc-libs < 4.15.0
> Obsoletes: thinlinc-tlmisc-libs32 < 4.15.0
> Obsoletes: thinlinc-tlprinter < 4.15.0
> Obsoletes: thinlinc-vnc-server < 4.15.0
> Obsoletes: thinlinc-vsm < 4.15.0
> Obsoletes: thinlinc-webaccess < 4.15.0
> 
> # Make sure the deprecated rdesktop package gets removed
> Obsoletes: thinlinc-rdesktop
> # Workaround for dpkg which fails to realise that the dependency from
> # tlmisc on rdesktop goes away as part of the upgrade
> Provides: thinlinc-rdesktop

The last bit is inherited from the existing tlmisc package.

However this does not execute cleanly, giving me this error:

> dpkg: regarding thinlinc-server_4.14.0post-0.unknown_amd64.deb containing thinlinc-server:
>  thinlinc-tlmisc conflicts with thinlinc-rdesktop
>   thinlinc-server provides thinlinc-rdesktop and is to be installed.
> 
> dpkg: error processing archive thinlinc-server_4.14.0post-0.unknown_amd64.deb (--install):
>  conflicting packages - not installing thinlinc-server
> Errors were encountered while processing:
>  thinlinc-server_4.14.0post-0.unknown_amd64.deb

I'm not sure our current conversion of RPM's "Obsoletes" is entirely correct. We currently translate this to a "Replaces"/"Conflicts" pair, which seems to be what Debian suggests here:

https://www.debian.org/doc/debian-policy/ch-relationships.html#replacing-whole-packages-forcing-their-removal

We noticed this on bug 7279 comment 19. We also noticed that something wasn't entirely correct and added an extra "Provides" to work around the issue.

However when I look at this again, I notice more details in Debian's "Conflicts" chapter:

https://www.debian.org/doc/debian-policy/ch-relationships.html#conflicting-binary-packages-conflicts

And in Debian's guide for renaming a package:

https://wiki.debian.org/RenamingPackages

What they state is that "Breaks" should be used in this case, not "Conflicts". Let's see if that works better.
Comment 32 Pierre Ossman cendio 2022-02-23 15:29:47 CET
"Breaks" doesn't seem to have any effect unfortunately and I get the same error message.

Removing the "Provides: thinlinc-rdesktop" gets me a bit further though, and it fails with:

> dpkg: considering removing thinlinc-tladm in favour of thinlinc-server ...
> dpkg: yes, will remove thinlinc-tladm in favour of thinlinc-server
> dpkg: considering removing thinlinc-tlmisc in favour of thinlinc-server ...
> dpkg: no, cannot proceed with removal of thinlinc-tlmisc (--auto-deconfigure will help):
>  thinlinc-webaccess depends on thinlinc-tlmisc
>   thinlinc-tlmisc is to be removed.
> 
> dpkg: regarding thinlinc-server_4.14.0post-0.unknown_amd64.deb containing thinlinc-server:
>  thinlinc-server conflicts with thinlinc-tlmisc (<< 4.15.0)
>   thinlinc-tlmisc (version 4.14.0post-0.unknown) is present and installed.
> 
> dpkg: error processing archive thinlinc-server_4.14.0post-0.unknown_amd64.deb (--install):
>  conflicting packages - not installing thinlinc-server
> Errors were encountered while processing:
>  thinlinc-server_4.14.0post-0.unknown_amd64.deb

It seems dpkg is not bright enough to realise that thinlinc-webaccess will also be removed as part of this transaction.

It does suggest a flag there, that should work around the issue. It will basically deconfigure all packages in the way, which should be the packages being removed and hence safe. Unfortunately it doesn't work in practice because of bug 7842.

Since this fix would need to be applied on the old packages, this isn't a realistic way forward.

I can note that apt is more clever than dpkg and handles the upgrade without any extra flags. Not sure how though as as far as I know it used dpkg for the low level stuff.
Comment 33 Pierre Ossman cendio 2022-02-23 15:48:00 CET
So the --ignore-depends flag seem to work around dpkg's issue. If I add that and specify all the packages that are getting removed then dpkg seems to get through the transaction correctly.

This should hopefully be safe since all of those packages are getting removed, so any broken dependencies should disappear with them.
Comment 34 Pierre Ossman cendio 2022-02-24 09:26:02 CET
Also note that the old packages are left in a "deinstall" state. You have to manually purge them to fully get rid of them. I'm assuming that also happened with thinlinc-rdesktop on bug 7279.
Comment 37 Pierre Ossman cendio 2022-02-25 17:02:23 CET
Upgrading also stops the services as the old packages don't realise they are being replaced, so they think ThinLinc is being removed rather than upgraded.

Not sure if we should fix this, or if we should go ahead with implementing bug 7163.
Comment 38 Pierre Ossman cendio 2022-02-28 12:23:14 CET
Our ansible role will need to be adjusted as well as it uses dpkg to install packages. PR here:

https://github.com/cendio/ansible-role-thinlinc-server/pull/24
Comment 52 Pierre Ossman cendio 2022-03-01 09:45:04 CET
Should basically be done now. Need to write some release notes, and decide about bug 7163.

These things need regression testing:

* %ghost handling
* Upgrade RHEL 7 with tl-session running
* icons shown after install
* upgrade from TL < 4.7.0 (execute xsession)
* upgrade from TL < 4.8.0 (tlwebadm.hconf access)
* services stop on uninstall
* source code
* link kits

> * Common Makefile boilerplate should be clearly separated out so that it is easy to keep this up to date and consistent across the tree

Done. The common stuff is now in prologue.make which makes it trivial to check that things are in sync.

It adds a bit of noise to commits since there are so many duplicates of this file, but I think the advantages outweigh that.

> * All server module Makefiles should follow a common pattern so things are familiar between them

I think so. They are sorted in the same order and follow a common pattern how things are built and installed.

> * The new server RPM/deb should have identical contents to the previous setup

Yup. Cannot find any changes.

> * The new server RPM/deb should have identical behaviour in trigger script to the previous setup

Yup. Verified that all scripts are included. Will also check that the effect of the scripts are as expected (see regression tests above).

> * Upgrading ThinLinc to this new version should not require any extra manual steps from the user

Mostly. As mentioned above, a simple call to dpkg cannot handle this, unfortunately. Hopefully it is sufficient that apt does the right thing, as well as our installed.
Comment 53 Pierre Ossman cendio 2022-03-02 10:12:31 CET
It looks like this broke tab completion for some odd reason. Doing "make <TAB>" now just gets you "make default" for some reason.
Comment 54 Pierre Ossman cendio 2022-03-02 14:44:43 CET
(In reply to Pierre Ossman from comment #53)
> It looks like this broke tab completion for some odd reason. Doing "make
> <TAB>" now just gets you "make default" for some reason.

The problem is that I added a check to make sure these Makefile:s aren't used outside of cenbuild, since they assume a bunch of environment variables are set. And the bash completion for make runs make in order to produce a list of targets, which it no longer does as it bails early.

Removing the check, or just making an exception for completion, works a bit. But target names are incorrect as we now need those cenbuild environment variables to generate some file names.

What's needed to properly solve this is for the completion routines to run as if they were run by cbrun. We already have a hack to compensate for PATH, but nothing else. Hopefully we can extend that.
Comment 57 Pierre Ossman cendio 2022-03-03 10:48:27 CET
(In reply to Pierre Ossman from comment #52)
> These things need regression testing:
> 
> * %ghost handling

Works fine. Old Python 2 .pyc files were removed when upgrading from 4.6.0 to 4.14.0post. And the new __pycache__ directories and contents are removed when uninstalling the new 4.14.0post. rpm -qf also correctly identifies these cache files as belonging to thinlinc-server.

Tested on RHEL 7.

> * Upgrade RHEL 7 with tl-session running

Broken. This was originally bug 7161. I'm guessing the scripts don't consider this an upgrade so the fix doesn't trigger.

> * icons shown after install

Works on fresh install, but not on upgrade from 4.6.0. I think this is also an issue with the RPM scripts not considering this an upgrade. Before bug 7183 for 4.10.0 we manually installed and removed .desktop files and icons, which seems to be triggering now.

Tested on RHEL 7.

> * upgrade from TL < 4.7.0 (execute xsession)

Works. This was originally bug 5099 and only happens on .deb systems.

Tested on Ubuntu 18.04 by upgrading from 4.6.0.

> * upgrade from TL < 4.8.0 (tlwebadm.hconf access)

Works. This was originally bug 7043 and only happens on .deb systems. And this was actually done for 4.9.0.

Tested on Ubuntu 18.04 by upgrading from 4.6.0.

> * services stop on uninstall

Works. Tested on RHEL 7. No processes left after uninstall, and journalctl shows that the services were explicitly stopped.

> * source code

Contents of 4.14.0 and 4.14.0post is identical except that tigervnc/contrib was previously omitted for unclear reasons, and changes we've done to TigerVNC since release.

> * link kits
> 

Contents of 4.14.0 and 4.14.0post is identical except for the object file contents.
Comment 58 Pierre Ossman cendio 2022-03-03 16:12:13 CET
The upgrade issue doesn't seem to have a sane solution. RPM doesn't give the scripts any idea that they are being replaced, or that they are replacing something else. dpkg does include that information, but that doesn't directly help us as we use RPM as the base.

The issue with tl-session can probably be fixed by simply ignoring if it's an upgrade or not. If we find tl-session on disk we apply the workaround.

It also looks like the ordering of scripts by dpkg avoids the issue with .desktop/icons, but I haven't tested.

So the problem seems to mainly be that application launchers get borked on RPM systems when upgrading from pre-4.10.0. And bug 7163.

Not sure if that's worth implementing something special for.
Comment 59 Pierre Ossman cendio 2022-03-07 10:33:53 CET
(In reply to Pierre Ossman from comment #37)
> Upgrading also stops the services as the old packages don't realise they are
> being replaced, so they think ThinLinc is being removed rather than upgraded.
> 
> Not sure if we should fix this, or if we should go ahead with implementing
> bug 7163.

We've decided to do this now, so that will be handled on bug 7163.(In reply to Pierre Ossman from comment #57)

> > * icons shown after install
> 
> Works on fresh install, but not on upgrade from 4.6.0. I think this is also
> an issue with the RPM scripts not considering this an upgrade. Before bug
> 7183 for 4.10.0 we manually installed and removed .desktop files and icons,
> which seems to be triggering now.
> 

This should hopefully be very few users, and the consequences aren't too severe. We'll see if we can get install-server to do a remove/install sequence instead of a simple upgrade in this scenario, but otherwise we'll ignore this issue.
Comment 63 Pierre Ossman cendio 2022-03-07 12:28:48 CET
Should be all done now.

Tester can check the acceptance criteria, and the regression risks are listed above.
Comment 64 Samuel Mannehed cendio 2022-03-08 14:38:10 CET
The description in the RPM isn't very helpful at the moment:

> $ rpm -qip ./nightly-server/packages/thinlinc-server-4.14.0post-2486.x86_64.rpm
> Name        : thinlinc-server
> Version     : 4.14.0post
> Release     : 2486
> Architecture: x86_64
> Install Date: (not installed)
> Group       : System Environment/Daemons
> Size        : 45548749
> License     : Cendio EULA
> Signature   : (none)
> Source RPM  : thinlinc-server-4.14.0post-2486.src.rpm
> Build Date  : mån  7 mar 2022 18:36:30
> Build Host  : build.lkpg.cendio.se
> Packager    : Cendio AB <support@cendio.com>
> Vendor      : Cendio AB
> URL         : http://www.cendio.com
> Summary     : ThinLinc Remote Desktop Server
> Description :
> ThinLinc Remote Desktop Server.

We should probably make an effort to describe the product a bit here.
Comment 68 Samuel Mannehed cendio 2022-03-10 09:57:39 CET
The description is now updated.
Comment 70 William Sjöblom cendio 2022-03-15 11:05:13 CET
I have compared the packaged files in server build #2506 against the
4.14.0 release.

By using rpm2cpio to extract the RPMs of the development build and
release, then running `find . -printf "%M %p\n"' in each of the two
extracted trees, followed by diffing the two `find' outputs against each
other. Looking at the diff, the only things that differ are things added
during this release. All other files and their corresponding file
permission bits are the same in the RPMs of server build #2506 and the
4.14.0 release.

Similarly, the permission bits, owner/group and filenames for server
build #2506 and the 4.14.0 release were extracted as follows and then
diffed:
┌────
│ find <server-bundle>/packages/ | grep deb | xargs -L1 dpkg -c | \
│     tr -s " " | cut -d " " -f1,2,6 | uniq | sort > <output>
└────
This resulted in the same diff as for RPM comparison. Thus we can
conclude that both the RPMs and DEBs install files under the same paths
with the same permission bits and owner as before. Note that this does
not verify the contents of the installed files.
Comment 72 William Sjöblom cendio 2022-03-15 14:45:31 CET
Somewhat similar to what was performed in comment 70, I have now also
diffed the hashes of the installed tree before and after the changes to
packaging. This has been done for both RPM and DEB using server build
#2506 and the 4.14.0 release. This was done by diffing the output of the
following command before and after upgrading the 4.14.0 server to server
build #2506:
┌────
│ find /opt/thinlinc/ -type f -exec md5sum {} \; | awk '{ print $2,$1 }' | sort
└────

This resulted in a few mismatches, primarily compiled binaries. Since
we do not take any measures to produce reproducible builds, this is
expected even though the source files for these binaries have not been
changed. Other than that, things seem to have remained constant
before/after these packaging changes. Thus we can check one of the
acceptance criteria:
• ☑ The new server RPM/deb should have identical contents to the
  previous setup
Comment 73 William Sjöblom cendio 2022-03-15 16:34:48 CET
I have now compared the triggers in the RPM and DEB packages before and
after these packaging changes (also considering the order of
installation as done by `tlinstaller'). Things look good apart from the
inherent upgrade issues described in comment 37 and comment 58. These
upgrade issues will be evaluated when testing the upgrade-related
acceptance criterion. Nonetheless, installing and uninstalling the
ThinLinc server has seemingly identical trigger/hook-related
behavior. Thus, we can check another of the acceptance criteria:
• ☑ The new server RPM/deb should have identical behavior in trigger
  script to the previous setup
Comment 74 William Sjöblom cendio 2022-03-15 16:58:33 CET
I've also taken a look at both the Makefiles and their common prologue files. All of these look very good, consistent, and are a much welcome improvement over the previous structure!

Thus, we have now verified the following acceptance criteria:
• ☑ Common Makefile boilerplate should be clearly separated out so that
  it is easy to keep this up to date and consistent across the tree
• ☑ All server module Makefiles should follow a common pattern so things
  are familiar between them
• ☑ The new server RPM/deb should have identical contents to the
  previous setup
• ☑ The new server RPM/deb should have identical behavior in trigger
  script to the previous setup

What remains now is the following criterion (along with some exploratory testing):
• ☐ Upgrading ThinLinc to this new version should not require any extra
  manual steps from the user
Comment 75 William Sjöblom cendio 2022-03-16 14:41:42 CET
I have now tested upgrading the 4.5.0 server to server build #2506 on
RHEL7 and Ubuntu 20.04.
• ☑ Sessions created before the upgrade are still reachable after the
  upgrade (discussed in comment 57)
• ☑ Services are stopped after the upgrade, but before tl-setup has
  started them again (discussed in comment 57)
• ☑ tlwebadm.hconf has correct permissions
• ☑ xsession has execute bit set
• ☑ `*.desktop' icons work as expected when installed using
  `tlinstaller'
• ☑ Upgrading ThinLinc to this new version should not require any extra
  manual steps from the user


I have also had a look through possible regressions and from what I can
tell the previously discussed regression risks seem to cover all the
reasonable regressions. These have now all been tested. Thus:
• ☑ Upgrading ThinLinc to this new version should not require any extra
  manual steps from the user

The acceptance criteria are now all tested, so what remains is some 
additional exploratory testing before closing.
Comment 76 William Sjöblom cendio 2022-03-16 14:46:12 CET
I have also taken a look at the server-bundle build times for the 4.14.0 tag and the 4.14.0post (with ccache disabled) and they look roughly similar for both clean and partial builds. So the work done here did seemingly not introduce any build performance regressions.
Comment 77 William Sjöblom cendio 2022-03-16 15:35:31 CET
Other than that, bash-completion seems to work a treat and the release notes look good. Marking as closed.
Comment 78 Pierre Ossman cendio 2022-03-24 15:43:42 CET
There is a problem with the upgraded coreutils package. It has been linked to openssl, which isn't a specified dependency. So installing a more bare system results in:

> $ cbrun armhf sort
> sort: error while loading shared libraries: libcrypto.so.3: cannot open shared object file: No such file or directory
Comment 80 Pierre Ossman cendio 2022-03-24 15:54:48 CET
coreutils now rebuilt without the openssl dependency.
Comment 81 Linn cendio 2022-03-25 14:54:01 CET
Tested on RHEL 8 and could successfully reproduce the error in comment 78. After installing the new coreutils package, the error was no longer present.

Code changes also look minimal, closing.
Comment 82 Pierre Ossman cendio 2022-03-28 12:57:22 CEST
Even more coreutils breakage on a minimal system:

> ls: error while loading shared libraries: libcap.so.2: cannot open shared object file: No such file or directory
Comment 84 Pierre Ossman cendio 2022-03-28 14:15:25 CEST
New build up, now without a libcap dependency.
Comment 85 William Sjöblom cendio 2022-04-04 10:47:26 CEST
Things are now looking good for all executable binaries shipped by
`cendio-build-coreutils-$ARCH' for all architectures.

Tested using the following snippet on a clean RHEL8 system with only
`cendio-build-utils' and `cendio-build-coreutils-$ARCH' installed:
> ┌────
> │ rpm -ql cendio-build-coreutils-$ARCH \
> │     | grep bin/ | sed "s|.*$ARCH||" 2>&1 \
> │     | xargs -i% cbrun $ARCH % --version \
> │     | grep error
> └────
The above snippet results in a bunch of "error while loading shared
libraries"-errors with both coreutils 9.0-1 and -2 but runs cleanly on
9.0-3. Closing!
Comment 87 William Sjöblom cendio 2022-04-19 15:50:35 CEST
Upgrading from 4.12.0 to a recent Jenkins build will kill all active webaccess sessions. We have been able to reproduce the issue on Ubuntu 20.04 but not on Fedora 35, .

We've bisected the issue down to r37895 (the commit which merges all server packages into one). Note that uninstalling the 4.12.0 server also kills all webaccess sessions, at least on Ubuntu 20.04. So this smells a bit like an issue with how we replace the previous server packages with the new single server package.

I have yet to try upgrading from versions other than 4.12.0.
Comment 88 William Sjöblom cendio 2022-04-19 16:32:16 CEST
Okay so I've done some more digging and here is what I've found out:

- Both removing or upgrading 4.12.0 straight to 4.14.0post results in a webaccess disconnect

- Upgrading 4.12.0 to 4.14.0 and then to 4.14.0post (with the same webaccess session connected throughout) works just fine

- Both removing or upgrading 4.12.1 straight to 4.14.0post works just fine
Comment 89 William Sjöblom cendio 2022-04-19 16:57:41 CEST
I've also made another observation:

- Removing 4.12.0 on RHEL8 (and very likely Fedora 35 too) behaves just like on Ubuntu (i.e. webaccess sessions are disconnected)

- But, as mentioned above, upgrading from 4.12.0 to 4.14.0post works as intended on RHEL8.
Comment 90 Pierre Ossman cendio 2022-04-20 09:36:34 CEST
The disconnects are expected when removing 4.12.0, since that is bug 7578. Unfortunately, this also triggers on the upgrade, as the obsolete handling is more like an uninstall/install than an upgrade (as mentioned earlier). It happens to work on rpm systems since the relevant script there is executed after the service units have been replaced with the new (fixed) versions.
Comment 91 Pierre Ossman cendio 2022-04-21 09:01:17 CEST
I tried to verify that this only affects 4.12.0 by attempting an upgrade from 4.11.0. Unfortunately, that is also broken because of a similar bug.

4.11.0 is not possible to uninstall because remove_server was broken in bug 5974 and fixed in bug 6235. I.e. between version 4.7.0 and 4.12.0.

Again, this only happens on .deb systems.

Unfortunately, the reason it works on RPM systems is because of another major issue that we seem to have overlooked for some time. %preun is run after files on disk have been replaced. That means when we use support functions (like in /opt/thinlinc/libexec/functions), we will be using the new ones and not things from the version that %preun was designed for. That means we need to be extremely careful with things in %preun. Anything we call there must be stable long enough to support any upgrade.

So what happens in this case is that the function "init_is_systemd" no longer exists, and neither does "systemctl_redirect". This causes things to fall back on chkconfig and manually killing things. Which happens to be what we want. So things are working by luck rather than design there.

You can see this in the output during upgrade:

> /opt/thinlinc/libexec/service: line 14: init_is_systemd: command not found
> /etc/init.d/tlwebadm: line 29: systemctl_redirect: command not found
> Shutting down ThinLinc Web Administration
> /opt/thinlinc/libexec/remove_service: line 18: init_is_systemd: command not found
Comment 92 Pierre Ossman cendio 2022-04-21 11:02:00 CEST
We should also see bug 7478 on upgrade, but for some reason we are not. Either rpm has changed behaviour over the years, or it does things in a different order on an upgrade compared to a removal.
Comment 93 Pierre Ossman cendio 2022-04-21 12:44:50 CEST
My bad. I did not get the same problem¹ as mentioned in bug 7478, so it might still be possible for it to happen. And I can see that rpm fails to respect the dependency order on upgrade, so it is likely the bug can happen if things happen to be reordered differently.

¹ I got a problem with /opt/thinlinc/libexec/functions instead. And that file
  exists after an upgrade, but not after a removal. So it's not an equivalent
  test.
Comment 94 Pierre Ossman cendio 2022-04-21 13:30:05 CEST
Unfortunately, I don't see a clean way around these issues. The broken actions are the very first things executed (old prerm). There are no files or scripts from the updated package available yet. Any fixes would need to be in install-server. Which is not very easy or clean, and it doesn't help other installation methods.

The disconnect issue when upgrading from 4.12.0 is probably acceptable, as users can get their session back once the upgrade is completed. But upgrading from 4.7.0-4.11.0 is currently very difficult on .deb systems.
Comment 95 Pierre Ossman cendio 2022-04-21 13:49:31 CEST
As this is technically very difficult to fix, we will go with a simpler approach:

 * Add a platform specific note that you cannot upgrade directly from these
   versions

 * Mention the same thing in the release notes

 * See if install-server can enforce this

Hopefully, there are very few users (or none) doing big jumps in versions like this. And any that happen to be affected can be handled via support.
Comment 98 Pierre Ossman cendio 2022-04-21 14:34:26 CEST
Upgrading on RPM is now more robust, as we provide stub versions of the functions older versions need during the upgrade.
Comment 99 Pierre Ossman cendio 2022-04-22 07:51:01 CEST
These are the platform specific notes we need to add once the release is out:

It is unfortunately not possible to upgrade a version of ThinLinc older than 4.12.0 to ThinLinc 4.15.0 or newer. Such an upgrade must instead be done in multiple steps, e.g. 4.10.0 to 4.14.0 to 4.15.0, or by first removing the old version and then installing the new version of ThinLinc.

Some older versions of ThinLinc have bugs that prevent them from being easily uninstalled. Please follow the following instructions to uninstall anything older than ThinLinc 4.12.0:

> $ sudo systemctl stop tlwebadm tlwebaccess vsmserver vsmagent
> $ sudo rm /var/lib/dpkg/info/thinlinc-*.prerm
> $ sudo apt purge "thinlinc-*"
Comment 104 Pierre Ossman cendio 2022-04-22 07:55:24 CEST
A check in install-server is now in place. It has the following behaviour:

 * No complaints on RPM systems, ever

 * No complaints on .deb systems where ThinLinc isn't installed

 * No complaints on .deb systems where the installed ThinLinc is 4.12.0 or newer

 * Refuses to continue on .deb systems where the installed ThinLinc is older than 4.12.0
Comment 106 Samuel Mannehed cendio 2022-04-25 09:45:42 CEST
tl-setup fails to restart the 'tlwebaccess' service after an upgrade from tl-4.11.0 to build 2591 on RHEL8:

> Configuring and starting ThinLinc services... failed.
>
> An error occurred whilst trying to configure and start one or more of
> ThinLinc's services. You will need to manually install and start each of
> the ThinLinc services vsmserver, vsmagent, tlwebaccess and tlwebadm. It
> is not possible to use ThinLinc until this problem is solved.

The log says:

> 2022-04-25 09:18:18,454: Starting services...
> 2022-04-25 09:18:18,454: Starting service 'vsmagent'...
> 2022-04-25 09:18:18,721: Starting service 'tlwebadm'...
> 2022-04-25 09:18:19,020: Starting service 'vsmserver'...
> 2022-04-25 09:18:19,210: Examining service 'tlwebaccess'...
> 2022-04-25 09:18:19,224: Could not find a PID file for service 'tlwebaccess'

This seems to only happen when a Web Access connection is active during the upgrade.
Comment 108 Samuel Mannehed cendio 2022-04-25 13:26:09 CEST
(In reply to Pierre Ossman from comment #95)
>  * Mention the same thing in the release notes

No mention of this problem has been added so to the release notes so far.
Comment 109 Samuel Mannehed cendio 2022-04-25 14:10:13 CEST
Aside from the issues mentioned in comment #106 and comment #108 I have verified that things look good here.

To verify my understanding of the issue I have seen that, on Ubuntu 20.04:

 * I can't upgrade from 4.11.0 to build 2589
 * I get disconnected Web Access sessions when:
   - Upgrading from 4.11.0 to 4.12.0
   - Upgrading from 4.12.0 to build 2591

I then verified the fix on Ubuntu 20.04:

 * Upgrading from 4.11.0 to build 2591:
   - directly installing the .deb's fail with errors from dpkg/apt
   - install_server blocks this with an error in text mode
   - install_server blocks this with an error in GUI mode
   - the install_server error is easy to understand
 * Using the PSN (comment #99) worked well
   - I first uninstalled 4.11.0 and then install build 2591
 * Upgrading from 4.11.0 to 4.12.1 and then to build 2591:
   - installation and upgrades work well, no errors
   - existing Web Access connection was not disconnected

Lastly, I also verified the fix on RHEL 8:

 * Upgrading from 4.11.0 to build 2591 works mostly well
   - works well when no active Web Access sessions (see comment #106)
 * Upgrading from 4.11.0 to 4.12.1 and then to build 2591 works well
   - even with active Web Access sessions
   - tested by using install_server
   - tested by upgrading directly using the .rpm files with dnf

The commits also look good.
Comment 110 William Sjöblom cendio 2022-04-26 09:14:14 CEST
I was able to reproduce the issue (mentioned in comment 106) when
upgrading from 4.11.0 to server build #2591. On the other hand, when
upgrading from 4.11.0 to #2592, the tlwebaccess service start up as
expected even with tlwebaccess session(s) connected during the upgrade.

Note that several tests failed during build #2592. From what I've heard,
this is due to faulty tests. But let's postpone closing this bug until
we've verified that fixing these failing tests does not bring in any
changes to non-test code.

Regarding the release notes mentioned in comment 95, these would need to
be included in every upcoming release until a point where we decide that
EVERYONE is using a sufficiently new version. In other words, this is better
mentioned in a PSN exclusively.
Comment 111 William Sjöblom cendio 2022-04-26 10:13:18 CEST
There were a couple of changes to service handling in r38275 for bug 7163. I have retested the upgrade from 4.11.0 to the latest server build #2593 and things still work as intended.
Comment 112 William Sjöblom cendio 2022-04-26 15:54:57 CEST
The failing tests mentioned in comment 110 have now been fixed. Closing.
Comment 114 Linn cendio 2023-08-16 13:15:09 CEST
We found a regression regarding obfuscation for sitecustomize.py, where the copy of the file used by the installer is not obfuscated. This file is obfuscated when installed, and both copies should be obfuscated.
Comment 116 Linn cendio 2023-08-16 14:13:41 CEST
Our sitecustomize.py is now fully obfuscated, both in the installed file and the file included in the server bundle zip.

Also tested that environment variables starting with PYTHON are preserved after the server has been installed by setting PYTHONPATH to a custom value and verifying that the value was the same after installation. Tested on Ubuntu 22.04 and SLES 15.
Comment 117 Pierre Ossman cendio 2023-08-16 14:46:37 CEST
Tested on RHEL 9, and our functionality for the python3 wrapper seems to work:

 * Our modules directory is implicitly added to sys.path

 * $PYTHON* variables are ignored. I compared with an unwrapped python3, and
   saw a difference in sys.path for $PYTHONPATH, and in sys.flags for
   $PYTHONDEVMODE

 * $PYTHON* variables still propagate to subprocesses, tested via
   subprocess.run("env")

 * No remnants of $__TL_ORIGPYTHON* variables, except in /proc/<pid>/environ

Seems to work entirely as designed.

Note You need to log in before you can comment on or make changes to this bug.