diff --git a/docs/git-stacks.md b/docs/git-stacks.md index 82c4cf7a..cd48a88e 100644 --- a/docs/git-stacks.md +++ b/docs/git-stacks.md @@ -37,12 +37,13 @@ to your shell profile. - **The clone** lives on your array, by default under `/mnt/user/appdata/compose.manager/git/-`, never on the flash drive. Each git stack has its own clone of the whole repository. - **The stack folder** in the projects folder holds the stack's settings (`git.json`), what was last deployed - (`git_state.json`), the stack's `.env` and the plugin's override file. These are never inside the clone, so - pulling new commits never touches them. + (`git_state.json`), the stack's `.env` and the plugin's override file (and, until the next deploy that works, + `git_discarded_changes`, see [below](#config-files-and-recreating-containers)). These are never inside the + clone, so pulling new commits never touches them. - **A deploy changes no container until everything else has worked.** It fetches the commit, checks it out, checks it (see [below](#what-is-checked-before-a-deploy)), pulls and builds images, and only then runs - `docker compose up`. If anything before `up` fails, the previous commit is put back and your containers are - exactly as they were. + `docker compose up`, which neither builds nor pulls again, whatever a service's `pull_policy` says. If + anything before `up` fails, the previous commit is put back and your containers are exactly as they were. - **Nothing of yours is deleted.** Files the plugin replaces are moved into a backup folder, local changes are saved as a patch before they are discarded, and an old clone is moved aside rather than removed. The plugin removes only a clone it has just made itself, when setting it up fails (for example, the branch has no @@ -184,7 +185,8 @@ unchanged, the stack keeps its volumes and networks. Check that it works. ### 8. Tidy up Once you are happy, delete the `pre-git-` folder. Until then it is your way back: to undo the -conversion, delete `git.json`, `git_state.json`, `indirect` and `indirect_mode` from the stack folder, then +conversion, delete `git.json`, `git_state.json`, `git_discarded_changes` (if there is one), `indirect` and +`indirect_mode` from the stack folder, then copy the backup folder's files back into it (they include the old `indirect` settings, if the stack had any). ## Day to day @@ -229,6 +231,9 @@ With no custom env file set for the stack, a git stack uses, in order: Only one of the two is used; they are not merged. Editing a `.env` that came from the repository changes the clone, so the next deploy stops with "local changes": make the change in the repository instead. +Variables exported in the shell you run `compose-git deploy` from are not passed on, and `${PWD}` in the compose +file is the compose file's folder in the clone: a deploy uses only what its checks saw. Put values in the `.env`. + If the `.env` sets `COMPOSE_FILE` with relative paths, they are relative to the folder the `.env` is in. For the stack folder's `.env` that is the stack folder, not the clone, so use paths relative to the clone only in a `.env` committed next to the compose file. @@ -286,6 +291,10 @@ a config file the container mounts would otherwise be checked out but never take So by default, **when a deploy changes anything in the stack's folder in the repository, every container in the stack is recreated**, and the new config takes effect. +A deploy that saves local changes in the stack's folder as a patch and discards them recreates every +container too, even at the same commit: a container may still hold an edited file. Until a deploy works, the +stack folder holds a `git_discarded_changes` file that says so. + Only the stack's own folder counts: a change to a shared folder elsewhere in the repository does not recreate anything yet. To turn the behaviour off, set `"recreateOnFolderChange": false` in the stack's `git.json`. @@ -308,6 +317,13 @@ is, the deploy stops instead. - **Before `up`** (a check fails, the pull or build fails): the previous commit is put back and no container was changed. Fix the cause, usually in the repository, and deploy again. + + "No container was changed" is about the containers themselves. A running container that mounts a file or + folder from the clone (`./config`, say) sees the new commit's files from the moment it is checked out, while + the checks, pull and build run, and the old ones again once it is put back. Most apps read their config only + when they start, so this does not matter to them. For an app that reloads its config when the file changes + (a proxy watching its config folder, say), keep that config at an absolute path outside the clone if a + deploy that is put back must not reach it. - **During `up`** (a container fails to start, or does not get healthy with `--wait`): a half-finished `up` cannot be undone safely, so it is not rolled back. The commit is recorded as failed and shown by `compose-git status`. Fix it in the repository and deploy again, or deploy a known good commit with @@ -336,7 +352,8 @@ without changing anything when: folder, is missing; - a `container_name` or published port is already taken by another stack or container, or two services of the stack publish the same port; -- Docker could not be asked whether a name or port is free (the daemon is not answering, say). +- Docker could not be asked whether a name or port is free, or whether an external network or volume exists + (the daemon is not answering, say). The deploy log says which `.env` was used. diff --git a/source/compose.manager/include/DockerCommand.php b/source/compose.manager/include/DockerCommand.php index f19cb809..dddd6e04 100644 --- a/source/compose.manager/include/DockerCommand.php +++ b/source/compose.manager/include/DockerCommand.php @@ -28,6 +28,9 @@ public static function run(array $args, ?string $workingDirectory = null, int $t $environment['DOCKER_CONFIG'] = $dockerConfig; } + // As a shell would set it: a compose file can use ${PWD}, and compose.sh's runs see it too. + $environment['PWD'] = $workingDirectory ?? '/'; + return ProcessRunner::run( array_merge([COMPOSE_DOCKER_BIN], $args), $workingDirectory ?? '/', diff --git a/source/compose.manager/include/GitCommand.php b/source/compose.manager/include/GitCommand.php index 5ea5bddb..b6688e75 100644 --- a/source/compose.manager/include/GitCommand.php +++ b/source/compose.manager/include/GitCommand.php @@ -62,7 +62,13 @@ public static function run( array $extraEnvironment = [], array $trustedRepositories = [] ): ProcessResult { - $homeDirectory = self::prepareHomeDirectory(); + $homeDirectory = COMPOSE_GIT_HOME_DIR; + $homeProblem = self::homeDirectoryProblem($homeDirectory); + if ($homeProblem !== null) { + // 128, git's own code for "could not run": some callers read exit code 1 as an + // answer (merge-base --is-ancestor says "no" with 1). + return new ProcessResult(128, '', $homeProblem, false); + } $command = ['git']; foreach (self::FORCED_CONFIG as $setting) { @@ -142,14 +148,29 @@ private static function trustFile(string $homeDirectory, ?string $workingDirecto } /** - * The empty folder git runs with as HOME, so no personal config is read. + * Make the folder git runs with as HOME, so no personal config is read, and + * say what is wrong with it, if anything. + * + * It is in /var/tmp, where anyone can create things, and the global config + * git reads is kept in it: a folder someone else made first (or a symlink + * there) would let its owner give git a config of their own. So only a plain + * folder owned by whoever runs this (root on Unraid) is used. */ - private static function prepareHomeDirectory(): string + private static function homeDirectoryProblem(string $home): ?string { - $home = COMPOSE_GIT_HOME_DIR; - if (!is_dir($home)) { - @mkdir($home, 0700, true); + if (!file_exists($home) && @readlink($home) === false && !@mkdir($home, 0700, true) && !is_dir($home)) { + return "Could not create $home for git."; + } + clearstatcache(true, $home); + if (@readlink($home) !== false || !is_dir($home)) { + return "$home is not a plain folder, so git was not run. Remove it."; + } + if (@fileowner($home) !== (function_exists('posix_geteuid') ? posix_geteuid() : 0)) { + return "$home belongs to another user, so git was not run. Remove it."; + } + if (!chmod($home, 0700)) { + return "Could not make $home private, so git was not run."; } - return $home; + return null; } } diff --git a/source/compose.manager/include/GitDeploy.php b/source/compose.manager/include/GitDeploy.php index dda59159..6a9c352f 100644 --- a/source/compose.manager/include/GitDeploy.php +++ b/source/compose.manager/include/GitDeploy.php @@ -21,6 +21,15 @@ */ final class GitDeploy { + /** + * A file in the stack folder that says local changes to files in the stack's + * folder in the clone were discarded, and no deploy has succeeded since. The + * containers may still hold the discarded files (a single-file bind mount keeps + * the old copy), but no commit differs, so only this tells the next "up" to + * recreate them. + */ + public const DISCARDED_CHANGES_FILE = 'git_discarded_changes'; + /** @var callable(string): void */ private $say; @@ -113,6 +122,11 @@ public function upArguments(string $previousCommit): array $clone = new GitClone($settings); $current = $clone->checkedOutCommit(); + if (is_file($this->stackDir . '/' . self::DISCARDED_CHANGES_FILE)) { + ($this->say)("Local changes in the stack's folder were discarded, so every container is recreated."); + return ['--force-recreate']; + } + $compareFrom = [$previousCommit]; $deployed = GitStackState::load($this->stackDir)->deployedCommit; if ($deployed !== null && $deployed !== $previousCommit) { @@ -156,6 +170,12 @@ public function finish(bool $succeeded): void ? new GitStackState($commit, null) : new GitStackState($state->deployedCommit, $commit); $newState->save($this->stackDir); + if ($succeeded) { + // The containers were just made from the clone as it is. If this cannot be + // removed, every deploy recreates the containers until it is (it is in the + // stack folder, and removing it by hand is safe). + @unlink($this->stackDir . '/' . self::DISCARDED_CHANGES_FILE); + } } /** @@ -283,9 +303,45 @@ private function dealWithLocalChanges(GitClone $clone, GitStackState $state, str if ($patch !== null) { ($this->say)("Saved the local changes to $patch"); } + // Recorded before the discard, so a failure here leaves the changes in place. + if ($this->anyInStackFolder($clone, $changedFiles)) { + $this->recordDiscardedChanges(); + } $clone->discardLocalChanges(); } + /** + * Whether any of these paths (relative to the repository) is in the stack's folder. + * + * @param string[] $paths + */ + private function anyInStackFolder(GitClone $clone, array $paths): bool + { + $stackFolder = $clone->stackFolderInRepo(); + if ($stackFolder === '') { + return $paths !== []; + } + foreach ($paths as $path) { + if (str_starts_with($path, $stackFolder . '/')) { + return true; + } + } + return false; + } + + private function recordDiscardedChanges(): void + { + $stackDir = rtrim($this->stackDir, '/'); + if (str_starts_with($stackDir, rtrim(COMPOSE_GIT_MNT_DIR, '/') . '/')) { + GitPathGuard::assertSafeToWrite($stackDir); + } + $file = $stackDir . '/' . self::DISCARDED_CHANGES_FILE; + $content = date('c') . "\n"; + if (file_put_contents($file, $content) !== strlen($content)) { + throw new RuntimeException("Could not write $file, so the local changes were not discarded."); + } + } + /** * @param string[] $profiles * @throws RuntimeException listing every problem found diff --git a/source/compose.manager/include/GitDeployCheck.php b/source/compose.manager/include/GitDeployCheck.php index 11db8f19..d7498ec4 100644 --- a/source/compose.manager/include/GitDeployCheck.php +++ b/source/compose.manager/include/GitDeployCheck.php @@ -173,8 +173,14 @@ private function missingExternalResources(array $resolved): array continue; } $name = (string) ($definition['name'] ?? $key); - if (!DockerCommand::run([$kind, 'inspect', '--', $name], null, 30)->succeeded()) { + $inspect = DockerCommand::run([$kind, 'inspect', '--', $name], null, 30); + if ($inspect->succeeded()) { + continue; + } + if (self::failedOnlyOnMissing($inspect, $kind)) { $problems[] = "The external $kind '$name' does not exist. Create it before deploying."; + } else { + $problems[] = "Could not check whether the external $kind '$name' exists: " . self::failureText($inspect); } } } @@ -254,6 +260,15 @@ private function checkBindSource(string $source, string $cloneReal): ?string } if (GitPathGuard::findMountFor($source) === null) { + // A tmpfs the user mounted inside a disk or pool (a RAM transcode folder, say) is + // meant to be in RAM. A missing folder in it, empty after every reboot, is left for + // Docker to create, as below an existing share. + if (GitPathGuard::isOnRamMountInsideADisk($source)) { + if (!file_exists($source)) { + $this->foldersDockerWillCreate[] = $source; + } + return null; + } return "the bind mount $source is not on a mounted disk, pool or share. " . 'Docker would create it in RAM. Check the path for a typo.'; } @@ -520,6 +535,33 @@ private static function failedOnlyOnMissingContainers(ProcessResult $inspect): b return true; } + /** + * Whether "docker network inspect" or "docker volume inspect" failed only because + * nothing has that name, rather than because Docker could not be asked. + */ + private static function failedOnlyOnMissing(ProcessResult $inspect, string $kind): bool + { + if ($inspect->timedOut) { + return false; + } + $lines = array_filter(array_map('trim', explode("\n", $inspect->stderr)), static fn(string $line): bool => $line !== ''); + if ($lines === []) { + return false; + } + foreach ($lines as $line) { + // Docker today: "Error response from daemon: network x not found" and + // "Error response from daemon: get x: no such volume". Older Docker: + // "Error: No such network: x" and "Error: No such volume: x". + $missing = $kind === 'network' + ? preg_match('/network .+ not found$/i', $line) === 1 || stripos($line, 'no such network') !== false + : stripos($line, 'no such volume') !== false; + if (!$missing) { + return false; + } + } + return true; + } + private static function failureText(ProcessResult $result): string { return trim($result->stderr) !== '' ? trim($result->stderr) : $result->errorSummary(); diff --git a/source/compose.manager/include/GitPathGuard.php b/source/compose.manager/include/GitPathGuard.php index b4556860..56f56f79 100644 --- a/source/compose.manager/include/GitPathGuard.php +++ b/source/compose.manager/include/GitPathGuard.php @@ -47,43 +47,60 @@ public static function isArrayStarted(): bool * @return string[] */ public static function listMountPoints(): array + { + $mountPoints = []; + foreach (self::listMounts() as $mountPoint => $filesystem) { + if (!in_array($filesystem, self::RAM_FILESYSTEMS, true)) { + $mountPoints[] = $mountPoint; + } + } + return $mountPoints; + } + + /** + * Every mount listed in the mounts file, RAM filesystems included. + * + * @return array mount point => filesystem type + */ + private static function listMounts(): array { $content = @file_get_contents(COMPOSE_MOUNTS_FILE); if ($content === false) { return []; } - $mountPoints = []; + $mounts = []; foreach (explode("\n", $content) as $line) { $fields = explode(' ', trim($line)); if (count($fields) < 3) { continue; } - if (in_array($fields[2], self::RAM_FILESYSTEMS, true)) { - continue; - } // The kernel writes spaces, tabs, newlines and backslashes in mount // points as octal escapes, for example "\040" for a space. - $mountPoints[] = preg_replace_callback( + $mountPoint = preg_replace_callback( '/\\\\([0-7]{3})/', static fn(array $match): string => chr((int) octdec($match[1])), $fields[1] ); + $mounts[$mountPoint] = $fields[2]; } - return $mountPoints; + return $mounts; } /** * The mount under /mnt that holds a path, or null when the path is not on one. * - * Picks the longest listed mount point that contains the path. /mnt itself - * and / never count: on Unraid both are RAM. + * Picks the longest listed mount point that contains the path, RAM mounts + * included, and returns null when that one is in RAM: a tmpfs mounted inside + * a disk or pool keeps its contents in RAM even though the disk is below it. + * /mnt itself and / never count: on Unraid both are RAM. */ public static function findMountFor(string $path): ?string { $mntDir = rtrim(COMPOSE_GIT_MNT_DIR, '/'); $best = null; - foreach (self::listMountPoints() as $mountPoint) { + $bestFilesystem = null; + foreach (self::listMounts() as $mountPoint => $filesystem) { $mountPoint = rtrim($mountPoint, '/'); if (!str_starts_with($mountPoint, $mntDir . '/')) { continue; @@ -93,11 +110,44 @@ public static function findMountFor(string $path): ?string } if ($best === null || strlen($mountPoint) > strlen($best)) { $best = $mountPoint; + $bestFilesystem = $filesystem; } } + if ($bestFilesystem !== null && in_array($bestFilesystem, self::RAM_FILESYSTEMS, true)) { + return null; + } return $best; } + /** + * Whether a path is on a RAM mount that sits inside a disk, pool or share mount, + * such as a tmpfs transcode folder in appdata. Unassigned Devices' tmpfs over + * /mnt/disks is not: nothing real is mounted below it. + */ + public static function isOnRamMountInsideADisk(string $path): bool + { + $mntDir = rtrim(COMPOSE_GIT_MNT_DIR, '/'); + $deepestRam = null; + $diskBelow = false; + foreach (self::listMounts() as $mountPoint => $filesystem) { + $mountPoint = rtrim($mountPoint, '/'); + if (!str_starts_with($mountPoint, $mntDir . '/')) { + continue; + } + if ($path !== $mountPoint && !str_starts_with($path, $mountPoint . '/')) { + continue; + } + if (in_array($filesystem, self::RAM_FILESYSTEMS, true)) { + if ($deepestRam === null || strlen($mountPoint) > strlen($deepestRam)) { + $deepestRam = $mountPoint; + } + } else { + $diskBelow = true; + } + } + return $deepestRam !== null && $diskBelow && self::findMountFor($path) === null; + } + /** * Check the shape of an absolute path without touching the filesystem. * diff --git a/source/compose.manager/include/GitStackManager.php b/source/compose.manager/include/GitStackManager.php index 847ff566..e7ef4d05 100644 --- a/source/compose.manager/include/GitStackManager.php +++ b/source/compose.manager/include/GitStackManager.php @@ -57,6 +57,8 @@ public function add( if (file_exists($stackDir) || @readlink($stackDir) !== false) { throw new RuntimeException("A stack folder named '$folder' already exists. Choose another name, or use convert for that stack."); } + // Before the clone, so a projects folder whose mount is gone leaves nothing behind. + $this->assertSafeToWriteStackFolder($stackDir); $settings = GitStackSettings::createNew($url, $branch, $composePath, $clonesRoot ?? COMPOSE_GIT_DEFAULT_CLONES_ROOT, $folder); ($this->say)("Cloning $url ($branch) into {$settings->cloneDir}..."); @@ -107,7 +109,10 @@ public function convert(string $folder, string $url, string $branch, string $com return $this->withStackLock($stack, function () use ($stack, $stackDir, $folder, $url, $branch, $composePath, $clonesRoot): string { $oldOverride = $stack->getOverridePath(); $oldEnv = $stack->getEffectiveEnvFilePath(); - $hasExplicitEnvPath = trim((string) @file_get_contents($stackDir . '/envpath')) !== ''; + // Whether the .env in use is the one next to the old compose file. That one stops + // being used once the compose file is in the clone. An envpath setting is not enough + // to tell: one that names a missing file is ignored, and that .env is used instead. + $usesEnvNextToComposeFile = $oldEnv !== null && Path::refersToSamePath($oldEnv, $stack->composeSource . '/.env'); $oldComposeFile = $stack->isIndirect ? null : $stack->composeFilePath; $foldersNextToOldComposeFile = $this->foldersIn($stack->composeSource); @@ -152,7 +157,7 @@ public function convert(string $folder, string $url, string $branch, string $com // An indirect stack may have used a .env next to its old compose // file. Keep using it: copy it into the stack folder. $stackEnv = $stackDir . '/.env'; - if (!$hasExplicitEnvPath && $oldEnv !== null && $oldEnv !== $stackEnv && !is_file($stackEnv)) { + if ($usesEnvNextToComposeFile && $oldEnv !== null && $oldEnv !== $stackEnv && !is_file($stackEnv)) { if (!@copy($oldEnv, $stackEnv)) { throw new RuntimeException("Could not copy $oldEnv to $stackEnv."); } diff --git a/source/compose.manager/scripts/common.sh b/source/compose.manager/scripts/common.sh index 2f83b178..f89efd54 100644 --- a/source/compose.manager/scripts/common.sh +++ b/source/compose.manager/scripts/common.sh @@ -135,8 +135,9 @@ foreach ($config["networks"] ?? [] as $key => $network) { # Arguments: the docker compose command that reads the stack's config, for example # create_missing_external_networks docker compose -f compose.yaml -p mystack # A config that does not parse is skipped here; the compose command that follows reports it. +# Returns 1 if any network could not be created (after trying them all, so each is named). create_missing_external_networks() { - local config_json network + local config_json network failed=0 if ! config_json=$("$@" config --format json 2>/dev/null); then return 0 fi @@ -151,8 +152,10 @@ create_missing_external_networks() { else echo "✗ Failed to create external network: $network" composeLogger "Failed to create external network: $network" error compose + failed=1 fi done < <(printf '%s' "$config_json" | external_network_names) + return $failed } # Resolve effective env file for a stack. diff --git a/source/compose.manager/scripts/compose.sh b/source/compose.manager/scripts/compose.sh index 649e4a0c..05090688 100755 --- a/source/compose.manager/scripts/compose.sh +++ b/source/compose.manager/scripts/compose.sh @@ -574,7 +574,11 @@ case $command in # A deploy always builds services that have a build section (the new # commit may change their Dockerfile), as a step of its own before up. - up_args=("-d" "--remove-orphans") + # up itself neither builds nor pulls again, whatever a service's + # pull_policy says: a failure there would come after the point where the + # previous commit can be put back. --pull missing still fetches an image + # the pull step did not. + up_args=("-d" "--remove-orphans" "--no-build" "--pull" "missing") if [ "$wait_for_healthy" = true ]; then up_args+=("--wait") if [ -n "$wait_timeout" ]; then @@ -644,9 +648,18 @@ case $command in # Optionally create missing `external: true` networks, as up and update do # (Settings > Compose > Create Missing External Networks). This is done # here, after every check, pull and build has passed, so a deploy that - # stops earlier creates nothing. + # stops earlier creates nothing. A network that cannot be created would make + # up fail, so stop here, while the previous commit can still be put back. if plugin_setting_enabled CREATE_MISSING_EXTERNAL_NETWORKS; then - create_missing_external_networks "${git_compose[@]}" -p "$name" + if ! create_missing_external_networks "${git_compose[@]}" -p "$name"; then + save_result "failed" 1 "gitdeploy" + log_msg "ERROR" "A missing external network could not be created for git stack $name" + echo "" + echo "✗ Stack $name was not deployed: a missing external network could not be created. No container was changed." + echo "Any network created above is left in place." + put_back_previous_commit + exit 1 + fi fi # 4. Start the stack at the new commit. diff --git a/source/compose.manager/scripts/compose_git.php b/source/compose.manager/scripts/compose_git.php index 7d732395..a0d5adaf 100644 --- a/source/compose.manager/scripts/compose_git.php +++ b/source/compose.manager/scripts/compose_git.php @@ -329,6 +329,43 @@ function compose_git_profile_arguments(StackInfo $stack, array $options): array return $args; } +/** + * The environment compose.sh gets for a git deploy: what DockerCommand gives the deploy's + * checks, plus the documented lock settings. Nothing else of the caller's shell is passed on. + * + * @param array $current The caller's environment + * @return array + */ +function compose_git_deploy_environment(array $current): array +{ + $environment = [ + 'PATH' => '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin', + 'HOME' => $current['HOME'] ?? '/root', + 'LC_ALL' => 'C', + ]; + // TERM only changes how Compose draws its progress. + foreach (['DOCKER_CONFIG', 'COMPOSE_LOCK_TIMEOUT', 'COMPOSE_LOCK_DIR', 'TERM'] as $name) { + if (isset($current[$name]) && $current[$name] !== '') { + $environment[$name] = $current[$name]; + } + } + return $environment; +} + +/** + * The folder compose.sh runs in for a git deploy: the compose file's folder, where the + * deploy's checks run too. + */ +function compose_git_deploy_directory(?string $composeFilePath): string +{ + $composeFile = $composeFilePath === null ? false : realpath($composeFilePath); + if ($composeFile === false) { + // The compose file is checked again by the deploy, which stops before up if it is missing. + return '/'; + } + return dirname($composeFile); +} + /** * Deploy through compose.sh, so the deploy takes the same lock, credentials * and logging as every other stack action. Its output goes straight to ours. @@ -365,7 +402,16 @@ function compose_git_deploy(GitStackManager $manager, string $folder, array $opt $command[] = $arg; } - $process = proc_open($command, [0 => ['file', '/dev/null', 'r'], 1 => STDOUT, 2 => STDERR], $pipes); + // compose.sh runs with the same environment and folder as the deploy's checks, so a + // variable exported in the caller's shell (or the folder it was run from, ${PWD}) + // cannot make up deploy something other than what was checked. + $process = proc_open( + $command, + [0 => ['file', '/dev/null', 'r'], 1 => STDOUT, 2 => STDERR], + $pipes, + compose_git_deploy_directory($stack->composeFilePath), + compose_git_deploy_environment(getenv()) + ); if (!is_resource($process)) { fwrite($errors, "Could not start compose.sh.\n"); return COMPOSE_GIT_EXIT_FAILED; diff --git a/tests/unit/GitCommandTest.php b/tests/unit/GitCommandTest.php index a70af62d..cd36d875 100644 --- a/tests/unit/GitCommandTest.php +++ b/tests/unit/GitCommandTest.php @@ -39,6 +39,30 @@ public function testRunsGitAndCapturesOutput(): void $this->assertStringStartsWith('git version', $result->stdout); } + public function testAGitHomeThatIsASymlinkIsRefused(): void + { + // The home folder is in /var/tmp, where anyone can create things first, and holds the + // global config git reads. (One another user owns is refused too, but a test that does + // not run as root cannot make one.) + $home = rtrim(COMPOSE_GIT_HOME_DIR, '/'); + $target = sys_get_temp_dir() . '/compose_git_home_elsewhere'; + @mkdir($target, 0755); + $moved = $home . '.real'; + @rename($home, $moved); + symlink($target, $home); + try { + $result = GitCommand::run(['--version']); + } finally { + unlink($home); + @rename($moved, $home); + } + + $this->assertFalse($result->succeeded()); + $this->assertStringContainsString('is not a plain folder', $result->errorSummary()); + $this->assertSame(0755, fileperms($target) & 0777); + rmdir($target); + } + public function testFailureReportsTheLastErrorLine(): void { $result = GitCommand::run(['rev-parse', '--verify', 'no-such-ref'], $this->makeRepo()); diff --git a/tests/unit/GitDeployCheckTest.php b/tests/unit/GitDeployCheckTest.php index e5f8478d..0678eb4e 100644 --- a/tests/unit/GitDeployCheckTest.php +++ b/tests/unit/GitDeployCheckTest.php @@ -123,14 +123,45 @@ public function testMissingExternalNetworkAndVolumeAreProblems(): void $problems = $this->check(); $this->assertCount(2, $problems); - $this->assertStringContainsString("network 'zz-proxy'", $problems[0]); - $this->assertStringContainsString("volume 'zz-ext'", $problems[1]); + $this->assertStringContainsString("The external network 'zz-proxy' does not exist", $problems[0]); + $this->assertStringContainsString("The external volume 'zz-ext' does not exist", $problems[1]); $this->docker->addNetwork('zz-proxy'); $this->docker->addVolume('zz-ext'); $this->assertSame([], $this->check()); } + public function testExternalNetworkAndVolumeThatDockerCannotBeAskedAboutAreNotCalledMissing(): void + { + $config = $this->config(['image' => 'busybox']); + $config['networks'] = ['proxy' => ['name' => 'zz-proxy', 'external' => true]]; + $config['volumes'] = ['ext' => ['name' => 'zz-ext', 'external' => true]]; + $this->docker->setConfig($config); + $daemonDown = 'Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?'; + $this->docker->fail('network', $daemonDown); + $this->docker->fail('volume', $daemonDown); + + $problems = $this->check(); + $this->assertCount(2, $problems); + $this->assertStringContainsString("Could not check whether the external network 'zz-proxy' exists: Cannot connect", $problems[0]); + $this->assertStringContainsString("Could not check whether the external volume 'zz-ext' exists: Cannot connect", $problems[1]); + } + + public function testOlderDockersWordingForAMissingNetworkOrVolumeCountsAsMissing(): void + { + $config = $this->config(['image' => 'busybox']); + $config['networks'] = ['proxy' => ['name' => 'zz-proxy', 'external' => true]]; + $config['volumes'] = ['ext' => ['name' => 'zz-ext', 'external' => true]]; + $this->docker->setConfig($config); + $this->docker->fail('network', 'Error: No such network: zz-proxy'); + $this->docker->fail('volume', 'Error: No such volume: zz-ext'); + + $problems = $this->check(); + $this->assertCount(2, $problems); + $this->assertStringContainsString("The external network 'zz-proxy' does not exist", $problems[0]); + $this->assertStringContainsString("The external volume 'zz-ext' does not exist", $problems[1]); + } + public function testMissingExternalNetworkIsNotAProblemWhenItWillBeCreated(): void { $config = $this->config(['image' => 'busybox']); @@ -228,6 +259,27 @@ public function testBindSourceOnAMissingMountIsCalledOutAsRam(): void $this->assertStringContainsString('RAM', $this->check()[0]); } + public function testBindSourceOnARamMountInsideAPoolIsTheUsersChoice(): void + { + // A tmpfs in appdata, such as a RAM transcode folder, is meant to be in RAM. A missing + // folder in it (empty after every reboot) is left for Docker to create. + mkdir($this->mnt . '/cache/appdata/ram/transcode', 0755, true); + file_put_contents( + COMPOSE_MOUNTS_FILE, + "rootfs {$this->mnt} rootfs rw 0 0\nshfs {$this->mnt}/user fuse.shfs rw 0 0\n" + . "/dev/nvme0n1p1 {$this->mnt}/cache xfs rw 0 0\ntmpfs {$this->mnt}/cache/appdata/ram tmpfs rw 0 0\n" + ); + $missing = $this->mnt . '/cache/appdata/ram/missing'; + $this->docker->setConfig($this->config(['image' => 'busybox', 'volumes' => [ + ['type' => 'bind', 'source' => $this->mnt . '/cache/appdata/ram/transcode', 'target' => '/transcode'], + ['type' => 'bind', 'source' => $missing, 'target' => '/cache'], + ]])); + $check = new GitDeployCheck('whoami', ['-f', $this->clone . '/whoami/compose.yaml'], $this->clone, $this->clone . '/whoami', null); + + $this->assertSame([], $check->run()); + $this->assertSame([$missing], $check->foldersDockerWillCreate()); + } + public function testExistingBindSourceAndMissingSourceInsideTheCloneAreFine(): void { $this->docker->setConfig($this->config(['image' => 'busybox', 'volumes' => [ diff --git a/tests/unit/GitDeployTest.php b/tests/unit/GitDeployTest.php index 2f76a976..a07b6a36 100644 --- a/tests/unit/GitDeployTest.php +++ b/tests/unit/GitDeployTest.php @@ -331,6 +331,39 @@ public function testRedeployOfTheSameCommitRecreatesNothing(): void $this->assertSame([], $this->deploy()->upArguments($previous)); } + public function testDiscardedLocalChangesInTheStackFolderRecreateEveryContainer(): void + { + // A config file edited in the clone, then discarded at the same commit: no commit + // differs, but a container with a single-file mount of it still holds the edit. + $this->writeAndPush(['whoami/app.conf' => "from-repo\n"], 'config'); + $previous = $this->deploy()->prepare('whoami', [], null, false); + $this->deploy()->finish(true); + file_put_contents($this->clone->settings()->cloneDir . '/whoami/app.conf', "edited-locally\n"); + + $previous = $this->deploy()->prepare('whoami', [], null, true); + $this->assertSame(['--force-recreate'], $this->deploy()->upArguments($previous)); + + // Until a deploy succeeds, every deploy recreates: the containers may still hold the edit. + $this->deploy()->finish(false); + $this->assertSame(['--force-recreate'], $this->deploy()->upArguments($previous)); + $this->deploy()->finish(true); + $this->assertFileDoesNotExist($this->stackDir . '/' . GitDeploy::DISCARDED_CHANGES_FILE); + $previous = $this->deploy()->prepare('whoami', [], null, false); + $this->assertSame([], $this->deploy()->upArguments($previous)); + } + + public function testDiscardedLocalChangesOutsideTheStackFolderRecreateNothing(): void + { + $this->writeAndPush(['other/readme.txt' => "from-repo\n"], 'another folder'); + $previous = $this->deploy()->prepare('whoami', [], null, false); + $this->deploy()->finish(true); + file_put_contents($this->clone->settings()->cloneDir . '/other/readme.txt', "edited-locally\n"); + + $previous = $this->deploy()->prepare('whoami', [], null, true); + + $this->assertSame([], $this->deploy()->upArguments($previous)); + } + public function testWithTheSettingOffNothingIsRecreated(): void { GitStackSettings::load($this->stackDir)->withRecreateOnFolderChange(false)->save($this->stackDir); diff --git a/tests/unit/GitPathGuardTest.php b/tests/unit/GitPathGuardTest.php index 5cdd6f93..4fde95e9 100644 --- a/tests/unit/GitPathGuardTest.php +++ b/tests/unit/GitPathGuardTest.php @@ -100,6 +100,32 @@ public function testMountNameThatOnlySharesAPrefixDoesNotMatch(): void $this->assertNull(GitPathGuard::findMountFor($this->mnt . '/user0/appdata')); } + public function testRamMountInsideADiskMountIsRefused(): void + { + // A tmpfs mounted inside a pool (a RAM transcode folder, say) keeps its contents + // in RAM, even though the pool is mounted below it. + mkdir($this->mnt . '/cache/appdata/ram/git', 0755, true); + file_put_contents( + COMPOSE_MOUNTS_FILE, + "rootfs / rootfs rw 0 0\n" + . "rootfs {$this->mnt} rootfs rw 0 0\n" + . "/dev/nvme0n1p1 {$this->mnt}/cache xfs rw 0 0\n" + . "tmpfs {$this->mnt}/cache/appdata/ram tmpfs rw 0 0\n" + ); + + $this->assertNull(GitPathGuard::findMountFor($this->mnt . '/cache/appdata/ram/git')); + $this->assertSame($this->mnt . '/cache', GitPathGuard::findMountFor($this->mnt . '/cache/appdata/git')); + + try { + GitPathGuard::assertSafeToWrite($this->mnt . '/cache/appdata/ram/git'); + $this->fail('A folder on a tmpfs inside a pool was accepted for writing.'); + } catch (RuntimeException $error) { + $this->assertStringContainsString('not on a mounted disk', $error->getMessage()); + } + $this->expectException(InvalidArgumentException::class); + GitPathGuard::assertValidClonesRoot($this->mnt . '/cache/appdata/ram/git'); + } + // ----- clones root ----- public function testUsualClonesRootsAreAccepted(): void diff --git a/tests/unit/GitStackManagerTest.php b/tests/unit/GitStackManagerTest.php index ec913a65..2eab84b3 100644 --- a/tests/unit/GitStackManagerTest.php +++ b/tests/unit/GitStackManagerTest.php @@ -106,29 +106,30 @@ public function testAddWithAComposePathNotOnTheBranchLeavesNothingBehind(): void $this->assertDirectoryDoesNotExist($this->composeRoot . '/whoami'); } - public function testAddThatFailsAfterMakingTheStackFolderSaysToRemoveIt(): void + public function testAddDoesNothingWhileTheArrayIsStopped(): void + { + file_put_contents(COMPOSE_UNRAID_VAR_INI, "mdState=\"STOPPED\"\n"); + $this->expectException(RuntimeException::class); + $this->manager->add('whoami', $this->upstream, 'main', 'whoami/compose.yaml', $this->clonesRoot); + } + + public function testAddToAProjectsFolderWhoseMountIsGoneWritesNothing(): void { - // A projects folder on the array's root file system (RAM): the stack - // folder is made, then saving its git settings there is refused. + // A projects folder that is not on a mount (a pool that did not mount, while the + // folder is still there, in RAM): neither the clone nor the stack folder may be made. + // This used to make the stack folder first and fail only when saving its settings. $composeRoot = $this->mnt . '/projects'; mkdir($composeRoot); $manager = new GitStackManager($composeRoot); try { $manager->add('whoami', $this->upstream, 'main', 'whoami/compose.yaml', $this->clonesRoot); - $this->fail('Added a stack whose settings could not be saved'); + $this->fail('The stack was added to a projects folder that is not on a mounted disk.'); } catch (RuntimeException $error) { - $this->assertStringContainsString("the stack folder $composeRoot/whoami was only partly made", $error->getMessage()); - $this->assertStringContainsString('Remove both folders', $error->getMessage()); + $this->assertStringContainsString('not on a mounted disk', $error->getMessage()); } - $this->assertDirectoryExists($composeRoot . '/whoami'); - } - - public function testAddDoesNothingWhileTheArrayIsStopped(): void - { - file_put_contents(COMPOSE_UNRAID_VAR_INI, "mdState=\"STOPPED\"\n"); - $this->expectException(RuntimeException::class); - $this->manager->add('whoami', $this->upstream, 'main', 'whoami/compose.yaml', $this->clonesRoot); + $this->assertDirectoryDoesNotExist($composeRoot . '/whoami'); + $this->assertSame([], glob($this->clonesRoot . '/*') ?: []); } // ----- convert ----- @@ -194,6 +195,25 @@ public function testConvertOfAStackWithACustomEnvFileCopiesNoEnv(): void $this->assertSame($customEnv, \StackInfo::fromProject($this->composeRoot, $stack->projectFolder)->getEffectiveEnvFilePath()); } + public function testConvertWithAnEnvPathToAMissingFileCopiesTheEnvInUse(): void + { + // An env file setting that names a file which is gone is ignored, and the .env next + // to the old compose file is used. That is the one to keep using after the convert. + $oldFolder = $this->mnt . '/user/appdata/legacy'; + mkdir($oldFolder); + file_put_contents($oldFolder . '/compose.yaml', "services:\n whoami:\n image: traefik/whoami\n"); + file_put_contents($oldFolder . '/.env', "NEXT_TO_COMPOSE=1\n"); + $stack = \StackInfo::createNew($this->composeRoot, 'legacy', '', $oldFolder); + $stackDir = $this->composeRoot . '/' . $stack->projectFolder; + file_put_contents($stackDir . '/envpath', $this->mnt . '/user/appdata/moved-away.env'); + + $this->manager->convert($stack->projectFolder, $this->upstream, 'main', 'whoami/compose.yaml', $this->clonesRoot); + + $this->assertSame("NEXT_TO_COMPOSE=1\n", file_get_contents($stackDir . '/.env')); + \StackInfo::clearCache(); + $this->assertSame($stackDir . '/.env', \StackInfo::fromProject($this->composeRoot, $stack->projectFolder)->getEffectiveEnvFilePath()); + } + public function testConvertOfAnIndirectStackCarriesItsOverrideIntoTheStackFolder(): void { $oldFolder = $this->mnt . '/user/appdata/legacy'; @@ -464,6 +484,45 @@ public function testDeployWaitsAsTheStacksSettingSaysUnlessTheCommandLineOverrid $this->assertSame(['--wait'], compose_git_wait_arguments(['enabled' => true, 'timeout' => '5m'], [])); } + public function testDeployRunsComposeWithTheSameEnvironmentAsTheChecks(): void + { + // Exported variables win over the stack's .env in compose, and PWD is the caller's + // folder: neither may reach the deploy, since the checks never saw them. + $callersShell = [ + 'PATH' => '/root/bin:/usr/bin', + 'HOME' => '/root', + 'PWD' => '/root/somewhere', + 'PORT' => '8080', + 'DOCKER_HOST' => 'tcp://elsewhere:2375', + 'COMPOSE_PROFILES' => 'debug', + 'COMPOSE_LOCK_TIMEOUT' => '5', + 'DOCKER_CONFIG' => '/tmp/registry-login', + ]; + + $this->assertSame([ + 'PATH' => '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin', + 'HOME' => '/root', + 'LC_ALL' => 'C', + 'DOCKER_CONFIG' => '/tmp/registry-login', + 'COMPOSE_LOCK_TIMEOUT' => '5', + ], compose_git_deploy_environment($callersShell)); + } + + public function testDeployRunsComposeInTheComposeFilesFolder(): void + { + $folder = $this->mnt . '/user/appdata/git/whoami/whoami'; + mkdir($folder, 0755, true); + file_put_contents($folder . '/compose.yaml', "services: {}\n"); + // A compose file that is a symlink: the folder of the file it leads to, as the checks use. + mkdir($this->mnt . '/user/appdata/git/whoami/linked', 0755, true); + symlink($folder . '/compose.yaml', $this->mnt . '/user/appdata/git/whoami/linked/compose.yaml'); + + $this->assertSame(realpath($folder), compose_git_deploy_directory($folder . '/compose.yaml')); + $this->assertSame(realpath($folder), compose_git_deploy_directory($this->mnt . '/user/appdata/git/whoami/linked/compose.yaml')); + $this->assertSame('/', compose_git_deploy_directory($folder . '/missing.yaml')); + $this->assertSame('/', compose_git_deploy_directory(null)); + } + public function testDeployRefusesWaitAndNoWaitTogether(): void { $this->expectException(\ComposeGitUsageError::class); diff --git a/tests/unit/common.bats b/tests/unit/common.bats index 32559fa5..6cf31f6f 100644 --- a/tests/unit/common.bats +++ b/tests/unit/common.bats @@ -228,6 +228,26 @@ NETWORKS_CONFIG_JSON='{"networks":{"internalnet":{"name":"stack_internalnet"},"s [[ "$(cat "$TEST_TEMP_DIR/created")" == "svc-real-name" ]] } +@test "create_missing_external_networks tries every network and fails when one cannot be created" { + # shellcheck disable=SC1090 + source "$COMMON_SCRIPT" + # Neither network exists; creating "svc-real-name" fails, creating "web" works. + docker() { + case "$*" in + "compose -p teststack config --format json") echo "$NETWORKS_CONFIG_JSON" ;; + "network inspect "*) return 1 ;; + "network create svc-real-name") return 1 ;; + "network create "*) echo "$3" >> "$TEST_TEMP_DIR/created" ;; + esac + } + + run create_missing_external_networks docker compose -p teststack + + assert_failure + assert_output_contains "Failed to create external network: svc-real-name" + [[ "$(cat "$TEST_TEMP_DIR/created")" == "web" ]] +} + @test "create_missing_external_networks does nothing when the compose config does not parse" { # shellcheck disable=SC1090 source "$COMMON_SCRIPT" diff --git a/tests/unit/fixtures/fake-docker.sh b/tests/unit/fixtures/fake-docker.sh index 278218bc..9775a782 100644 --- a/tests/unit/fixtures/fake-docker.sh +++ b/tests/unit/fixtures/fake-docker.sh @@ -56,7 +56,12 @@ case "$1" in cat "$here/$kind/$name" exit 0 fi - echo "Error: No such $1: $name" >&2 + # What Docker prints for a network or volume that does not exist. + if [ "$1" = network ]; then + echo "Error response from daemon: network $name not found" >&2 + else + echo "Error response from daemon: get $name: no such volume" >&2 + fi exit 1 ;; ps) diff --git a/tests/unit/gitdeploy.bats b/tests/unit/gitdeploy.bats index a1c5b8a5..c292162a 100644 --- a/tests/unit/gitdeploy.bats +++ b/tests/unit/gitdeploy.bats @@ -26,8 +26,9 @@ test_setup() { export PREPARE_OUTPUT="0000000000000000000000000000000000000001" # The plugin's settings: none, so Create Missing External Networks is off unless a test turns it on. export COMPOSE_MANAGER_CFG_FILE="$TEST_TEMP_DIR/compose.manager.cfg" - # What "docker compose config --format json" prints, and whether "docker network inspect" finds the network. - export CONFIG_JSON='{}' NETWORK_INSPECT_EXIT=0 + # What "docker compose config --format json" prints, whether "docker network inspect" finds the network, + # and whether "docker network create" works. + export CONFIG_JSON='{}' NETWORK_INSPECT_EXIT=0 NETWORK_CREATE_EXIT=0 STACK="$TEST_TEMP_DIR/whoami" mkdir -p "$STACK" "$TEST_TEMP_DIR/bin" @@ -84,6 +85,7 @@ for arg in "$@"; do up) exit "$UP_EXIT" ;; config) printf '%s' "$CONFIG_JSON"; exit 0 ;; inspect) exit "$NETWORK_INSPECT_EXIT" ;; + create) exit "$NETWORK_CREATE_EXIT" ;; esac done exit 0 @@ -216,7 +218,7 @@ calls_matching() { [ "$build_line" -lt "$up_line" ] [ "$up_line" -lt "$finish_line" ] - grep -q "docker compose -f /clone/whoami/compose.yaml --env-file /stack/.env -p whoami up -d --remove-orphans$" "$CALLS" + grep -q "docker compose -f /clone/whoami/compose.yaml --env-file /stack/.env -p whoami up -d --remove-orphans --no-build --pull missing$" "$CALLS" grep -q "git_stack finish $STACK success" "$CALLS" grep -q "docker rmi img-old$" "$CALLS" grep -q '"operation":"gitdeploy"' "$STACK/last_result.json" @@ -232,7 +234,7 @@ calls_matching() { @test "gitdeploy waits for healthy containers when asked" { run_gitdeploy --wait --wait-timeout 90 [ "$status" -eq 0 ] - grep -q -- "up -d --remove-orphans --wait --wait-timeout 90" "$CALLS" + grep -q -- "up -d --remove-orphans --no-build --pull missing --wait --wait-timeout 90" "$CALLS" } @test "gitdeploy recreates every container when the stack's folder changed" { @@ -240,7 +242,7 @@ calls_matching() { run_gitdeploy --wait [ "$status" -eq 0 ] grep -q "git_stack up-arguments $STACK 0000000000000000000000000000000000000001" "$CALLS" - grep -q -- "up -d --remove-orphans --wait --force-recreate$" "$CALLS" + grep -q -- "up -d --remove-orphans --no-build --pull missing --wait --force-recreate$" "$CALLS" } @test "gitdeploy puts the previous commit back when it cannot tell whether to recreate, and never runs up" { @@ -260,10 +262,13 @@ calls_matching() { [ "$(calls_matching ' up ')" -eq 0 ] } -@test "gitdeploy never rebuilds during up, so a build failure cannot happen part-way" { +@test "gitdeploy tells up never to build and to pull only missing images" { + # A service's pull_policy (build or always) would otherwise build or pull again during up, + # after the point where the previous commit can be put back. run_gitdeploy --build [ "$status" -eq 0 ] [ "$(grep -c -- ' up .*--build' "$CALLS" || true)" -eq 0 ] + grep -q -- ' up .*--no-build --pull missing' "$CALLS" } @test "gitdeploy creates a missing external network after the build and before up when the setting is on" { @@ -280,6 +285,18 @@ calls_matching() { [ "$create_line" -lt "$up_line" ] } +@test "gitdeploy puts the previous commit back when a missing network cannot be created, and never runs up" { + echo 'CREATE_MISSING_EXTERNAL_NETWORKS="true"' > "$COMPOSE_MANAGER_CFG_FILE" + export CONFIG_JSON='{"networks":{"proxy":{"name":"zz-proxy","external":true}}}' NETWORK_INSPECT_EXIT=1 + export NETWORK_CREATE_EXIT=1 + run_gitdeploy + [ "$status" -eq 1 ] + [[ "$output" == *"a missing external network could not be created"* ]] + grep -q "git_stack restore $STACK 0000000000000000000000000000000000000001" "$CALLS" + [ "$(calls_matching ' up ')" -eq 0 ] + [ "$(calls_matching 'finish')" -eq 0 ] +} + @test "gitdeploy creates no network when the setting is off" { export CONFIG_JSON='{"networks":{"proxy":{"name":"zz-proxy","external":true}}}' NETWORK_INSPECT_EXIT=1 run_gitdeploy