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

[teleport-update] needrestart and systemd drop-in #49806

Merged
merged 4 commits into from
Dec 10, 2024

Conversation

sclevine
Copy link
Member

@sclevine sclevine commented Dec 5, 2024

This PR sets the default needrestart configuration to disabled for the Teleport systemd service. This PR only affects Teleport when it is installed via the teleport-update command, and does not change the behavior of the teleport deb/rpm package.

needrestart is a Debian service that restarts services when their underlying libraries change. Networking services like Teleport are not always safe to restart during package upgrades, because they may disrupt connections. If needed, users may still opt-in to enable restarts for Teleport via dpkg (including interactive prompting). Internal link with more details.

This PR also adds a systemd drop-in that teleport-update can use to provide env vars to teleport. This will let teleport read agent group identifiers in the future. It's included here because the logic is very similar to the needrestart feature, and both features drove out some minor refactoring.

ubuntu@legendary-mite:~$ cat /etc/systemd/system/teleport.service.d/teleport-update.conf 
# teleport-update
# DO NOT EDIT THIS FILE
[Service]
Environment=TELEPORT_UPDATE_CONFIG_FILE=/opt/teleport/default/update.yaml

ubuntu@legendary-mite:~$ cat /etc/needrestart/conf.d/teleport-update.conf 
$nrconf{override_rc}{qr(^teleport\.service)} = 0;

The teleport-update binary will be used to enable, disable, and trigger automatic Teleport agent updates. The new auto-updates system manages a local installation of the cluster-specified version of Teleport stored in /opt/teleport.

RFD: #47126
Goal (internal): https://github.com/gravitational/cloud/issues/10289

@sclevine sclevine requested review from vapopov and hugoShaka December 5, 2024 06:20
@sclevine sclevine added the no-changelog Indicates that a PR does not require a changelog entry label Dec 5, 2024
@sclevine
Copy link
Member Author

sclevine commented Dec 6, 2024

Reviews appreciated when anyone has an opportunity 🙂

lib/autoupdate/agent/setup.go Show resolved Hide resolved
lib/autoupdate/agent/setup.go Outdated Show resolved Hide resolved
@sclevine
Copy link
Member Author

@codingllama @greedy52 @hugoShaka looking for another review when you have a chance 🙂

Copy link
Contributor

@hugoShaka hugoShaka left a comment

Choose a reason for hiding this comment

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

We had an interesting discussion about what to do with needrestart and ended up with this solution. Could you summarize it in a comment somewhere in setup.go so we have a written trace about this decision and why we chose to opt-out instead of teaching needrestat to do soft reloads?

@sclevine
Copy link
Member Author

summarize it in a comment somewhere in setup.go

👍 Great idea, will do.

@sclevine sclevine enabled auto-merge December 10, 2024 21:55
@sclevine sclevine added this pull request to the merge queue Dec 10, 2024
Merged via the queue into master with commit bc68383 Dec 10, 2024
42 checks passed
@sclevine sclevine deleted the sclevine/teleport-update-needrestart branch December 10, 2024 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Indicates that a PR does not require a changelog entry size/sm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants