Document how to upgrade the rtcd service - #9136
Conversation
The Calls deployment docs covered deploying and scaling rtcd, but never how to upgrade a running deployment. The only mention of upgrade ordering was a v7.7-specific note in important upgrade notes. Adds an "Upgrade the rtcd service" section covering version compatibility with the Calls plugin, rtcd's drain-on-shutdown behavior, a rolling upgrade procedure for horizontally scaled fleets, single instance upgrades, and the Kubernetes/Helm defaults that support draining. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
48c1afc to
66e7bc6
Compare
`systemctl stop` sends SIGTERM but escalates to SIGKILL after TimeoutStopSec, which defaults to 90s. The unit file recommended in the rtcd docs doesn't set it, so a drain longer than 90 seconds gets killed and drops calls. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe RTCD setup guide now documents release independence, persistent store handling, version compatibility, graceful shutdown, single-server upgrades, rolling upgrades, and Kubernetes replacement settings. ChangesRTCD upgrade guidance
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@source/administration-guide/configure/calls-deployment.md`:
- Line 656: Replace the hardcoded docs.mattermost.com links at the referenced
upgrade-notes and rtcd-service-url references with repository-relative page
links, using {ref} syntax for the rtcd-service-url section. Update all affected
links while preserving their destinations and link text.
- Around line 658-667: Update the version-check procedure around the Mattermost
plugin endpoint and direct RTCD `/version` query by adding an `{important}` note
that checks must run from the Mattermost host or an approved application subnet,
use the configured `http` or `https` scheme, and keep `8045/TCP` internal for
mandatory Mattermost-to-RTCD traffic; state the source, destination, direction,
protocol, and access scope explicitly.
- Around line 698-703: Update the “Upgrading a single instance” procedure to
require removing the instance from DNS or otherwise taking it out of rotation,
then confirm the plugin no longer assigns new calls before sending SIGTERM.
Preserve the existing drain and forced-stop options, while ensuring new calls
are blocked during the drain.
- Line 641: Update the rtcd upgrade guidance around the persistence statement to
acknowledge that store.data_source persists registered client IDs and hashed
credentials, typically under /tmp/rtcd_db, and may be lost during pod
replacement without mounted persistent storage. Add the required
persistent-volume, backup, and client re-registration considerations before
presenting the Kubernetes workflow as out of the box.
- Around line 709-714: Update the rtcd Helm chart deployment prose and warning
to identify the grace-period value using the actual path
daemonset.terminationGracePeriod. Replace any unqualified or
configuration-prefixed references while preserving the documented 18000-second
default and call-draining behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 055797ac-8a7c-4664-a966-60af3e093cb5
📒 Files selected for processing (1)
source/administration-guide/configure/calls-deployment.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@source/administration-guide/configure/calls-deployment.md`:
- Around line 673-675: Update the systemd configuration code fence containing
TimeoutStopSec=18000 to include the ini language tag, preserving the existing
configuration content.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9e4fb9d2-127d-45cb-bf4d-82a47b556f9a
📒 Files selected for processing (1)
source/administration-guide/configure/calls-deployment.md
master split calls-deployment.md into six pages, so the upgrade section has been ported to the new calls-rtcd-setup.md page and adapted to its conventions (title case headings, RTCD in prose, relative cross-page links). Also adds the Calls plugin's startup dependency on RTCD reported by the Calls team: the plugin opens the connection when it starts and fails to start if the service isn't reachable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Newest code from mattermost has been published to preview environment for Git SHA 41de4bf |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
source/administration-guide/configure/calls-deployment.md (3)
709-714: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the chart's actual Helm value path.
The current chart defines
configuration.terminationGracePeriod: 18000, and both Deployment and DaemonSet templates render.Values.configuration.terminationGracePeriod. The unqualified key in this page is not an actionable values path. Update both references and verify the path against the chart version supported by the guide. (raw.githubusercontent.com)Proposed wording
-sets `terminationGracePeriod` to `18000` seconds +sets `configuration.terminationGracePeriod` to `18000` seconds -Don't reduce `terminationGracePeriod` +Don't reduce `configuration.terminationGracePeriod`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/administration-guide/configure/calls-deployment.md` around lines 709 - 714, Update both references to the termination grace-period setting in the RTCD deployment guide to use the actionable Helm values path configuration.terminationGracePeriod, matching the chart’s values.yaml and Deployment/DaemonSet templates; verify this path against the chart version supported by the guide.Source: MCP tools
641-641: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not describe the Kubernetes upgrade as storage-safe by default.
The current
rtcdconfiguration stores registered client IDs and hashed credentials instore.data_source, which defaults to/tmp/rtcd_db. The current Helm chart leaves extra volumes and mounts empty by default. A pod replacement can therefore lose the credentials unless the operator mounts persistent storage. State the storage, backup, and client re-registration requirements before calling this workflow “out of the box”. (raw.githubusercontent.com)Also applies to: 709-711
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/administration-guide/configure/calls-deployment.md` at line 641, Update the rtcd upgrade guidance to state that Kubernetes pod replacement can lose registered client IDs and hashed credentials when store.data_source defaults to /tmp/rtcd_db and no persistent volume is configured. Document the need for persistent storage, backups, and client re-registration considerations before presenting the workflow as generally safe or out of the box.Source: MCP tools
700-703: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRepeat the DNS withdrawal step before
SIGTERM.The preceding section requires DNS removal before signalling because listeners remain open. This single-instance procedure starts with
SIGTERMbut does not repeat the DNS removal and plugin-confirmation wait. Without that prerequisite, new calls can still be assigned during the drain, so “new calls fail” is not guaranteed.Proposed clarification
- **Wait for the drain to complete.** Send `SIGTERM` and let the service exit after the last call ends. + Remove the instance from DNS, wait for the plugin to stop assigning new calls, then send `SIGTERM` and let the service exit after the last call ends.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/administration-guide/configure/calls-deployment.md` around lines 700 - 703, Update the single-instance upgrade procedure before the SIGTERM step to repeat the required DNS withdrawal and plugin-confirmation wait from the preceding section. Ensure traffic is removed before draining so no new calls are assigned during shutdown, while preserving the existing drain and forced-stop options.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@source/administration-guide/configure/calls-deployment.md`:
- Line 669: Update the systemd shutdown guidance in the deployment documentation
to instruct administrators to run `systemctl show rtcd -p TimeoutStopUSec` and
check the effective `TimeoutStopSec` value before relying on the drain timeout;
do not present 90 seconds as universal, since it depends on the systemd
configuration.
---
Duplicate comments:
In `@source/administration-guide/configure/calls-deployment.md`:
- Around line 709-714: Update both references to the termination grace-period
setting in the RTCD deployment guide to use the actionable Helm values path
configuration.terminationGracePeriod, matching the chart’s values.yaml and
Deployment/DaemonSet templates; verify this path against the chart version
supported by the guide.
- Line 641: Update the rtcd upgrade guidance to state that Kubernetes pod
replacement can lose registered client IDs and hashed credentials when
store.data_source defaults to /tmp/rtcd_db and no persistent volume is
configured. Document the need for persistent storage, backups, and client
re-registration considerations before presenting the workflow as generally safe
or out of the box.
- Around line 700-703: Update the single-instance upgrade procedure before the
SIGTERM step to repeat the required DNS withdrawal and plugin-confirmation wait
from the preceding section. Ensure traffic is removed before draining so no new
calls are assigned during shutdown, while preserving the existing drain and
forced-stop options.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 738ed453-f1d2-4fe6-bd0a-8676e6e98f52
📒 Files selected for processing (1)
source/administration-guide/configure/calls-deployment.md
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@source/administration-guide/configure/calls-rtcd-setup.md`:
- Line 389: Replace the absolute Mattermost documentation URL in the
upgrade-notes reference with the repository’s native relative link or {ref}
target, using the confirmed local filename. Keep the curl command and its
surrounding guidance unchanged.
- Around line 451-454: Update the “Stop the service at a set time” option to
explain how administrators configure the fixed stop period: for systemd,
reference TimeoutStopSec and provide a safely executable systemctl stop rtcd
command, and direct users of other supervisors to their equivalent stop-timeout
setting. Preserve the warning that active calls are dropped.
- Line 397: Fix the punctuation in the documentation sentences at the referenced
clauses: replace the comma splices before “otherwise” with sentence breaks or
semicolons, and add the required comma where two independent clauses are joined.
Preserve the existing wording and nearby documentation style.
- Around line 393-398: Update the RTCD drain documentation to qualify the “calls
in progress are never force-closed” statement: clarify that normal draining does
not force-close calls, but a process supervisor may still terminate the service
after its timeout and drop active calls. Keep the existing guidance about
removing DNS entries and configuring sufficient stop timeouts.
- Around line 460-465: Update the RTCD Helm chart upgrade warning to state
explicitly that terminationGracePeriod is an upper bound: calls still running
after five hours are force-terminated and dropped when Kubernetes sends SIGKILL.
Replace the claim that the default is long enough to outlast extended meetings
while preserving the existing guidance not to reduce the grace period.
- Around line 372-376: Update the RTCD availability guidance in the important
note to distinguish startup from runtime failures: state that an initial
connection failure during plugin activation requires restarting the Calls plugin
after RTCD is available, while an established connection is retried
automatically after later disconnections until the retry limit is reached.
- Around line 460-462: Update the RTCD Helm chart upgrade documentation to state
that configuration.terminationGracePeriod maps to Kubernetes
terminationGracePeriodSeconds. Remove the claims that pods are replaced one at a
time and that each pod drains before the next replacement; describe that,
without maxSurge configured, Kubernetes may run old and new pods concurrently
during the RollingUpdate.
- Line 368: Update the RTCD upgrade guidance to remove the claim that it has no
persistent state and document StoreConfig.DataSource as the persistent Bitcask
store containing registered client IDs and bcrypt-hashed authentication keys.
State that the configured data directory must be preserved across binary or
container replacements, including mounting a persistent volume for Docker, and
specify the required backup or migration steps before upgrading.
- Around line 408-418: Update the RTCD server-draining procedure to require an
observable Mattermost-side check that the server is absent from the resolved
RTCD host set before waiting for rtcd_rtc_sessions_total to reach zero. Add
equivalent guidance for Kubernetes, clarifying that terminationGracePeriod does
not prevent new call assignments and must not replace the service-discovery
check.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 19423c6d-cb7c-4c13-bc36-1e8dc9f290c2
📒 Files selected for processing (1)
source/administration-guide/configure/calls-rtcd-setup.md
The simpler case reads first, and the rolling upgrade builds on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Describe what systemd's default stop timeout does to a drain without recommending a change to it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Newest code from mattermost has been published to preview environment for Git SHA abc7fc0 |
|
Newest code from mattermost has been published to preview environment for Git SHA e476326 |
- Use {doc} cross-references instead of hardcoded docs.mattermost.com URLs
- Document the Bitcask data store at [store] data_source, what it holds,
and how to preserve it across binary and container replacements
- Correct the Helm rollout description: the chart sets no maxSurge, so a
Deployment rollout can run old and new pods concurrently
- Fix a comma splice before "otherwise"
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An activation failure leaves Calls deactivated with no automatic retry, while an established connection reconnects on its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
source/administration-guide/configure/calls-rtcd-setup.md (1)
465-465: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the DNS re-entry description.
When an IP returns to DNS,
hostsCheckerunflags the existing host. It creates a client only when the IP is not already present inm.hosts. Since DNS removal flags the existing host instead of deleting it, “creates a client” is not generally accurate. (raw.githubusercontent.com)Proposed wording
- The plugin creates a client for the server on its next resolution cycle and starts assigning new calls to it. + On its next resolution cycle, the plugin unflags the existing server, or creates a client if the IP is new, and starts assigning new calls to it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/administration-guide/configure/calls-rtcd-setup.md` at line 465, Update the DNS re-entry description to state that hostsChecker unflags the existing host when its IP returns to DNS, rather than claiming it creates a client; retain the description that the host can receive new calls on the next resolution cycle.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@source/administration-guide/configure/calls-rtcd-setup.md`:
- Line 478: Before the Helm upgrade step, add a clearly labeled Kubernetes
prerequisite requiring persistent storage for the RTCD store. Explain that
configuration.extraVolumes and configuration.extraVolumeMounts must mount a
persistent volume at the configured data_source path, and ensure administrators
complete this storage configuration before changing image.tag and applying the
chart.
- Around line 381-386: Change the admonition around the lost-store recovery
guidance from `{note}` to `{warning}`. Explicitly state that
`allow_self_registration` permits unauthenticated client registration, remains
disabled by default, and must not be enabled on an internet-exposed RTCD
service; restrict it to a private, access-controlled network.
---
Outside diff comments:
In `@source/administration-guide/configure/calls-rtcd-setup.md`:
- Line 465: Update the DNS re-entry description to state that hostsChecker
unflags the existing host when its IP returns to DNS, rather than claiming it
creates a client; retain the description that the host can receive new calls on
the next resolution cycle.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c8b5e436-fed3-472f-91e4-bd5b8adf5dc3
📒 Files selected for processing (1)
source/administration-guide/configure/calls-rtcd-setup.md
|
Newest code from mattermost has been published to preview environment for Git SHA c6d8cc9 |
|
Newest code from mattermost has been published to preview environment for Git SHA 20f20fd |
|
@hanzei - Thank you for your submission! As a next step, can you help add a dev who can help give this a dev review for us? And can you also update the appropriate server release milestone if applicable? |
- Promote the lost-store guidance to a warning and caution that allow_self_registration permits unauthenticated registration - Document how the data store is handled across pod replacements, and why a single shared volume isn't an option for it - Drop the client-creation mechanism claim from the DNS re-entry step Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Newest code from mattermost has been published to preview environment for Git SHA d121d7f |
|
Newest code from mattermost has been published to preview environment for Git SHA a8a33be |
|
|
||
| ### Version Compatibility | ||
|
|
||
| The Calls plugin enforces a minimum RTCD version and won't use a server running an older one. All Calls versions from v1.5.0 onwards require RTCD v0.17.0 or later. |
There was a problem hiding this comment.
I think it would be better to drop the specific versions here, as they are ancient, and simply refer the user to check for the minimum RTCD version required by calls plugin. This is currently found in the plugin manifest as props.min_rtcd_version.
|
|
||
| 3. **Wait for the server to go idle**: | ||
|
|
||
| The `rtcd_rtc_sessions_total` metric reports the number of active RTC sessions (see [RTCD Metrics](calls-metrics-monitoring.md#rtcd-metrics)). The server can be restarted safely once this reaches zero. |
There was a problem hiding this comment.
I think you need to sum RTC sessions over all calls:
The rtcd_rtc_sessions_total metric reports the number of active RTC sessions per call group (see RTCD Metrics (calls-metrics-monitoring.md#rtcd-metrics)). The server can be restarted safely once the sum across all groups reaches zero.
bgardner8008
left a comment
bgardner8008
left a comment
There was a problem hiding this comment.
This is a great improvement, thanks for doing this! I have a few suggestions.
3 participants
Review page
http://mattermost-docs-preview-pulls.s3-website-us-east-1.amazonaws.com/9136/administration-guide/configure/calls-rtcd-setup.html#upgrading-rtcd
AI Summary
We had no documentation on how to upgrade a running RTCD deployment. The Calls pages covered deploying and scaling the service, and the upstream rtcd repo docs cover install and config, but neither described the upgrade procedure. The only mention of upgrade ordering anywhere was a v7.7-specific row in Important Upgrade Notes stating that rtcd must be upgraded to >= 0.8.0 first.
This adds an
Upgrading RTCDsection tocalls-rtcd-setup.md, placed afterHorizontal Scaling(whose DNS-based load balancing the rolling procedure depends on) and beforeIntegration with Mattermost.[store] data_source(default/tmp/rtcd_db) holding registered client IDs and bcrypt hashes of their auth keys. Covers why it has to survive a binary or image replacement, the bare-metal vs Docker difference, backup, and what happens when it's lost: recovery is automatic whenallow_self_registrationis on and impossible when it's off (the default), since registration itself requires authenticating first. Includes a warning that the setting permits unauthenticated registration and belongs only on a private, access-controlled network.SIGTERM/SIGINT, RTCD waits for all active sessions to end, with no drain timeout. Two consequences are called out: the HTTP/WS listeners stay open during the drain, so the server must leave DNS before it's signalled or it keeps getting new calls; and any supervisor that force-kills after a timeout cuts off calls in progress, includingsystemctl stopunder systemd's 90 secondDefaultTimeoutStopSec.rtcd_rtc_sessions_totalto reach zero, stop, install, verify via/version, return to DNS.configuration.terminationGracePeriodmaps to the pod'sterminationGracePeriodSecondsand warns against lowering the 5 hour default, since that turns a graceful drain into aSIGKILLthat drops calls. Also notes that the chart sets nomaxSurge, so a Deployment rollout can run old and new pods concurrently for the length of the drains, and covers how the data store is handled across pod replacements.🤖 Generated with Claude Code