-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 dnf/yum-based distros #8678
Conversation
a586726
to
cb0e3b4
Compare
track := "unstable" | ||
if stable, ok := versionIsStable(ver); !ok { | ||
return fmt.Errorf("malformed version %q", ver) | ||
} else if stable { | ||
track = "stable" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is already handled here:
tailscale/cmd/tailscale/cli/update.go
Lines 118 to 132 in cb0e3b4
if version.IsUnstableBuild() { | |
up.track = "unstable" | |
} else { | |
up.track = "stable" | |
} | |
if updateArgs.version != "" { | |
stable, ok := versionIsStable(updateArgs.version) | |
if !ok { | |
return nil, fmt.Errorf("malformed version %q", updateArgs.version) | |
} | |
if stable { | |
up.track = "stable" | |
} else { | |
up.track = "unstable" | |
} |
cb0e3b4
to
c26387c
Compare
cmd/tailscale/cli/update.go
Outdated
switch { | ||
case haveExecutable("pacman"): | ||
up.update = up.updateArchLike | ||
case haveExecutable("apt-get"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe do haveExecutable("apt") || haveExecutable("apt-get")
? Belt and suspenders, or overkill? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a TODO, since updateDebLike
assumes apt-get
and will fail if that's missing, even if apt
is present.
Will add apt
support in a separate PR.
cmd/tailscale/cli/update.go
Outdated
} else if stable { | ||
track = "stable" | ||
} | ||
|
||
if os.Geteuid() != 0 { | ||
return errors.New("must be root; use sudo") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I keep thinking about this. It is bad to simply re-invoke ourselves with sudo?
And/or, should we re-tool the error message to talk about doas
(OBSD at least, maybe others?) and runas.exe
(Windows, when we get to that point)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a helper to check running as root and print an OS-appropriate message.
It doesn't look like updateWindows
requires admin privileges, at least not explicitly in the code today.
8a288e1
to
62704fa
Compare
This is the Fedora family of distros, including CentOS, RHEL and others. Tested in `fedora:latest` and `centos:7` containers. Updates #6995 Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
62704fa
to
25490d6
Compare
…ale#8678) This is the Fedora family of distros, including CentOS, RHEL and others. Tested in `fedora:latest` and `centos:7` containers. Updates tailscale#6995 Signed-off-by: Andrew Lytvynov <awly@tailscale.com> Signed-off-by: Alex Paguis <alex@windscribe.com>
This is the Fedora family of distros, including CentOS, RHEL and others. Tested in
fedora:latest
andcentos:7
containers.