Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/tailscale/cli: implement update for arch-based distros #8655

Merged
merged 1 commit into from
Jul 20, 2023

Conversation

awly
Copy link
Contributor

@awly awly commented Jul 19, 2023

Arch version of tailscale is not maintained by us, but is generally up-to-date with our releases. Therefore "tailscale update" is just a thin wrapper around "pacman -Sy tailscale" with different flags.

Updates #6995

@awly awly requested a review from noncombatant July 19, 2023 22:44
@@ -44,8 +44,8 @@ var updateCmd = &ffcli.Command{
fs := newFlagSet("update")
fs.BoolVar(&updateArgs.yes, "yes", false, "update without interactive prompts")
fs.BoolVar(&updateArgs.dryRun, "dry-run", false, "print what update would do without doing it, or prompts")
fs.StringVar(&updateArgs.track, "track", "", `which track to check for updates: "stable" or "unstable" (dev); empty means same as current`)
fs.StringVar(&updateArgs.version, "version", "", `explicit version to update/downgrade to`)
fs.StringVar(&updateArgs.track, "track", "", `which track to check for updates: "stable" or "unstable" (dev); empty means same as current; ignored on Arch-based distros`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we can gate printing this additional caveat on GOOS=linux? It won't mean anything to Windows or macOS people, and might even confuse them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a condition for these flags to only register on non-Arch hosts 👍

cmd/tailscale/cli/update.go Outdated Show resolved Hide resolved
@awly awly force-pushed the awly/tailscale-update-arch branch from 34a060d to 3ebbda1 Compare July 19, 2023 23:23
Arch version of tailscale is not maintained by us, but is generally
up-to-date with our releases. Therefore "tailscale update" is just a
thin wrapper around "pacman -Sy tailscale" with different flags.

Updates #6995

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
@awly awly force-pushed the awly/tailscale-update-arch branch from 3ebbda1 to 7012f3d Compare July 20, 2023 00:37
@awly awly merged commit efd6d90 into main Jul 20, 2023
36 checks passed
@awly awly deleted the awly/tailscale-update-arch branch July 20, 2023 00:53
}
}()

out, err := exec.Command("pacman", "--sync", "--refresh", "--info", "tailscale").CombinedOutput()
Copy link

@Foxboron Foxboron Aug 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to break peoples systems as partial updates are not supported on Arch, it's everything or nothing. This will at some point lead to someone installing incompatible so-name updates on their system and hose it.

You have access to a reasonably responsive Arch maintainer, please reach out before implementing things like this.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this command in short it is the prime example for a partial update pacman -Syi tailscale, more info on that here: https://wiki.archlinux.org/title/System_maintenance#Partial_upgrades_are_unsupported

Copy link

@jelly jelly Aug 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively you don't have to parse the whole information snippet but instead use --print-format.

pacman -S tailscale --print-format "%v"
1.48.0-1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your comments!
https://wiki.archlinux.org/title/System_maintenance#Partial_upgrades_are_unsupported talks mostly about libraries. AFAIK the tailscale package itself doesn't contain any shared libraries and only depends on libc.
Is it still risky to update this kind of "leaf" package?

As for the general topic of auto-updates, is there any "standard" way to apply updates in the background for a rolling release like Arch? Other than writing a cronjob, of course.

We would really like to keep all tailscale clients on recent-ish versions, both for general bug/security fixes and for reducing the backwards-compatibility kludges in APIs that tailscale clients talk to.
But we also want to play nice with the package ecosystem of the distro that people use. So if we can't help users keep tailscale up-to-date on Arch without breaking stuff, then we'll have to resort to docs and hope people read them.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://wiki.archlinux.org/title/System_maintenance#Partial_upgrades_are_unsupported talks mostly about libraries. AFAIK the tailscale package itself doesn't contain any shared libraries and only depends on libc.
Is it still risky to update this kind of "leaf" package?

The command is not acting on the package at all.

pacman -Sy tailscale is the same as pacman -Sy; pacman -S tailscale. You are refreshing the databases and will pull inn updates relevant for the tailscale package. Currently tailscale depends on glibc so if you are so unlucky there is a new glibc version (which your system does not yet support), you are pulling inn both.

This will break your system.

As for the general topic of auto-updates, is there any "standard" way to apply updates in the background for a rolling release like Arch? Other than writing a cronjob, of course.

You just don't do this at all on Arch. Arch is a rolling release distro where users needs to resolve the result of any updates. Stuffing this into a cronjob is just a disaster waiting to happen. Unattended upgrades has the same issue.

We would really like to keep all tailscale clients on recent-ish versions, both for general bug/security fixes and for reducing the backwards-compatibility kludges in APIs that tailscale clients talk to.
But we also want to play nice with the package ecosystem of the distro that people use.

On Linux distros, how would implementing a completely new command encourage people to update more frequently? People are lazy when things work and updating unattended is bad behavior. Telling users to update more often is not really going to make them do it either.

. So if we can't help users keep tailscale up-to-date on Arch without breaking stuff, then we'll have to resort to docs and hope people read them.

Arch assumes competent users, so this is completely fine to expect of them.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, pacman -Sy tailscale is going to make all the next pacman -S operations dangerous until a full upgrade , pacman -Syu, is completed.

Comment on lines +384 to +389
// Trim the Arch patch version.
ver = strings.Split(ver, "-")[0]
if ver == "" {
return "", fmt.Errorf("version output from pacman is malformed: %q, cannot determine upgrade version", line)
}
return ver, nil

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This misses versions like 1:1.4.2-1 which would be a valid version if we where to preform a downgrade on the package.

alexelisenko pushed a commit to Control-D-Inc/tailscale that referenced this pull request Feb 15, 2024
…#8655)

Arch version of tailscale is not maintained by us, but is generally
up-to-date with our releases. Therefore "tailscale update" is just a
thin wrapper around "pacman -Sy tailscale" with different flags.

Updates tailscale#6995

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
Signed-off-by: Alex Paguis <alex@windscribe.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants