Repository navigation
Upgrade Runbook GA - #1844
Upgrade Runbook GA#1844
Conversation
Bowei Du (bowei)
added
kind/docs
|
Overall this is much simpler to operate, but it gives up zero-loss and instant rollback. That tradeoff is the right one if it's deliberate, but the PR description ("Rewrite the runbook to GA solution") doesn't say so. Haven Xia (@HavenXia) can you please clarify what the rationale is? |
|
Haven Xia (@HavenXia) Some recommendations -
|
eb63eb6 to
0cb116b
Compare
Yes, it's deliberate. Thanks for calling it out; I'll add it to the description. Zero loss:The in-place roll replaces workers through the same eviction path as a node upgrade or a spotVM preemption. A worker gets An actor that is suspended in time loses nothing. One that isn't got suspend will crash and lose the work since its last snapshot. The old runbook got zero loss by having the operator suspend every actor on a node before moving that node, and that was most of what made it so heavy to run. Instant rollback:That needs the old and new dataplane running side by side, which means spare capacity and the version-label mechanism that #1812 removes. Rollback is now the same roll in reverse, from the old release. |
ab62606 to
d308711
Compare
2b045e6 to
4e33851
Compare
|
Raji (@Raji14) |
Raji (Raji14)
left a comment
Raji (Raji14)
left a comment
There was a problem hiding this comment.
Some of big issues:
-
U1. No way to see where the upgrade stands
"Check your progress" was removed instead of fixed. Step 3 can take hours (see U5), which means breaks and handoffs.
The only record of "the installed release" is a local file that every deploy overwrites. After step 1 it already shows the new version.
Fix: add a short "Where am I?" section that reads only from the cluster. -
Placeholders still break when pasted
OLD_RELEASE=<installed release, for example v0.2.0> gives syntax error near unexpected token 'newline' in bash and parse error near '\n' in zsh.
The next line then runs with an empty variable: git clone --branch https://… old. Git reports fatal: repository 'old' does not exist.
The same pattern appears at L34-35, L45, L51-52, L124-125, and L261-263.
Fix: put one "fill these in" block at the top (export OLD_RELEASE=v0.2.0 # ← change me). Work out everything else with commands. -
The doc never says where the install record is
"The record directory record.dir names" (L48): that setting is unset by default. The default is the OS cache directory (record.go L53-62):
Linux: ~/.cache/ate-setup/installs/.yaml
macOS: ~/Library/Caches/ate-setup/installs/.yaml. operator-install.md only shows the Linux path.
Records only exist on the machine that ran the install. Anyone upgrading from another laptop, or a cluster installed before #2223, has to write the file by hand. There is no template and no way to check it.
Fix: give the path for each OS (ls -t "${ATE_RECORD_DIR:-$HOME/.cache/ate-setup}/installs/") and a minimal template. Recommend keeping records in a shared repo by setting ATE_RECORD_DIR. -
Step 3 contradicts itself, and every pool is done by hand
Sub-step 4 ends with kubectl rollout status (L143), which blocks until the whole pool has rolled. Sub-step 5 then says to patch the other pools "without waiting for the roll to finish" (L146). The Ctrl-C tip is far away, at L156.
The doc doesn't say how to pick the first pool, even though "a bad image still removes the pool's first old worker" (L137).
Fix: split the step into:
3a canary: one small, non-critical pool per CLASS
3b the rest: a loop over the remaining pools
3c wait for all: one check that nothing was missed -
You still can't plan the total time
The summary now says "Seconds to hours per pool". The accurate rule of thumb was removed. It follows directly from the code (workerpool_apply.go L39-56):
Workers roll with maxSurge 0 and maxUnavailable 10%, rounded down to at least 1. Pools under 20 workers therefore replace one worker per batch.
Without spare capacity, each batch can wait up to 30 minutes, the suspend window. The pod grace period is 60 minutes.
Rule of thumb: pools under 20 workers take up to 30 minutes × workers. Larger pools take roughly 5–7 hours.
atelet rolls one node at a time with a 330 s grace period (atelet.yaml L138), so step 2 takes up to about 6 minutes × nodes.
The --rollout-timeout 1h in step 2 (L86) can expire on a healthy roll once a cluster has more than about 10 slow nodes. ate-setup then prints "retry with … failed/….yaml". Say that a timeout here does not mean the roll failed. -
Actor owners are never told
Substrate doesn't suspend actors. Each one needs a SIGTERM handler that gets it suspended within 30 minutes. Yet no step says "tell actor owners before step 3", and there's nothing to send them. -
Step 4 understates the impact on actors and covers too much
Egress isn't mentioned. atenet-egress connections still open after its ~15 s drain are cut (atenet-egress.yaml L1327-1336). For agents streaming model calls, this is the most visible effect of step 4. Only the router's >10 s cut is listed (L212).
The step 4 row of the summary table (L23) leaves out the possible second round of SIGTERM when a release changes the worker pod template.
Four separate deploys and a possible multi-hour roll share one Done/Stuck block. "An old pod keeps serving" can't be checked as written.
Fix: split the step into 4a–4d, each with its own Done/Stuck. Give a command that checks images. Link "Roll back step 4" (L210); it's the only Stuck bullet without a link. -
Rolling back step 3 means copying values out of a table
before-workerpools.txt is a column-aligned table for people, and rollback asks you to copy "the exact string" for each pool (L259).
|
Some nits:
|
|
Blockers Jetski called out: B1. The runbook needs #1812, which is still open |
B1. #1812 I plan to submit at the same second with this pr so it's ok. |
43650ec to
4eaec81
Compare
|
Thanks for the thorough pass. I pushed a commit that addresses most of it. I'm keeping the runbook short on purpose, so a few suggestions went in smaller than you proposed. Nits
Big issues
|
8c76a89 to
b31260a
Compare
…d one (agent-substrate#2588) `ate-setup` doesn't remember an external PostgreSQL's connection string between runs. The install record leaves it out intentionally because it can carry a password. If a run forgets `ATE_API_POSTGRES_READ_WRITE_CONNECTION_STRING`, `ate-setup` assumes the bundled database and writes its address into the API server's Secret: - `deploy apiserver` fails waiting for a bundled PostgreSQL that doesn't exist, and the next API server restart can't reach the database. - `deploy ate-system` starts an empty bundled PostgreSQL and moves the API server onto it, so every actor looks gone. ------ Now, if the run names no database and the Secret already holds an address that isn't the bundled one, `ate-setup` stops before it changes the database settings and names the missing variable. A fresh installs, bundled installs and Cloud SQL work as before, and so does leaving Cloud SQL with an empty `ATE_API_POSTGRES_CLOUDSQL_INSTANCE`. A second commit does the same for `ATE_API_POSTGRES_OWNER_CONNECTION_STRING`. If the install uses a separate owner login and a run gives only the read-write string, ate-setup stops instead of replacing the owner login. Found while reviewing the upgrade runbook (agent-substrate#1844). Part of agent-substrate#1686 - [ ] Tests pass - [ ] Appropriate changes to documentation are included in the PR
…1812) `ate-setup` no longer labels nodes with `ate.dev/substrate-version`. The atelet DaemonSet is now named `atelet` and runs on every node, instead of `atelet-<version suffix>` pinned to nodes carrying its version. The label let two Substrate versions run side by side on disjoint sets of nodes, which the clone-WorkerPool upgrade runbook relied on. The GA upgrade doesn't split nodes by version: atelet rolls in place and worker pools roll as ordinary Deployment updates. With the label, deploying a new atelet creates a second DaemonSet pinned to a version no node carries, so it schedules nowhere. This merges together with agent-substrate#1844, which replaces the label-based runbook in `docs/upgrade.md`. #### Breaking change Due to the removal of version label system. An install from v0.4 or earlier can't be upgraded in place. Reinstall it, as with every pre-GA minor release. Part of agent-substrate#1686 - [ ] Tests pass - [x] Appropriate changes to documentation are included in the PR
Upgrading ate-controller first did not save a worker roll. When a release changes the worker pod template, the controller step rolls every pool on its own. It also had a new ate-controller call an older ate-api-server, which rejects fields it does not know, and had an older ateom start with flags from a newer ate-controller. Move the CRDs and ate-controller into step 4, after ate-api-server. New workers then start with the flags the old ate-controller renders, and ate-controller only calls an ate-api-server at least as new as itself. Rollback runs the same order in reverse. The atunnel broker identity comments no longer refer to the runbook order.
ate-setup now records the settings of each deploy run in its cache directory, in the format --config reads. Before you start says where to find that record, and the TODO that waited for it is gone. A later deploy run replaces the record, and it holds no database connection strings. So the step asks the operator to check the copy and to export the connection strings when the database is external. It also asks them to point kubectl at the cluster first, because every command in the runbook uses the current context.
Releases do not publish prebuilt images for now, so the runbook no longer waits on where they would live or which tag they would carry. The three TODOs left say what each waits for and what changes in its step once decided: the two policy docs to link, whether a compatible release may change the worker pod template, and per-release SandboxConfig names.
A full upgrade on a GKE cluster showed places where the runbook does not match what the commands print or what happens to actors. ate-setup prints nothing while it waits for the atelet roll, so the kubectl rollout status that followed it only ever printed the final line. Run it in a second terminal instead. kubectl rollout status prints "X of N updated replicas are available" only after all N replicas are updated, so it cannot tell the operator that a pool's first new worker is up. Check the newest ReplicaSet's READY count instead. publish worker-images now prints the configuration report before the refs, so read NEW_IMAGE from its output with sed rather than by hand. Nothing suspends actors during the worker roll. A running demo actor that exited on SIGTERM crashed seconds into the window. Say so in step 3, and leave a TODO for the eviction section of the API guide. The cluster had no record of which release was installed. The install record now carries it as cluster.substrateVersion, so Before you start points there for OLD_RELEASE. WorkerPool now names its sandbox class in the first entry of spec.sandboxClasses. Step 3 reads CLASS from there, in the saved pool list and for each pool.
Operators need to know whether their upgrade path is supported before they start. The runbook used to say only that it works within one v0.x window. It now lists the cases: patch releases can be skipped, minor releases go one at a time, a new major needs a reinstall, and before 1.0 only patch releases of the same v0.x upgrade in place. It also says how far a rollback goes. The policies these rules come from are not in the repository yet, so the TODO now only waits on linking them.
Each release will ship its default SandboxConfig under its own name, with the release version as a suffix, and leave the old ones in place. A WorkerPool without a configRef moves to the newest default. One with a configRef stays where it is. Every actor start resolves the sandbox from its pool's current config, so an actor suspended under binaries its config no longer offers cold boots and keeps only its files. A separate sandbox upgrade guide will cover pinning a pool and what an actor keeps. None of this is in a release yet: deploy sandboxconfig still overwrites gvisor-default. Keep the TODO and say what the step becomes. Say now that the step does not roll workers. That holds either way, because worker pods take nothing from a SandboxConfig.
The API guide now has an Eviction section that says what an actor needs to keep its state through a worker roll. Link it from step 3 and drop the TODO that waited for it.
A review walked through the runbook the way an operator would. The commands had <...> placeholders that break when pasted into a shell. Put the three values an operator has to supply in one block with example values, read the installed release from the install record, and look up a pool's old image in the saved list instead of copying it out of a table. Say where the record is by default. Step 3 told the operator to watch rollout status, which blocks until the whole pool has rolled, and then to move on before the roll finishes. Drop the blocking watch from the per-pool steps and start with a small pool of each class. Say that a batch is 10% of the pool rounded down, and that a deploy atelet timeout does not stop the roll. Step 4 ran four deploys under one Done and Stuck. Give each its own, add the egress connections the atenet roll cuts, and say in the summary that workers can roll again. Add a Where am I section that lists each component's image, whose tag is its release in a prebuilt install, and remind the operator to tell actor owners before step 3. The atenet README no longer calls the router drain lossless, since a request still open about 15 seconds into the drain is cut.
The setup block now keeps an OLD_RELEASE set by hand, prints both releases so an empty one is caught before cloning, and sets images.tag in new.yaml for prebuilt installs. That edit used to be a prose step after the block, and missing it left new workers on old atelets. Also: - Before 1.0, only patch releases of the same minor upgrade in place. - Back up the database before step 4a, since rollback keeps the new schema, and check that an external database's connection string is set in the shell. - Where am I? tells what has moved from ReplicaSet and ControllerRevision ages rather than image tags, which source builds do not carry. It covers podcertificate-controller and each pool's roll progress and paused state, and gives the upgrade a final check. - The prebuilt NEW_IMAGE is a command, the needed tools are listed, and the revert command is spelled out. - A Git or CI sync of WorkerPools can put the old worker image back. - Actor owners are told before the upgrade starts, not at step 3. - The TODOs are HTML comments, so operators do not read them. - "Why this order" is gone. It explains the version skew policy, which is where it belongs once that policy is in the repository.
b31260a to
6593779
Compare
The component steps leave out what only the system bundle carries, such as the ate-system NetworkPolicies and the bundled PostgreSQL, so an upgrade never applied them. Step 4e runs deploy ate-system once every component is on the new release, with --setup-csi=none so the CSI drivers stay as they are. Rollback does not undo it.
4 participants
Rewrites the upgrade runbook for an
in-placeroll by K8s deployment.Each component is upgraded where it runs, in order: podcertificate-controller, atelet, the workers, ate-api-server, the CRDs and ate-controller, atenet, then the default SandboxConfig. ate-controller follows the API server because it is a client of it, and new workers start from the pod template the old ate-controller renders. The old runbook ran the old and new dataplane side by side instead.
This deliberately gives up two things the old runbook had.
The first is zero loss. Workers are replaced through the eviction path:
SIGTERM, then 30 minutes for the actor to be suspended beforeSIGKILL. Actors already need to handle this for node upgrades and preemption. An actor that is suspended in time loses nothing. One that isn't crashes and loses the work since its last snapshot. The old runbook avoided this by having the operator suspend every actor on a node before moving it.The second is instant rollback. Rollback is the same roll in reverse, from the old release, instead of flipping a node label back. Keeping both versions running needs spare capacity and the version label.
TODOs
Some places are marked TODO because they depend on unfinished work:
ate-setupwrites (Need an install settings from a config file to persist configurations #1887, implemented in ate-setup: save used settings locally after installation #2223)Merges together with #1812