Repository navigation
refactor: give each rule and value of runplane one owner - #11
Merged
Merged
Conversation
Each event of the bus is now a type of the events package, and the type
is the topic. A publisher cannot send the data of a different topic, and
a consumer does a type switch or reads a typed channel. events.Event{Type,
Data any}, events.As, and the JSON convert of internal/api are removed.
events.PoolStats is the one pool.stats type for the scheduler, the SSE
stream, and the OpenAPI schema. events.HostStats is store.ServerStat with
the name of the SSE schema. The copies in internal/api and the scheduler
are removed. The OpenAPI spec does not change.
The bus has one delivery rule for each kind of subscriber:
- events.Subscribe[E] gives each event of one type and never loses one.
When the subscriber is slow, the events wait in a queue without a size
limit. notify, github, and fleet use it.
- Bus.Watch gives all events to a client that can read the state again.
When more than buffer events wait, the channel closes. The SSE stream
then ends, and the browser connects again and reads all its queries.
Before, a full buffer of 64 or 256 events lost settings changes and job
failures for github and notify without a record. A browser lost events and
kept old data until it connected again.
The image row from the API and the image build from the fleet are two
types, ImageState and ImageBuildState. The fleet provisions the images
only for ImageState, as before. The SSE stream sends the two types as
image.state.
The new package internal/sandbox/guest holds the contract between runplane and runplane-init. Before this change, init, control, the two drivers, and image each had copies of parts of the contract. The package holds: - The guest layout: the ctl folder and socket, the vsock port, the scratch folders, the drive order, the kernel options, the init path, the mount points, the runner user, and the managed label. - The control channel protocol: the paths, the hold body, the shell upgrade names, the status rules, the yamux setup, and the log stream format with a calculated buffer size. - A guest client. init and the tests use it in place of their copies. - The runner patch (Patch and Unpatch) and the cache env. - NewConfig. The two drivers use it in place of their guestConfig and hostname copies. Behavior changes: - The VM driver adds the cache env only for a patched runner, as the container driver does. - The container driver sends only the IPv4 DNS servers in the config. Init uses them only in VM mode. - The config has an ActionsCache field. Init removes the runner patch when the field is false. Before, init looked for ACTIONS_RESULTS_URL. - A nil config replaces the empty JIT config as the sign of a sandbox from before a runplane restart. - The log stream cuts a source name of more than 16 bytes. Before, the host divided a long line with such a name. As a result, sysbox does not import internal/cache, and its build does not include store and SQLite. init does not import control. A new test compares the builtin Dockerfiles with the layout constants.
…sion internal/version now has Compare and IsRelease. Before this change, three rules decided if a version is a release: isRelease in cmd/runplane, the releaseVersion regex in internal/fleet, and parse in internal/update. The rules did not agree. Thus a v1.0.0-rc.1 primary did update checks, but its agents got no RUNPLANE_VERSION and installed the latest stable release. IsRelease accepts the tag of a release that release.yml makes, also a pre-release such as v1.3.0-rc.1. It rejects "dev" and the git describe output of other builds, such as v1.2.0-3-g3f2a9c1 and v1.2.0-dirty. update.Check, update.Apply, the fleet, and checkDaily use IsRelease. internal/store, internal/image, and internal/fleet/provision.go imported internal/update only for Compare. They now import internal/version, which has no imports.
runplane had three GitHub REST clients: internal/github/rest.go, update.Check, and the actions/runner release read in internal/image. They had copies of the headers and the base URLs, and only rest.go tried again after network errors and rate limits. The other packages could not use rest.go, because it was in the large github package. The new package internal/ghrest imports only the standard library and internal/version. It holds the headers, the base URLs, the retries, the rate-limit waits, the pagination, the redirect option, and the exported APIError. Download gets a file outside the API, such as a release asset, without the API headers and without a token. Anonymous calls to one API URL share one rate limit, because GitHub counts them by IP address. After GitHub limits an anonymous call for more than one minute, the other anonymous calls fail at once until the limit ends. Only an Auth with a cached credential (Renewable) gets a second try after a 401. Thus the app JWT, which cannot change, gets no useless second try. internal/github, internal/update, and internal/image now use internal/ghrest. The update check and the downloads of releases now try again after network errors and 5xx answers. TestUpdaterCheck uses a 403 fault, because a 5xx fault now gets two more reads.
…gests Thirteen places wrote a temp file and renamed it by hand, each with a different temp name, and seven of them did no sync. The cache and the mirror each had a copy of countingWriter. The mirror and the images had two different checks of the "sha256:<hex>" form. atomicfile now also has a streaming writer. Create and CreateIn make a temp file, Commit syncs it and renames it, and Close removes it if Commit did not run. The file stays open after Commit, because the mirror streams a blob to the clients from the temp file. CreateIn puts the temp file in a different folder, because the cache and the mirror keep temp files out of the folders that they scan. These writes now use atomicfile: the cache blobs and blocks, the mirror blobs, manifests, and tags, the runner stamps, the ext4 drives, the update binary and guest kernel, the VM and container records, the backup files, and the scratch templates. The new package internal/blob holds the digest form (Digest and ValidDigest) and Serve, which serves a file and counts the bytes. The cache, the mirror, and the images use it. The cache, the runner stamps, the tag files, the driver records, and the scratch templates now sync before the rename. The mirror syncs a file before it takes the store lock.
# Conflicts: # internal/image/runner.go
Each GET endpoint has one query in web/src/api/<resource>.ts. The cache helpers (put, drop, refresh) use the queryKey of these queries, thus no code writes the key shape of openapi-react-query by hand. The page folders keep no query modules, and the import cycle between servers and server-detail does not occur. The event stream gets the event names and payloads from schema.d.ts. One typed map in api/events.ts gives what each event writes into the cache and the queries that it makes stale. The stream and the mutations whose answer is the object of an event (applyChange, refreshServers, dropServer) use this map. pool.stats and host.stats write into the API caches, thus each page reads one value. job.state also refreshes /api/status, thus the first-job step does not refresh the status itself. One EventSource serves the app layout and the first-job step. One useFollowedText hook streams the job log and the runner log. One formProblems helper divides a submit error into field messages and one alert. requireSession and useIsAdmin replace the copies of the guards and of the admin checks. Each enum has one label map next to its icon or dot, and lib/format has the one relative-time formatter.
internal/image held two features: the OCI images and the runner bundles. The new package internal/runner now holds the runner bundles: the release reads of actions/runner, the download, the SHA-256 check, guest.Patch, the ext4 drive of VM mode, and the prune. internal/image holds only images. The two packages shared keyLock, writeExt4, and tarDir. These moved to two leaf packages: internal/keylock and internal/ext4. The test helpers for ext4 files moved to internal/ext4/ext4test. One Releases type reads a release by tag for the bundles and the latest release for the version check. Its API URL, web URL, and auth are config values, thus the tests do not change private fields. The release read rejects a tag that is not a runner version. The old release function gave a version that only the tests read. The callers (cmd, agent, fleet tests, sysbox, devdocker, and sysboxtest) change only the import and the type name.
The two Dockerfiles had 120 of 124 lines the same. One images/Dockerfile
now builds each builtin image. The build args UBUNTU and IMAGE_OS select
the Ubuntu release.
The images package now holds the facts of the builtin images:
- Names and Default
- Ref, the registry reference ghcr.io/<repo>/runner-<name>:latest
- Prefix ("builtin:") and Source ("builtin") of the image rows
- BuildArgs, the build args of each image
The fleet seed, internal/image, and internal/api/images.go use them. The
seed no longer reads the folders of images.FS. internal/image gets the
repository of the builtin images from its config and not from the global
version.Repo, thus the integration tests do not change the global.
The embedded build sends the build args. It no longer adds the managed
label, because the Dockerfile has it. The images workflow builds the same
images with the same build args. A test compares images.yml and the table
in images/README.md with Names and BuildArgs.
The migrations must have numbers with no gaps. Work package D adds the migration 0005, and this branch adds 0006. This empty file keeps the numbers of this branch complete. Remove it when the migration 0005 of package D merges.
The runner-version check was in the GitHub adapter. It started the bundle builds of the fleet, wrote the runner_bundles rows, and sent the end date warnings. The fleet also wrote the rows. This was the only reason for the notify import and for the Bundles port of internal/github. The new service internal/runner/versions owns the runner_bundles rows: - Built records the bundle build of one server. The fleet calls it. - Each hour, the service reads the latest actions/runner release and the end dates of the known versions, and writes them in the rows. - It warns about each version that the servers give new sandboxes and that ends in less than 14 days. The service talks to GitHub through ghrest and a narrow interface: Token and ReadScope. internal/github keeps only ReadScope, the organization or repository that the app can read. The fleet no longer has EnsureLatest. Its provision loop already makes the newest version of the rows ready on each server each minute. Thus a new release has one path to the servers, with the same retry rules. Bugs that this corrects: - The sent warnings were only in memory, thus each restart in the last 14 days sent the warning again. The new column runner_bundles.warned (migration 0006) keeps them. - The warning used the newest ready version of each arch. A server that cannot download that version gives new sandboxes an older version, and its end got no warning. The warning now uses the versions of the servers (Fleet.RunnerVersions). The rows no longer keep the bundle folder of the agent (path), because no code reads it. RunnerBundle.EndsSoon holds the 14-day window for the warning and the API. UpsertRunnerBundle and LatestReadyBundle had no production callers and are gone.
The cache and the registry mirror use one data disk, but each had its own limit. The cache used cache_max_gb (at most a quarter of the disk) and min_free_disk_percent. The mirror used a tenth of the disk (2 to 50 GiB), read one time at the start, and ignored min_free_disk_percent. Thus the cache removed its entries to free the disk that the mirror used. The new package internal/datadisk has one function, Limits. It gives the two limits from the settings and the disk size, and the free-disk rule for the two caches. The cache server, the mirror setup in cmd/runplane, and through the cache server the API use it. The mirror now keeps no new content while the disk is low, and it removes old content to free the disk. The API no longer calculates the cache limit again. The cache server and its store API: - The cache accepts a small Store interface, not *store.Store. - MatchCacheEntry takes one ref. The cache holds the search order. - DeleteCacheEntry takes an ID. Only the API examines the delete input. The adapter in cmd/runplane is now the cache server itself. - The rows of the unfinished uploads keep the bytes of their files. Thus CacheSize, and the size that the API shows, agree with what the cache enforces. - No code reads or writes the path column. The cache finds the blob of an entry from its ID. - The server owns its hourly counts, and its Run writes them. The store alone calculates the hour bucket. The cache config has a clock and the URL of the results service, and the mirror config has a clock and the upstream URL. The tests use them and no longer change private fields.
The store controls the state changes of jobs and sandboxes (X14): - Package sandbox gives the live states and the rank of the states, and the store makes its SQL from them. A test compares the live term with the index of migration 0002. - UpsertJob gives the state before the message. ExpireQueuedJobs uses the merge rules of UpsertJob. - LinkJobSandbox writes the job and the sandbox in one transaction. - The new column result_guessed identifies a result that is an estimate of runplane. Thus a cancel from GitHub and a cancel estimate are different, and a result from GitHub replaces each estimate. - One query, PoolCounts, gives the jobs that wait for a runner. The pool.stats event and GET /api/pools use it. - The store methods for jobs, scale sets, pools, images, and channels get the time from the caller. - Each package removes its own expired rows. The scheduler keeps only the retention of jobs, sandboxes, and logs. Package github holds the scale set protocol (X15): - Package github changes the listener messages and statistics into scheduler types. Thus the scheduler does not import actions/scaleset. - Enrich, DownloadLog, and Rerun give data and do not write the store. The scheduler records the data and writes the log file. - Only the scheduler keeps CheckJob results. - ghrest.Account is the one type for organizations and personal accounts. - This change removes the GHES host code, the base parameter of Callback and Setup, and the stored client and webhook secrets. Migration 0005 removes the columns of the secrets. - While a listener runs, only the listener writes the status of its row. This corrects F7. - The scheduler has a small Driver interface, and its test fakes keep "connected" and "online" apart. - Package github subscribes to settings changes in New. Thus Run gets all changes. One function in the new package internal/policy makes the decision about jobs of public repositories and fork pull requests (X7). The scheduler and the github targets use it.
The web had copies of API limits that the Go API types did not have: the minimum length of a password, the format of the setup code, and the maximum length of a pool label. Thus the spec could not give them. The new passwords of setup, invite, and PATCH /api/me get minLength. The setup code of POST /api/setup and GET /api/setup/host gets a pattern. The pattern accepts lowercase letters and one dash or space between the groups, as hashCode does. The pool label gets maxLength 63, which its pattern already sets. pnpm gen:api now also writes web/src/api/limits.ts with scripts/gen-limits.mjs. This module has the limits, enums, and defaults of the fields of each schema and of each query parameter, as values for the forms. A new step of the web CI job runs pnpm gen:api and then git diff --exit-code. Thus the step is not successful when openapi.json, schema.d.ts, or limits.ts is different from the output of the Go API.
The pages had copies of the same widgets: 8 confirm dialogs, 8 sets of skeleton rows, 6 load errors, 2 error hints, 3 lists of doctor checks, and 4 optimistic saves. components/app now has ConfirmDialog, SkeletonRows, LoadError, NotFound, ErrorHint, and DoctorChecks. api/optimistic-patch.ts has useOptimisticPatch, because api/settings.ts uses it. JobAccess now uses SwitchSetting. Thus a rejected change sets only its own key to the old value, not the full settings object. A route file now has only the search, the guards, the loader, and the pending UI. It gives the params and the search to a page in components/pages. Each page module exports prefetch, thus the loaders do not know the queries of dialogs and sections. The pages do not use route IDs. The empty Overview and setup step 3 use one first-job module. The doctor dialog, Server detail, and the setup host check use one order of the checks and one doctor run. Pages use the ui Fieldset, CheckboxGroup, Meter, Progress, and IconButton where they made these parts again. The forms read the limits and enums from web/src/api/limits.ts, not from copies: the setting ranges, the pool sizes, the label pattern, the setup-code pattern, the password minimum, and the status and range lists. The component gallery moved to web/src/dev. The router adds its route only in dev, so the production build has neither the route nor the gallery. The gallery now shows the app data of the Jobs page and of the first-job module, and the new shared widgets.
# Conflicts: # cmd/runplane/primary.go # cmd/runplane/services.go # internal/github/github.go # internal/github/helpers_test.go
…le core The sandbox package holds the complete driver contract. Driver has Probe and a required Configure. ErrNotFound, ErrNoProbe, and ErrProbeInService are in sandbox. Capacity tells the limits of each driver: the memory overhead of each sandbox, the maximum vCPUs, and the maximum count. The scheduler and the pool placement of the API apply the reserve and the CPU overcommit of the settings, thus the drivers do not copy them. Settings has only the fields that the drivers use. The doctor gets min_free_disk_percent through Node.Doctor. Host moved to the scheduler. LabelSandbox and Spec.Labels moved into sysbox. One method, RunnerEvent.State, changes a runner event into a state. FindByIP has one rule for the address form and the live states, and its three callers use it. The new package internal/sandbox/lifecycle holds the parts that the two drivers had as copies: the record file, the state machine, the deadline with one grace, the end state, the event queue, Logs, Shell, Hold, the metrics, and the takeover after a restart. firecracker and sysbox give only a small backend. sysbox has two named profiles, Container and Dev, and does not import internal/config, internal/image, or internal/host. devdocker owns the rules of dev mode: no Actions cache, the loopback address at the service port, and the RUNPLANE_DEV_INSECURE check. netrules.Manager owns the TAP range and opens and closes full TAP leases. cmd/runplane makes the Manager in container mode and in VM mode. Behavior changes: - At the time limit, a sandbox ends as failed in the two modes. - The grace after the time limit is 5 minutes in the two modes. - A failed Start sends no events in the two modes. - Stop with an ended ctx returns at once, and the stop continues. - An agent gets its settings before the scheduler can start a sandbox.
# Conflicts: # cmd/runplane/agent.go # cmd/runplane/local.go # cmd/runplane/primary.go # internal/fleet/fleet.go # internal/sandbox/devdocker/devdocker.go # internal/sandbox/sysbox/driver.go # internal/sandbox/sysbox/sysboxtest/sysboxtest.go # internal/scheduler/contract.go # internal/scheduler/events.go # internal/scheduler/fakes_test.go
The backup service moves into internal/backup with its daily schedule and its S3 step. A failed S3 upload now records the status failed and keeps the file. Before, the row had the status done with an error, and the web did not show "S3 upload failed". internal/update gets Service. One update state with one lock serves the dashboard, the auto update, and runplane update. The API and cmd/runplane keep no update state of their own. Run reads the version and the key from its Updater, not from the globals. runplane update rejects an agent, because an agent must run the release of its primary. The new package internal/joblog holds the path of the job logs, the download with its retries, the log states, the expiry with the file removal, and the read with the step parser. The scheduler and the API use it. config.Parse reads runplane.env after it parses the flags. Thus -config-dir selects the file, and no command calls LoadEnvFile first. install.sh no longer loads the file in the shell. Each command gets only its flags. fleet, api, shell, and firecracker get their ports and paths from config. Parse rejects a listen address that is not host:port. cmd/runplane no longer has the adapters for the mirror stats and the certificate status. The fleet adds the mirror stats of all servers, and the API changes the mirror and certificate types into its own types.
# Conflicts: # cmd/runplane/local.go # cmd/runplane/primary.go # internal/sandbox/firecracker/firecracker.go # internal/sandbox/firecracker/firecracker_test.go # internal/sandbox/firecracker/recover.go # internal/scheduler/fakes_test.go
The Dockerfile comment on the tool cache said "Major versions", but PYTHON_VERSIONS and GO_VERSIONS hold minor versions. The comment now tells the two types of versions.
The doc comment of fetch said that fetch downloads the archive into dir. fetch downloads the archive into a temp folder, and then replaces dir with the extracted and patched files. The comment now tells this.
The comment on ghrest.Account.Type was not correct. GitHub can send account types other than Organization and User, and Supported identifies them.
TestCleanup and TestStatsAddPerHour called the unexported cleanup, removeOrphans, and stats functions. TestCleanup now runs Run and waits for the removal of the orphan files. TestStatsPerHour makes cache requests at three times and expects hand-calculated counts for each hour. TestCacheRequestWaitsForTheJob sends a request to the fixture, not to the unexported identify. TestOpenUploadsPerSandbox finalized an upload without a blob, so the finalize was not successful. It now puts a blob first. The tests compare the bytes in the cache folder, not the names of its subfolders. The skip messages of the client tests tell what the tests need.
Without e2fsprogs, the ext4 checks of TestEnsureRootDisk only wrote a log line. The checks now run in an ext4 subtest, so that ext4test can skip them and the other checks continue. The skip message of TestRootDiskMounts tells that the test needs root for a loop device.
Without e2fsprogs, the ext4 checks of TestEnsureBundle only wrote a log line. The checks now run in an ext4 subtest, so that ext4test can skip them. The skip message of TestRealRunnerInBaseImage names the image.
Check and Debugfs wrote only a log line when e2fsck or debugfs was missing. Thus the content checks of the root disks and the runner drives did not occur, and the tests passed. The helpers now skip, and the message names the program.
TestApplyReplacesTheGuestKernel had a copy of the arch map of KernelName. At this time, the test uses KernelName to know the kernel that Apply must write. TestInstallFilesAgreeWithGo in cmd/runplane compares KernelName with the files of kernel/build.sh, install.sh, and release.yml. When Apply did not refuse a second update, TestServiceAppliesOneUpdate stopped only at the timeout of go test: the second download closed the downloading channel again, and the first download waited forever. At this time, the fake answers 409 to a second download of SHA256SUMS, and a deferred call lets the first download continue. The test then fails at once with the error of the second update.
TestSubscriptionEndsWithContext counted the sinks of the bus under its lock. Thus the test broke when the bus changed how it keeps the subscribers. Publish puts an event in the channel of each subscriber before it returns. At this time, the test publishes an event after the end of the context and reads the channel at once. When no event comes, the bus removed the subscription. Subscribe and Watch each get their own bus.
TestNextLink called the unexported nextLink. At this time, TestList reads three pages from a server with the Link header of GitHub, which also has the prev, first, and last pages. The test thus also examines the items key of a page and the end of the list.
TestStepLines called the unexported filterLines and stepLines, and TestJobLog wrote the log file with a copy of the file name of Logs. At this time, TestStoredLogOfAStep stores the log with Fetch, as the finish step does, and reads each step with Job. The test also reads step 0, which gives the whole log. TestJobLog keeps the live log, the expired log, and a job without a GitHub log.
The test calculated the SHA-256 of agent.crt again, with the same steps as Fingerprint, and its comment named an openssl command of the install script. No install script, document, or tool calculates the fingerprint outside Go. TestPinnedClientConfig already makes sure that the reported fingerprint pins the certificate of the listener.
TestWrite and TestWriteKeepsNewest called the unexported write with a VACUUM INTO of their own, so the WAL part examined SQLite and not the backup code. TestBackupsRemoveTheRowsOfPrunedFiles made an old backup with a copy of the file name format. At this time, TestCreateWritesTheDatabase makes a backup with Create and opens the gunzipped file with store.Open. TestCreateKeepsTheNewest makes three backups with backup_keep 2, and the oldest file and row must go. Thus no test has a copy of the name format. TestFreeName now requires only that the second name sorts after the first. TestUpload read the day after the upload, thus it was not successful at midnight UTC. It now takes the day from X-Amz-Date, and X-Amz-Date must be the time of the upload.
TestPasswordHashFormat called the unexported hashWithSalt. At this time, CheckPassword must accept the libargon2 reference hash, and a hash of HashPassword must pass CheckPassword, have new salt, and have at least the OWASP parameters. TestSetupCode deleted the first admin with SQL to use the code a second time. The store keeps its last admin, so no user of runplane can make this state. TestSetupRefusedAfterFirstAdmin already refuses a setup after the first user. TestCanDo had a copy of each row of the switch in CanDo. TestRoles and TestShellBridgesTerminal in internal/api examine each action of a member through HTTP, and they fail when a row of CanDo changes.
The dispatch tests called the unexported hostStats, serverState, and settingsChanged, waited on d.wg, and wrote the channel rows with the store and the box of the dispatcher. Thus they did not examine the subscriptions of Run, and the waits for a missing alert were sleeps of 100 and 200 ms. At this time, the tests publish the events on the bus and make the channels with Channels.Create. To know that the dispatcher handled some events, a test publishes an event of the same type with an alert and waits for that alert. Before a test counts the alerts, it does three of these flushes. The disk test sets a limit of 20% in the settings, so the default of 15% sends no alert, and TestDispatcherSendsAFailedBackup publishes before Run, as New subscribes at once. TestDispatcherDiskLowBeforeTheSettings is gone: Run reads the settings before the first event, so no caller can make that state. The payload, link, retry, and URL tests called send, link, and checkTarget. They now send through Dispatcher.Notify and Channels.TestURL, with the dashboard URL of the settings, and make channels with Channels.Create. Two internal values stay, because the package has no option for them: the clock of the dispatcher in the disk test, and enrichWait, attemptTimeout, and retryDelays. The HTTPS receiver of the Slack and Discord payloads still replaces http.DefaultClient.
TestShellAllowedOnlyForLiveDebugJobs called the unexported shellAllowed with a copy of the table of the shell rule, which package shell tests. At this time, TestShellNeedsALiveDebugJob asks for the shell of one sandbox for each answer of the rule, and examines the HTTP status. TestCloseReasonKeepsWholeCharacters called closeReason. At this time, TestShellErrorKeepsWholeCharacters opens a shell whose driver gives a long error, and reads the close reason in the browser. TestSetupMakesAdminWithSession had the cookie lifetime as a literal. It now requires the lifetime of auth.SessionTTL, so that the cookie and the session end at the same time.
Many tests called unexported code: signJWT, appName, appDeleteURL, matchJob, moveJobs, targets, polledChanged, reconcileNow, scalesetClient, and a setListener that the test made itself. TestManifest also read the manifest state with a copy of hashState. At this time: - The fake GitHub refuses an app JWT that GitHub refuses: a wrong issuer, an issue time in the future, or more than 10 minutes of life. Thus each new installation token examines the JWT, and TestSignJWT is gone. Its exact iat and exp values refused other correct values. - TestInstallationToken uses CheckJob, TestManifestNamesTheApp reads the name in the manifest, TestEnrichFindsTheGitHubJob uses Enrich, and TestPullRequestMovesTheJobs reads the tree of the pull request. - The scale set tests call Sync, which reads the installations of the fake. TestSuspendedInstallationPausesListener suspends the installation in the fake, as GitHub does. TestRemoveRunner uses the client that the scheduler gets. - TestTargets is gone. Its mode column had no effect, and TestRunFollowsSettingsAndPools now also has an archived repository and a disabled pool, without the mode setting that had no effect. - TestGitHubStateEvents counts the events of 10 polls of a listener. - TestAppDeleteURL is gone. TestCallbackRejectsAppOfAnotherAccount examines the delete page, and ghapp holds the one copy of the layout. The env still sets the GitHub hosts of the service, because Config has no option for them. It also gives the client a retry wait of 1 ms.
Run wrote the buffered host samples only when ctx ended in its loop. If ctx ended during the start, Run returned without the write, and it lost the samples that the local server sent after its connect. Also, the goroutines that forward the samples added samples after the write, until stop returned. Run lost these samples too. Now Run writes the samples in its deferred function, after stop. stop waits for the end of the background work, thus no sample comes after the write. TestHostStats has a second case: a stop during the start. A pools fake holds the start in the default label until the stop. Without the change to Run, each run of this case is not successful.
initbin.TestPath skips when the build has no runplane-init. The go job ran go test without make init, thus the test did not run in CI. make init writes only ignored files (bin/ and internal/initbin/bin/runplane-init) in the checkout of the go job. The other jobs use their own checkout, thus they build the same files as before. The builtin runner image has make (build-essential) and gzip, and setup-go gives go.
One Dockerfile with build args builds all builtin images. The comment named images/*/Dockerfile.
parse_flags examined --domain with its own rule: lowercase letters, digits, dots, and dashes, with one dot or more. set-domain uses validDomain, which also rejects IP addresses, empty labels, labels that start or end with a dash, and names that are too long. install.sh runs set-domain after stop_service. Thus a value such as the IP address of the server stopped the script after the stop, and runplane did not run until the admin ran the script again. The new function valid_domain is a copy of validDomain, and parse_flags uses it. Thus the script stops before it changes the host. validDomain stays the owner of the rule. TestInstallDomainAgreesWithSetDomain runs parse_flags and set-domain on the same values and compares the results.
write_file writes a new runplane.service at each run, and systemd then sees that the file of the loaded unit changed on disk. stop_service ran after write_units. Thus at each update of a running primary, "systemctl stop" showed the warning "Run 'systemctl daemon-reload' to reload units." start_service does this step some lines later. stop_service now runs before write_units, and the stop uses the unit that systemd loaded. The stop still comes before the first command of the new binary that opens the database. TestInstallStopsThePrimaryBeforeTheDatabaseCommands now also examines the position of the unit write.
…ents Some tests called unexported code: place, updateMaxRunners, resizeAll, and stopBg. Others wrote the reruns map or read the reserved map without the lock, and the steps and restart tests waited with sleeps. At this time: - TestPlacement and TestMaxRunners send a listener message to the Scaler. TestPlacement finds the server whose driver got the start. - TestConcurrentStartsDoNotOverbook has one row for each part of the reservation: the vCPUs, the memory with the overhead of the driver, and the sandbox limit. When the first sandbox stops, the second scale set must get a start. Thus a reservation that stays makes it fail. - TestDebugJobOnWarmSandbox asks for the debug re-run with RerunJob. - TestFinishStepRunsAgainAfterRestart stops Run, and Run of a new scheduler does the finish step again. The test waits for the log download and for the job event. - TestStepsRefreshWhileTheJobRuns waits for the job events of the run data and of the steps. After the completion, the background work must end in 10 seconds. - TestDetachStopsClientCalls lets the failed start continue only after Detach returned. Before, a slow goroutine could make it fail. - TestCheckErrorDoesNotBlockTheMessage takes the job after the GitHub outage, in place of a total count of the CheckJob calls. - New TestDashboardChangesResizeTheScaleSets: Run resizes the scale sets at the PoolsChanged and SettingsChanged events. No test examined these subscriptions, which replaced a PoolsChanged call of the API. The tests set the time limit, the debug hold, and the retention days, so that a change of a default does not change them. The runner name must be the name of the JIT request and IsRunnerOf the pool label, in place of a copy of the name format. The retention test writes and reads the logs with joblog, in place of a copy of the log path. The fake driver counts the overhead of each sandbox, as the lifecycle core does. The fake GitHub returns no steps for a job without a GitHub job ID, as the github package does. The tests still call cleanup, retention, and flushMetrics, and wait on the wait group of the scheduler, because Run uses real tickers. The steps test still changes stepsEvery.
…ction TestSettingsDefaultsAndUpdate and TestOpenMigrations wrote and read the database through the unexported writer and reader of the store. They now use a separate SQLite connection, as a runplane of another version does. The comment of TestListAndCountJobsFilters said that CountJobs repeats the conditions of ListJobs. The two use one function, and the test compares the count with the literal list of each filter.
Publish gives the event to the channel of the subscriber before Update returns. Thus the test reads the channel at once, in place of a wait of 5 seconds for the event and a sleep of 50 ms for the event that must not come.
The tests of set-mode, set-domain, setup-code, and update called the functions of the commands. They now call run with the command name, as install.sh does. Thus a wrong row in the command table makes them fail.
…stStop After Stop returns with its ended ctx, the stop continues and asks the guest to shut down. The test marked the guest as exited before that request in some runs, and then the backend calls had a different order. The fake now sends a signal at each Shutdown, and the test waits for it.
ephraimduncan
marked this pull request as ready for review
November 6, 2026 11:00
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This refactor puts the parts of runplane that change together in one place. Before, many rules and values had copies in two to six places: Go packages, the web,
install.sh, and SQL. A change to one copy did not change the others, and no compile step or test found the difference. Several copies already disagreed, and the users saw the result as bugs. Now each rule has one owner. Where a copy must stay in a different language, a test compares it with the owner and names the files that must change together.The new boundaries
The host and runplane-init share one contract in
internal/sandbox/guest: the guest file layout, the control-channel protocol, the log format, the runner patch, and one constructor for the guest config. Before, init and the host each had their own copy of the routes, the paths, and the patch code.Firecracker and Sysbox share one lifecycle core in
internal/sandbox/lifecycle. Each driver gives only a small backend. Before, the two drivers had about 15 copies of the same lifecycle code, and the copies gave different results at the time limit and at a failed start. Each driver package also gives its own doctor checks, with the same binary paths, users, and ID ranges that the driver uses.internal/hosthas only host facts and no branch on the mode.The API handlers call services (
settings,pools,auth,notify,backup,update,fleet). Each service writes the store, records the audit entry, and sends the event in one place. Before, about 65 handler calls wrote the store and added these side effects by hand. Each event on the bus has its own Go type, and internal consumers do not lose events when a buffer is full.internal/policyholds the mode rules and the public-repository rule for the scheduler, the API, and the GitHub scale sets.The web reads the Go rules from the API. The limits and enums come from
openapi.jsonthrough a generatedlimits.ts. The API gives the results of the policy rules as fields, for example the capabilities of the session, the actions of each server, and the reason that a server waits. CI makes sure that the committed spec,schema.d.ts, andlimits.tsagree with the Go API. A Go test compares the dashboard paths that Go writes into links withrouteTree.gen.ts.TestInstallFilesAgreeWithGoreadsinstall.sh, the Makefile, and the release workflows, and compares approximately 35 values with Go. Some of these values are the ports, the paths, therunplane-vmID range, the nft tables,SYSBOX_MIN, the supported systems, and the archive names. A second test runs the--domaincheck ofinstall.shand the Go check on the same values.Bugs that the refactor corrects
*.example.comand IPv6 entries, but the firewall ignored them. Thus the sandboxes could not reach these hosts, and nothing told the admin.install.shmade a second code.install.shmakes the one setup code before the first start.runplane setup-codesandbox.ssh. A password reset from the shell had no entry.reset-passwordwrites an entry.Behavior changes to examine
At the time limit, a sandbox now ends
failedin the two modes, with a grace of 5 minutes. Before, a VM endedgoneafter 1 minute, and a container endedfailedafter 5 minutes. Withfailed, the scheduler also removes the GitHub runner. A start that is not successful sends no events, and the scheduler records the end.While
allow_public_reposis on, only servers in VM mode get runners. The other servers wait, and the servers page, the server page, and the label editor show the reason. When no server can run a public job, the job waits in the queue with no reject reason, as a private job does. Jobs that runplane recorded before migration 0011 count as private.In VM mode, a runner without the cache patch no longer gets the Actions cache env. Sysbox already had this rule. The
CheckJobcache now keeps a result for 10 minutes, not 1 hour. Thus a repository that becomes public shows sooner, and GitHub gets a few more reads. The results proxy of the cache now refuses loopback clients that are not sandboxes.During an update with
install.sh, the primary service stops before the new binary opens the database, and it starts again at the end. The sandboxes continue. Ifset-modeorset-domainis not successful after the stop, the primary stays stopped, and the error message tells this.install.shnow refuses a--domainthatset-domainrefuses, before it changes the server.runplane updateon an agent refuses, because an agent must run the release of its primary. A command flag that the command does not use is now an error. The doctor shows its checks in a new order, and the disk check fails at exactlymin_free_disk_percent, where the scheduler stops. An agent of the previous release uses 0% for its doctor disk check until it updates.The migrations 0005 to 0011 add five columns, and drop six columns and one index that no code reads. The store refuses a schema that is newer than the binary, as before. Thus, to go back to an earlier release after this change, you must restore a backup.
Visible changes
install.sh