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

RFD 144: Client tools auto updates #30317

Closed
wants to merge 7 commits into from

Conversation

bernardjkim
Copy link
Contributor

Contributes to https://github.com/gravitational/cloud/issues/4880
Supersedes #28337

For the initial implementation we've decided to limit scope to Cloud users only. We also will not be supporting multiple versions as all Cloud tenants are expected to be running the same Teleport version.

Copy link
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

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

Overall makes sense and I like the reduction in scope, just left a bunch of suggestions/clarifications.

rfd/0144-client-tools-auto-updates.md Outdated Show resolved Hide resolved
rfd/0144-client-tools-auto-updates.md Outdated Show resolved Hide resolved
rfd/0144-client-tools-auto-updates.md Show resolved Hide resolved
rfd/0144-client-tools-auto-updates.md Show resolved Hide resolved
rfd/0144-client-tools-auto-updates.md Outdated Show resolved Hide resolved
rfd/0144-client-tools-auto-updates.md Outdated Show resolved Hide resolved
rfd/0144-client-tools-auto-updates.md Outdated Show resolved Hide resolved
rfd/0144-client-tools-auto-updates.md Outdated Show resolved Hide resolved
rfd/0144-client-tools-auto-updates.md Outdated Show resolved Hide resolved
rfd/0144-client-tools-auto-updates.md Outdated Show resolved Hide resolved
@r0mant r0mant requested a review from xinding33 August 11, 2023 22:45
Copy link
Contributor

@jentfoo jentfoo left a comment

Choose a reason for hiding this comment

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

Some initial questions, thank you!

rfd/0144-client-tools-auto-updates.md Show resolved Hide resolved
rfd/0144-client-tools-auto-updates.md Outdated Show resolved Hide resolved
rfd/0144-client-tools-auto-updates.md Outdated Show resolved Hide resolved
rfd/0144-client-tools-auto-updates.md Outdated Show resolved Hide resolved
- More details on binary execution
- More security details
rfd/0144-client-tools-auto-updates.md Show resolved Hide resolved
rfd/0144-client-tools-auto-updates.md Outdated Show resolved Hide resolved
rfd/0144-client-tools-auto-updates.md Outdated Show resolved Hide resolved

Ideally, we'd like this feature to be integrated seamlessly. Users of the Teleport client tools should not need to search for documentation and spend time figuring out how to enable auto updates for their cleint tools.

After client tools auto updates is deployed and the first time the client tools are executed, the user will be asked two questions: 1) if they would like to update to the latest version (default yes) and 2) if they would like to enroll in auto updates for client tools (default yes).
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to turn this on by default when Cloud clusters are targeted, and require an opt-out command to disable it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have a strong opinion on either way. I think most users would opt-in to auto updates so I think it'd be fine to turn this on by default.

It could also be nice to prompt the user when auto updates are first available to let the user know that auto updates is configurable.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we should ask whether they want to enroll or not. Let's just enable it by default.

rfd/0144-client-tools-auto-updates.md Outdated Show resolved Hide resolved
rfd/0144-client-tools-auto-updates.md Outdated Show resolved Hide resolved
rfd/0144-client-tools-auto-updates.md Outdated Show resolved Hide resolved
rfd/0144-client-tools-auto-updates.md Outdated Show resolved Hide resolved
rfd/0144-client-tools-auto-updates.md Outdated Show resolved Hide resolved
@bernardjkim
Copy link
Contributor Author

Sorry for the back and forth. After some more discussion we're leaning more towards just using the system package manager to update the client tools. This might end up being tedious to add support for all our installation methods, but the solution is a lot simpler and straight forward.


Another downside to this approach would be that upgrades will require sudo/admin privileges.

### Caching (alternative)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: I would move this section way down below, maybe under Appendix/Alternatives or something so the reader does not lose context of how things will actually work with the selected approach.


### Config

Automatic updates for client tools can be configured through tsh configuration. If `DISABLE_AUTO_UPDATE=true`, then auto updates for client tools will be disabled. If `DISABLE_UPDATE_PROMPT=true`, then the user will not be prompted to confirm the update. Both values will default to false.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are DISABLE_AUTO_UPDATE and DISABLE_UPDATE_PROMPT environment variables? It's not super clear what "tsh configuration" means in this case. If these are env. vars, let's say so explicitly.

I think it would also make sense to add these options to tsh.yaml config and give an example of what tsh config will look like.


Ideally, we'd like this feature to be integrated seamlessly. Users of the Teleport client tools should not need to search for documentation and spend time figuring out how to enable auto updates for their cleint tools.

After client tools auto updates is deployed and the first time the client tools are executed, the user will be asked two questions: 1) if they would like to update to the latest version (default yes) and 2) if they would like to enroll in auto updates for client tools (default yes).
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we should ask whether they want to enroll or not. Let's just enable it by default.


We should provide some observability into the download status. Whenever a new cloud-stable version is available for download, the progress of the update will be output to stdout. e.g. `New cloud-stable version of Teleport detected`, `Downloading latest version of tsh/tctl... `, `Updated tsh/tctl to version v13.2.3!`.

To avoid breaking existing scripts, a `--[no-]auto-update` flag will be included. Without the flag given, when a tty is detected and an update is available, the user will be prompted to update. If the flag is not provided and a tty is not detected, updates will be skipped to avoid breaking scripts.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need the flag at all? Why not just skip updates when there's no tty?


### Package update

The client tools will be updated using the package manager available on the system. Teleport supports multiple package repositories and installation methods so we'll need to handle the update differently on each system. This will probably require a decent amount of testing and maintenance work, but we think this is the simplest and most straight forward solution.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The client tools will be updated using the package manager available on the system.

This needs a little bit more clarification. Say, I'm on Ubuntu but installed teleport binary directly. Will auto-update still try to attempt to use apt in this case? There may not be apt repository configured.

I think we need to flesh out how detection/installation logic will work a bit more, for each supported package type (direct download, Debian/Ubuntu deb, RHEL/CentOS rpm, macOS pkg, etc.) and include this in the RFD.

Copy link

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@russjones
Copy link
Contributor

Let's move the discussion to #39805.

@russjones russjones closed this Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rfd Request for Discussion size/sm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants