From f756fe88d3c95e3ddce08fb7d74fc34b5fac65dc Mon Sep 17 00:00:00 2001 From: Ali Date: Sat, 18 Apr 2026 11:11:17 +0200 Subject: [PATCH] feat(IS-667): Add automatic cleanup of outdated ExApp Docker images Signed-off-by: Ali --- appinfo/info.xml | 1 + lib/AppInfo/Application.php | 2 + lib/BackgroundJob/DockerImageCleanupJob.php | 96 +++++++ lib/Command/ExApp/Update.php | 36 +++ lib/DeployActions/DockerActions.php | 54 +++- lib/Settings/Admin.php | 2 + specs/docker-image-cleanup/plan.md | 65 +++++ specs/docker-image-cleanup/spec.md | 118 +++++++++ src/components/AdminSettings.vue | 25 ++ .../DockerImageCleanupJobTest.php | 230 +++++++++++++++++ .../Command/ExApp/UpdateImageCleanupTest.php | 175 +++++++++++++ .../DockerActionsImageCleanupTest.php | 236 ++++++++++++++++++ 12 files changed, 1039 insertions(+), 1 deletion(-) create mode 100644 lib/BackgroundJob/DockerImageCleanupJob.php create mode 100644 specs/docker-image-cleanup/plan.md create mode 100644 specs/docker-image-cleanup/spec.md create mode 100644 tests/php/BackgroundJob/DockerImageCleanupJobTest.php create mode 100644 tests/php/Command/ExApp/UpdateImageCleanupTest.php create mode 100644 tests/php/DeployActions/DockerActionsImageCleanupTest.php diff --git a/appinfo/info.xml b/appinfo/info.xml index f1fa1fff..9ed46b11 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -62,6 +62,7 @@ See the [admin documentation](https://docs.nextcloud.com/server/latest/admin_man OCA\AppAPI\BackgroundJob\ExAppInitStatusCheckJob + OCA\AppAPI\BackgroundJob\DockerImageCleanupJob diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index a47c1be0..20d790a9 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -44,6 +44,8 @@ class Application extends App implements IBootstrap { public const TEST_DEPLOY_APPID = 'test-deploy'; public const TEST_DEPLOY_INFO_XML = 'https://raw.githubusercontent.com/nextcloud/test-deploy/main/appinfo/info.xml'; public const MINIMUM_HARP_VERSION = '0.3.0'; + public const CONF_IMAGE_CLEANUP_INTERVAL_DAYS = 'image_cleanup_interval_days'; + public const CONF_IMAGE_CLEANUP_ON_UPDATE = 'image_cleanup_on_update'; public function __construct(array $urlParams = []) { parent::__construct(self::APP_ID, $urlParams); diff --git a/lib/BackgroundJob/DockerImageCleanupJob.php b/lib/BackgroundJob/DockerImageCleanupJob.php new file mode 100644 index 00000000..1cfded6f --- /dev/null +++ b/lib/BackgroundJob/DockerImageCleanupJob.php @@ -0,0 +1,96 @@ +appConfig->getValueInt( + Application::APP_ID, Application::CONF_IMAGE_CLEANUP_INTERVAL_DAYS, 7, lazy: true + ); + $this->setInterval(max($intervalDays, 1) * self::SECONDS_PER_DAY); + } + + protected function run($argument): void { + if ($this->isCleanupDisabled()) { + return; + } + + $this->pruneImagesOnAllDaemons(); + } + + private function isCleanupDisabled(): bool { + $intervalDays = $this->appConfig->getValueInt( + Application::APP_ID, Application::CONF_IMAGE_CLEANUP_INTERVAL_DAYS, 7, lazy: true + ); + return $intervalDays === 0; + } + + private function pruneImagesOnAllDaemons(): void { + $daemons = $this->daemonConfigService->getRegisteredDaemonConfigs(); + foreach ($daemons as $daemon) { + if (!$this->supportsPrune($daemon)) { + continue; + } + + try { + $this->dockerActions->initGuzzleClient($daemon); + $dockerUrl = $this->dockerActions->buildDockerUrl($daemon); + $result = $this->dockerActions->pruneImages($dockerUrl, ['dangling' => ['true']]); + if (isset($result['error'])) { + $this->logger->error(sprintf( + 'Image prune failed for daemon "%s": %s', + $daemon->getName(), $result['error'] + )); + } + } catch (\Exception $e) { + $this->logger->error(sprintf( + 'Exception during image prune for daemon "%s": %s', + $daemon->getName(), $e->getMessage() + )); + } + } + } + + private function supportsPrune(DaemonConfig $daemon): bool { + if ($daemon->getAcceptsDeployId() !== DockerActions::DEPLOY_ID) { + return false; + } + + // HaRP daemons are Docker-type but route through a proxy that doesn't + // support the /images/prune endpoint yet. Skip until upstream HaRP adds it. + if (!empty($daemon->getDeployConfig()['harp'])) { + $this->logger->debug(sprintf( + 'Skipping image prune for HaRP daemon "%s" (not yet supported)', + $daemon->getName() + )); + return false; + } + + return true; + } +} diff --git a/lib/Command/ExApp/Update.php b/lib/Command/ExApp/Update.php index b73e33c5..3a7390af 100644 --- a/lib/Command/ExApp/Update.php +++ b/lib/Command/ExApp/Update.php @@ -9,6 +9,8 @@ namespace OCA\AppAPI\Command\ExApp; +use OCA\AppAPI\AppInfo\Application; +use OCA\AppAPI\Db\DaemonConfig; use OCA\AppAPI\DeployActions\DockerActions; use OCA\AppAPI\DeployActions\KubernetesActions; use OCA\AppAPI\DeployActions\ManualActions; @@ -19,6 +21,7 @@ use OCA\AppAPI\Service\ExAppDeployOptionsService; use OCA\AppAPI\Service\ExAppService; +use OCP\IAppConfig; use Psr\Log\LoggerInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; @@ -39,6 +42,7 @@ public function __construct( private readonly ExAppArchiveFetcher $exAppArchiveFetcher, private readonly ExAppFetcher $exAppFetcher, private readonly ExAppDeployOptionsService $exAppDeployOptionsService, + private readonly IAppConfig $appConfig, ) { parent::__construct(); } @@ -208,7 +212,13 @@ private function updateExApp(InputInterface $input, OutputInterface $output, str $auth = []; $harpK8sUrl = null; $k8sRoles = []; + $oldImageName = ''; if ($daemonConfig->getAcceptsDeployId() === $this->dockerActions->getAcceptsDeployId()) { + // Capture old image name before deploying the new version + $oldAppInfo = array_merge($appInfo, ['version' => $exApp->getVersion()]); + $oldDeployParams = $this->dockerActions->buildDeployParams($daemonConfig, $oldAppInfo); + $oldImageName = $this->dockerActions->buildBaseImageName($oldDeployParams['image_params'], $daemonConfig); + $deployParams = $this->dockerActions->buildDeployParams($daemonConfig, $appInfo); if (boolval($exApp->getDeployConfig()['harp'] ?? false)) { $deployResult = $this->dockerActions->deployExAppHarp($exApp, $daemonConfig, $deployParams); @@ -314,6 +324,8 @@ private function updateExApp(InputInterface $input, OutputInterface $output, str $output->writeln(sprintf('ExApp %s update successfully deployed.', $appId)); } + $this->removeOldImageIfEnabled($oldImageName, $daemonConfig, $appId); + $this->service->dispatchExAppInitInternal($exApp); if ($input->getOption('wait-finish')) { $error = $this->exAppService->waitInitStepFinish($appId); @@ -342,4 +354,28 @@ private function updateExApp(InputInterface $input, OutputInterface $output, str return 0; } + private function removeOldImageIfEnabled(string $oldImageName, DaemonConfig $daemonConfig, string $appId): void { + if (empty($oldImageName)) { + return; + } + // Image cleanup only applies to Docker daemons + if ($daemonConfig->getAcceptsDeployId() !== $this->dockerActions->getAcceptsDeployId()) { + return; + } + + $isEnabled = $this->appConfig->getValueBool( + Application::APP_ID, Application::CONF_IMAGE_CLEANUP_ON_UPDATE, false, lazy: true + ); + if (!$isEnabled) { + return; + } + + $dockerUrl = $this->dockerActions->buildDockerUrl($daemonConfig); + $result = $this->dockerActions->removeImage($dockerUrl, $oldImageName); + if ($result) { + $this->logger->warning(sprintf('Old image cleanup for %s: %s', $appId, $result)); + } else { + $this->logger->info(sprintf('Old image %s removed after updating %s', $oldImageName, $appId)); + } + } } diff --git a/lib/DeployActions/DockerActions.php b/lib/DeployActions/DockerActions.php index 9c372430..f8dfc43c 100644 --- a/lib/DeployActions/DockerActions.php +++ b/lib/DeployActions/DockerActions.php @@ -14,13 +14,14 @@ use GuzzleHttp\Exception\GuzzleException; use OCA\AppAPI\AppInfo\Application; use OCA\AppAPI\Db\DaemonConfig; - use OCA\AppAPI\Db\ExApp; + use OCA\AppAPI\Service\AppAPICommonService; use OCA\AppAPI\Service\ExAppDeployOptionsService; use OCA\AppAPI\Service\ExAppService; use OCA\AppAPI\Service\HarpService; use OCP\App\IAppManager; +use OCP\AppFramework\Http; use OCP\IAppConfig; use OCP\ICertificateManager; @@ -28,6 +29,7 @@ use OCP\ITempManager; use OCP\IURLGenerator; use OCP\Security\ICrypto; +use OCP\Util; use Phar; use PharData; use Psr\Log\LoggerInterface; @@ -488,6 +490,56 @@ public function imageExists(string $dockerUrl, string $imageId): bool { } } + public function removeImage(string $dockerUrl, string $imageId): string { + $url = $this->buildApiUrl($dockerUrl, sprintf('images/%s', $imageId)); + try { + $response = $this->guzzleClient->delete($url); + if ($response->getStatusCode() === Http::STATUS_OK) { + $this->logger->info(sprintf('Successfully removed Docker image: %s', $imageId)); + return ''; + } + return sprintf('Unexpected status %d removing image %s', $response->getStatusCode(), $imageId); + } catch (GuzzleException $e) { + if ($e->getCode() === Http::STATUS_NOT_FOUND) { + $this->logger->debug(sprintf('Image %s not found (already removed)', $imageId)); + return ''; + } + if ($e->getCode() === Http::STATUS_CONFLICT) { + $this->logger->warning(sprintf('Image %s is in use, skipping removal', $imageId)); + return ''; + } + $this->logger->error(sprintf('Failed to remove image %s: %s', $imageId, $e->getMessage())); + return sprintf('Failed to remove image %s: %s', $imageId, $e->getMessage()); + } + } + + /** + * @throws \JsonException + */ + public function pruneImages(string $dockerUrl, array $filters = []): array { + $url = $this->buildApiUrl($dockerUrl, 'images/prune'); + $queryParams = []; + if (!empty($filters)) { + $queryParams['filters'] = json_encode($filters, JSON_THROW_ON_ERROR); + } + try { + $response = $this->guzzleClient->post($url, ['query' => $queryParams]); + $result = json_decode((string)$response->getBody(), true, 512, JSON_THROW_ON_ERROR); + $spaceReclaimed = $result['SpaceReclaimed'] ?? 0; + $imagesDeleted = $result['ImagesDeleted'] ?? []; + $count = is_array($imagesDeleted) ? count($imagesDeleted) : 0; + $this->logger->info(sprintf( + 'Docker image prune completed: %d images removed, %s reclaimed', + $count, + Util::humanFileSize($spaceReclaimed) + )); + return $result ?? []; + } catch (GuzzleException $e) { + $this->logger->error(sprintf('Failed to prune Docker images: %s', $e->getMessage())); + return ['error' => $e->getMessage()]; + } + } + public function createContainer(string $dockerUrl, string $imageId, DaemonConfig $daemonConfig, array $params = []): array { $createVolumeResult = $this->createVolume($dockerUrl, $this->buildExAppVolumeName($params['name'])); if (isset($createVolumeResult['error'])) { diff --git a/lib/Settings/Admin.php b/lib/Settings/Admin.php index 7d4356d0..7f5f3198 100644 --- a/lib/Settings/Admin.php +++ b/lib/Settings/Admin.php @@ -36,6 +36,8 @@ public function getForm(): TemplateResponse { 'default_daemon_config' => $this->appConfig->getValueString(Application::APP_ID, 'default_daemon_config', lazy: true), 'init_timeout' => $this->appConfig->getValueString(Application::APP_ID, 'init_timeout', '40'), 'container_restart_policy' => $this->appConfig->getValueString(Application::APP_ID, 'container_restart_policy', 'unless-stopped'), + 'image_cleanup_interval_days' => $this->appConfig->getValueString(Application::APP_ID, Application::CONF_IMAGE_CLEANUP_INTERVAL_DAYS, '7'), + 'image_cleanup_on_update' => $this->appConfig->getValueString(Application::APP_ID, Application::CONF_IMAGE_CLEANUP_ON_UPDATE, '0'), ]; $defaultDaemonConfigName = $this->appConfig->getValueString(Application::APP_ID, 'default_daemon_config', lazy: true); diff --git a/specs/docker-image-cleanup/plan.md b/specs/docker-image-cleanup/plan.md new file mode 100644 index 00000000..e0003d11 --- /dev/null +++ b/specs/docker-image-cleanup/plan.md @@ -0,0 +1,65 @@ +# Plan: Automatic Clean-up of Outdated ExApp Docker Images + +**Spec:** [spec-docker-image-cleanup.md](./spec-docker-image-cleanup.md) +**Issue:** [nextcloud/app_api#667](https://github.com/nextcloud/app_api/issues/667) + +--- + +## Phases + +### Phase 1: Docker API Methods — DONE + +| Task | File | What | +|------|------|------| +| `removeImage()` | `DockerActions.php` | `DELETE /images/{id}`, uses `Http::STATUS_*` constants | +| `pruneImages()` | `DockerActions.php` | `POST /images/prune`, uses `Util::humanFileSize()` | +| Config constants | `Application.php` | `CONF_IMAGE_CLEANUP_INTERVAL_DAYS`, `CONF_IMAGE_CLEANUP_ON_UPDATE` | + +### Phase 2: Background Job — DONE + +| Task | File | What | +|------|------|------| +| `DockerImageCleanupJob` | New file | Dynamic `setInterval()` from config, `isCleanupDisabled()`, `pruneImagesOnAllDaemons()`, `supportsPrune()` | +| Register job | `info.xml` | Added to `` | + +### Phase 3: Cleanup on Update — DONE + +| Task | File | What | +|------|------|------| +| Old image removal | `Update.php` | Added `IAppConfig` dep, captures old image before deploy, `removeOldImageIfEnabled()` after success | + +### Phase 4: Admin Settings — DONE + +| Task | File | What | +|------|------|------| +| Initial state | `Admin.php` | Added both settings to `$adminInitialData` | +| UI controls | `AdminSettings.vue` | Interval input + checkbox, uses existing `onInput()`/`onCheckboxChanged()` | + +--- + +## Verification + +- [x] `removeImage()` handles 200, 404, 409 correctly +- [x] `pruneImages()` sends correct filters, logs space reclaimed +- [x] Background job disabled when interval = 0 +- [x] Background job skips non-Docker and HaRP daemons +- [x] Background job errors don't block other daemons +- [x] Update command only removes old image when checkbox enabled +- [x] Update cleanup failure doesn't block the update +- [x] Admin UI displays and saves both settings +- [x] Config keys centralized as constants +- [x] Registered in `info.xml` +- [x] Psalm: 0 errors +- [x] CS Fixer: 0 issues +- [x] Tests: 25 passing (8 + 11 + 6) + +--- + +## Risks + +| Risk | Mitigation | +|------|------------| +| DSP haproxy blocks `/images/prune` | Log error, document in release notes | +| Prune removes non-AppAPI images | Standard `docker image prune` behavior, only dangling images | +| HaRP doesn't support prune | Skip + log, follow-up when upstream adds it | +| Shared image between ExApps | Docker refuses with 409, handled gracefully | diff --git a/specs/docker-image-cleanup/spec.md b/specs/docker-image-cleanup/spec.md new file mode 100644 index 00000000..1752cd7d --- /dev/null +++ b/specs/docker-image-cleanup/spec.md @@ -0,0 +1,118 @@ +# Spec: Automatic Clean-up of Outdated ExApp Docker Images + +**Issue:** [nextcloud/app_api#667](https://github.com/nextcloud/app_api/issues/667) +**Status:** Implemented + +--- + +## Problem + +When ExApps are updated, new Docker images are pulled but old ones are never removed, consuming disk space over time. + +--- + +## Solution + +Two complementary mechanisms: + +### Mechanism A: Periodic Background Prune Job + +- `DockerImageCleanupJob` — a `TimedJob` that calls `POST /images/prune` (filter: `dangling=true`) on each eligible Docker daemon. +- Interval configurable via `image_cleanup_interval_days` (default 7, 0 = disabled). +- Uses `getValueInt()` and dynamic `setInterval()` — Nextcloud handles scheduling. +- Skips HaRP daemons (upstream doesn't support prune endpoint yet). +- Errors are logged per-daemon; one failing daemon doesn't block others. + +### Mechanism B: Immediate Cleanup on Update + +- After a successful ExApp update, optionally removes the old image via `DELETE /images/{id}`. +- Controlled by `image_cleanup_on_update` checkbox (default: disabled). +- Uses `getValueBool()` to read the setting. +- Failures are logged as warnings, never block the update. + +### Admin UI + +Both settings exposed in `/settings/admin/app_api`: +- Number input for cleanup interval (days) +- Checkbox for "Remove old images after ExApp update" + +--- + +## Architecture + +``` +AdminSettings.vue + |-- saveOptions() / onCheckboxChanged() + v +ConfigController → IAppConfig (DB) + | | + v v +DockerImageCleanupJob Update.php + | | + v v +pruneImages() removeImage() +POST /images/prune DELETE /images/{id} +(bulk, per daemon) (specific old image) +``` + +### New on `DockerActions` + +- `removeImage(dockerUrl, imageId): string` — deletes specific image. 404/409 treated as success. +- `pruneImages(dockerUrl, filters): array` — bulk prune. Uses `Http::STATUS_*` constants, `Util::humanFileSize()` for logging. + +### `DockerImageCleanupJob` structure + +- Constructor: reads interval from config, passes to `setInterval()` +- `run()` → `isCleanupDisabled()` → `pruneImagesOnAllDaemons()` → `supportsPrune(daemon)` + +### Update command (`Update.php`) + +- Before deploy: captures old image name via `buildDeployParams()` + `buildBaseImageName()` +- After successful deploy: `removeOldImageIfEnabled()` checks setting, calls `removeImage()` + +--- + +## Configuration + +| Key | Type | Default | Notes | +|-----|------|---------|-------| +| `image_cleanup_interval_days` | int | `7` | 0 = disabled. Constant: `Application::CONF_IMAGE_CLEANUP_INTERVAL_DAYS` | +| `image_cleanup_on_update` | bool | `false` | Constant: `Application::CONF_IMAGE_CLEANUP_ON_UPDATE` | + +--- + +## Error Handling + +| Scenario | Behavior | +|----------|----------| +| Daemon unreachable | Log error, skip, continue to next | +| Image not found (404) | Treat as success | +| Image in use (409) | Log warning, skip | +| Prune fails | Log error, skip daemon | +| Cleanup fails during update | Log warning, don't block update | +| HaRP daemon | Log debug, skip | + +--- + +## Files Changed + +| File | Change | +|------|--------| +| `lib/BackgroundJob/DockerImageCleanupJob.php` | **New** — periodic prune job | +| `lib/DeployActions/DockerActions.php` | Added `removeImage()`, `pruneImages()` | +| `lib/Command/ExApp/Update.php` | Added `removeOldImageIfEnabled()` | +| `lib/AppInfo/Application.php` | Added config key constants | +| `lib/Settings/Admin.php` | Added settings to initial state | +| `src/components/AdminSettings.vue` | Added interval input + checkbox | +| `appinfo/info.xml` | Registered background job | +| `tests/php/BackgroundJob/DockerImageCleanupJobTest.php` | **New** — 8 tests | +| `tests/php/DeployActions/DockerActionsImageCleanupTest.php` | **New** — 11 tests | +| `tests/php/Command/ExApp/UpdateImageCleanupTest.php` | **New** — 6 tests | + +--- + +## Out of Scope + +- HaRP prune API (requires upstream changes) +- Docker Socket Proxy haproxy config (requires upstream changes) +- Kubernetes image cleanup (different mechanism) diff --git a/src/components/AdminSettings.vue b/src/components/AdminSettings.vue index db466c5e..b8763d43 100644 --- a/src/components/AdminSettings.vue +++ b/src/components/AdminSettings.vue @@ -49,6 +49,28 @@ :aria-label-combobox="t('app_api', 'ExApp container restart policy')" @update:model-value="onInput" /> + +
+ +
+
+ + {{ t('app_api', 'Remove old images after ExApp update') }} + +
+ @@ -63,6 +85,7 @@ import NcSettingsSection from '@nextcloud/vue/components/NcSettingsSection' import NcInputField from '@nextcloud/vue/components/NcInputField' import NcSelect from '@nextcloud/vue/components/NcSelect' import NcNoteCard from '@nextcloud/vue/components/NcNoteCard' +import NcCheckboxRadioSwitch from '@nextcloud/vue/components/NcCheckboxRadioSwitch' import AppAPIIcon from './icons/AppAPIIcon.vue' import DaemonConfigList from './DaemonConfig/DaemonConfigList.vue' @@ -76,6 +99,7 @@ export default { NcNoteCard, NcInputField, NcSelect, + NcCheckboxRadioSwitch, }, data() { return { @@ -103,6 +127,7 @@ export default { this.saveOptions({ init_timeout: this.state.init_timeout, container_restart_policy: this.state.container_restart_policy, + image_cleanup_interval_days: this.state.image_cleanup_interval_days, }) }, 2000)() }, diff --git a/tests/php/BackgroundJob/DockerImageCleanupJobTest.php b/tests/php/BackgroundJob/DockerImageCleanupJobTest.php new file mode 100644 index 00000000..9d765ae2 --- /dev/null +++ b/tests/php/BackgroundJob/DockerImageCleanupJobTest.php @@ -0,0 +1,230 @@ +createJob(0); + + $this->daemonConfigService->expects(self::never()) + ->method('getRegisteredDaemonConfigs'); + + $this->dockerActions->expects(self::never()) + ->method('pruneImages'); + + $this->invokeRun($job); + } + + public function testRunPrunesDockerDaemons(): void { + $job = $this->createJob(); + + $daemon = $this->createDaemonConfig('local-docker', DockerActions::DEPLOY_ID); + + $this->daemonConfigService->method('getRegisteredDaemonConfigs') + ->willReturn([$daemon]); + + $this->dockerActions->expects(self::once()) + ->method('initGuzzleClient') + ->with($daemon); + + $this->dockerActions->expects(self::once()) + ->method('buildDockerUrl') + ->with($daemon) + ->willReturn('http://localhost'); + + $this->dockerActions->expects(self::once()) + ->method('pruneImages') + ->with('http://localhost', ['dangling' => ['true']]) + ->willReturn(['SpaceReclaimed' => 1048576, 'ImagesDeleted' => []]); + + $this->invokeRun($job); + } + + public function testRunSkipsNonDockerDaemons(): void { + $job = $this->createJob(); + + $k8sDaemon = $this->createDaemonConfig('k8s-cluster', 'kubernetes-install'); + $manualDaemon = $this->createDaemonConfig('manual', 'manual-install'); + + $this->daemonConfigService->method('getRegisteredDaemonConfigs') + ->willReturn([$k8sDaemon, $manualDaemon]); + + $this->dockerActions->expects(self::never()) + ->method('pruneImages'); + + $this->invokeRun($job); + } + + public function testRunSkipsHarpDaemons(): void { + $job = $this->createJob(); + + $harpDaemon = $this->createDaemonConfig( + 'harp-daemon', + DockerActions::DEPLOY_ID, + ['harp' => true] + ); + + $this->daemonConfigService->method('getRegisteredDaemonConfigs') + ->willReturn([$harpDaemon]); + + $this->dockerActions->expects(self::never()) + ->method('pruneImages'); + + $this->logger->expects(self::once()) + ->method('debug') + ->with(self::stringContains('Skipping image prune for HaRP daemon')); + + $this->invokeRun($job); + } + + public function testRunLogsErrorWhenPruneFails(): void { + $job = $this->createJob(); + + $daemon = $this->createDaemonConfig('local-docker', DockerActions::DEPLOY_ID); + + $this->daemonConfigService->method('getRegisteredDaemonConfigs') + ->willReturn([$daemon]); + + $this->dockerActions->method('buildDockerUrl')->willReturn('http://localhost'); + $this->dockerActions->method('pruneImages') + ->willReturn(['error' => 'Connection refused']); + + $this->logger->expects(self::once()) + ->method('error') + ->with(self::stringContains('Connection refused')); + + $this->invokeRun($job); + } + + public function testRunCatchesExceptionsPerDaemon(): void { + $job = $this->createJob(); + + $daemon1 = $this->createDaemonConfig('failing-daemon', DockerActions::DEPLOY_ID); + $daemon2 = $this->createDaemonConfig('working-daemon', DockerActions::DEPLOY_ID); + + $this->daemonConfigService->method('getRegisteredDaemonConfigs') + ->willReturn([$daemon1, $daemon2]); + + $this->dockerActions->method('buildDockerUrl')->willReturn('http://localhost'); + + $callCount = 0; + $this->dockerActions->method('pruneImages') + ->willReturnCallback(function () use (&$callCount) { + $callCount++; + if ($callCount === 1) { + throw new \Exception('Daemon unreachable'); + } + return ['SpaceReclaimed' => 0, 'ImagesDeleted' => null]; + }); + + $this->logger->expects(self::once()) + ->method('error') + ->with(self::stringContains('Daemon unreachable')); + + $this->invokeRun($job); + + // Verify the second daemon was still processed + self::assertSame(2, $callCount); + } + + public function testRunPrunesMultipleDaemons(): void { + $job = $this->createJob(); + + $daemon1 = $this->createDaemonConfig('docker-1', DockerActions::DEPLOY_ID); + $daemon2 = $this->createDaemonConfig('docker-2', DockerActions::DEPLOY_ID); + + $this->daemonConfigService->method('getRegisteredDaemonConfigs') + ->willReturn([$daemon1, $daemon2]); + + $this->dockerActions->method('buildDockerUrl')->willReturn('http://localhost'); + + $this->dockerActions->expects(self::exactly(2)) + ->method('pruneImages'); + + $this->invokeRun($job); + } + + public function testRunFiltersMixedDaemons(): void { + $job = $this->createJob(); + + $dockerDaemon = $this->createDaemonConfig('docker', DockerActions::DEPLOY_ID); + $harpDaemon = $this->createDaemonConfig('harp', DockerActions::DEPLOY_ID, ['harp' => true]); + $k8sDaemon = $this->createDaemonConfig('k8s', 'kubernetes-install'); + + $this->daemonConfigService->method('getRegisteredDaemonConfigs') + ->willReturn([$dockerDaemon, $harpDaemon, $k8sDaemon]); + + $this->dockerActions->method('buildDockerUrl')->willReturn('http://localhost'); + + // Only the plain Docker daemon should be pruned + $this->dockerActions->expects(self::once()) + ->method('pruneImages'); + + $this->invokeRun($job); + } + + private function createJob(int $intervalDays = 7): DockerImageCleanupJob { + $this->appConfig = $this->createMock(IAppConfig::class); + $this->daemonConfigService = $this->createMock(DaemonConfigService::class); + $this->dockerActions = $this->createMock(DockerActions::class); + $this->logger = $this->createMock(LoggerInterface::class); + $this->timeFactory = $this->createMock(ITimeFactory::class); + $this->timeFactory->method('getTime')->willReturn(time()); + + $this->appConfig->method('getValueInt') + ->willReturnCallback(function (string $appId, string $key, int $default) use ($intervalDays) { + if ($key === Application::CONF_IMAGE_CLEANUP_INTERVAL_DAYS) { + return $intervalDays; + } + return $default; + }); + + return new DockerImageCleanupJob( + $this->timeFactory, + $this->appConfig, + $this->daemonConfigService, + $this->dockerActions, + $this->logger, + ); + } + + private function createDaemonConfig(string $name, string $deployId, array $deployConfig = []): DaemonConfig { + return new DaemonConfig([ + 'name' => $name, + 'accepts_deploy_id' => $deployId, + 'deploy_config' => $deployConfig, + ]); + } + + /** + * Invoke the protected run() method via reflection. + */ + private function invokeRun(DockerImageCleanupJob $job): void { + $method = new \ReflectionMethod($job, 'run'); + $method->invoke($job, null); + } +} diff --git a/tests/php/Command/ExApp/UpdateImageCleanupTest.php b/tests/php/Command/ExApp/UpdateImageCleanupTest.php new file mode 100644 index 00000000..6900c96b --- /dev/null +++ b/tests/php/Command/ExApp/UpdateImageCleanupTest.php @@ -0,0 +1,175 @@ +dockerActions = $this->createMock(DockerActions::class); + $this->logger = $this->createMock(LoggerInterface::class); + $this->appConfig = $this->createMock(IAppConfig::class); + + $this->dockerActions->method('getAcceptsDeployId') + ->willReturn(DockerActions::DEPLOY_ID); + + $this->command = new Update( + $this->createMock(AppAPIService::class), + $this->createMock(ExAppService::class), + $this->createMock(DaemonConfigService::class), + $this->dockerActions, + $this->createMock(KubernetesActions::class), + $this->createMock(ManualActions::class), + $this->logger, + $this->createMock(ExAppArchiveFetcher::class), + $this->createMock(ExAppFetcher::class), + $this->createMock(ExAppDeployOptionsService::class), + $this->appConfig, + ); + } + + public function testRemovesOldImageWhenEnabled(): void { + $daemon = $this->createDaemonConfig(DockerActions::DEPLOY_ID); + + $this->appConfig->method('getValueBool') + ->willReturnCallback(function (string $appId, string $key, bool $default) { + if ($key === Application::CONF_IMAGE_CLEANUP_ON_UPDATE) { + return true; + } + return $default; + }); + + $this->dockerActions->expects(self::once()) + ->method('buildDockerUrl') + ->with($daemon) + ->willReturn('http://localhost'); + + $this->dockerActions->expects(self::once()) + ->method('removeImage') + ->with('http://localhost', 'ghcr.io/nextcloud/app:1.0') + ->willReturn(''); + + $this->logger->expects(self::once()) + ->method('info') + ->with(self::stringContains('removed after updating')); + + $this->invokeRemoveOldImage('ghcr.io/nextcloud/app:1.0', $daemon, 'test-app'); + } + + public function testSkipsWhenDisabled(): void { + $daemon = $this->createDaemonConfig(DockerActions::DEPLOY_ID); + + $this->appConfig->method('getValueBool') + ->willReturnCallback(function (string $appId, string $key, bool $default) { + if ($key === Application::CONF_IMAGE_CLEANUP_ON_UPDATE) { + return false; + } + return $default; + }); + + $this->dockerActions->expects(self::never()) + ->method('removeImage'); + + $this->invokeRemoveOldImage('ghcr.io/nextcloud/app:1.0', $daemon, 'test-app'); + } + + public function testSkipsWhenOldImageNameEmpty(): void { + $daemon = $this->createDaemonConfig(DockerActions::DEPLOY_ID); + + $this->dockerActions->expects(self::never()) + ->method('removeImage'); + + $this->invokeRemoveOldImage('', $daemon, 'test-app'); + } + + public function testSkipsForNonDockerDaemons(): void { + $daemon = $this->createDaemonConfig('kubernetes-install'); + + $this->appConfig->method('getValueBool') + ->willReturn(true); + + $this->dockerActions->expects(self::never()) + ->method('removeImage'); + + $this->invokeRemoveOldImage('ghcr.io/nextcloud/app:1.0', $daemon, 'test-app'); + } + + public function testLogsWarningWhenRemoveFails(): void { + $daemon = $this->createDaemonConfig(DockerActions::DEPLOY_ID); + + $this->appConfig->method('getValueBool') + ->willReturnCallback(function (string $appId, string $key, bool $default) { + if ($key === Application::CONF_IMAGE_CLEANUP_ON_UPDATE) { + return true; + } + return $default; + }); + + $this->dockerActions->method('buildDockerUrl')->willReturn('http://localhost'); + $this->dockerActions->method('removeImage') + ->willReturn('Failed to remove image: server error'); + + $this->logger->expects(self::once()) + ->method('warning') + ->with(self::stringContains('Old image cleanup for test-app')); + + $this->invokeRemoveOldImage('ghcr.io/nextcloud/app:1.0', $daemon, 'test-app'); + } + + public function testDefaultSettingIsDisabled(): void { + $daemon = $this->createDaemonConfig(DockerActions::DEPLOY_ID); + + // Don't configure appConfig — let it return the default (false) + $this->appConfig->method('getValueBool') + ->willReturnCallback(function (string $appId, string $key, bool $default) { + return $default; + }); + + $this->dockerActions->expects(self::never()) + ->method('removeImage'); + + $this->invokeRemoveOldImage('ghcr.io/nextcloud/app:1.0', $daemon, 'test-app'); + } + + private function createDaemonConfig(string $deployId): DaemonConfig { + return new DaemonConfig([ + 'name' => 'test-daemon', + 'accepts_deploy_id' => $deployId, + 'deploy_config' => [], + ]); + } + + private function invokeRemoveOldImage(string $oldImageName, DaemonConfig $daemonConfig, string $appId): void { + $method = new \ReflectionMethod($this->command, 'removeOldImageIfEnabled'); + $method->invoke($this->command, $oldImageName, $daemonConfig, $appId); + } +} diff --git a/tests/php/DeployActions/DockerActionsImageCleanupTest.php b/tests/php/DeployActions/DockerActionsImageCleanupTest.php new file mode 100644 index 00000000..61832d40 --- /dev/null +++ b/tests/php/DeployActions/DockerActionsImageCleanupTest.php @@ -0,0 +1,236 @@ +appConfig = $this->createMock(IAppConfig::class); + $this->logger = $this->createMock(LoggerInterface::class); + $this->guzzleClient = $this->createMock(Client::class); + + $this->appConfig->method('getValueString') + ->willReturnCallback(function (string $appId, string $key, string $default) { + if ($key === 'docker_api_version') { + return ''; + } + return $default; + }); + + $this->dockerActions = new DockerActions( + $this->logger, + $this->appConfig, + $this->createMock(IConfig::class), + $this->createMock(ICertificateManager::class), + $this->createMock(IAppManager::class), + $this->createMock(IURLGenerator::class), + $this->createMock(AppAPICommonService::class), + $this->createMock(ExAppService::class), + $this->createMock(ITempManager::class), + $this->createMock(ICrypto::class), + $this->createMock(ExAppDeployOptionsService::class), + ); + + // Inject mock Guzzle client via reflection (private property) + $reflection = new \ReflectionClass($this->dockerActions); + $property = $reflection->getProperty('guzzleClient'); + $property->setValue($this->dockerActions, $this->guzzleClient); + } + + // --- removeImage() tests --- + + public function testRemoveImageReturnsEmptyStringOnSuccess(): void { + $this->guzzleClient->expects(self::once()) + ->method('delete') + ->with(self::DOCKER_URL . '/' . self::API_VERSION . '/images/test-image:1.0') + ->willReturn(new Response(200)); + + $this->logger->expects(self::once()) + ->method('info') + ->with(self::stringContains('Successfully removed Docker image')); + + $result = $this->dockerActions->removeImage(self::DOCKER_URL, 'test-image:1.0'); + + self::assertSame('', $result); + } + + public function testRemoveImageReturnsEmptyStringOn404(): void { + $this->guzzleClient->method('delete')->willThrowException( + new ClientException('Not Found', new Request('DELETE', ''), new Response(404)) + ); + + $this->logger->expects(self::once()) + ->method('debug') + ->with(self::stringContains('not found (already removed)')); + + $result = $this->dockerActions->removeImage(self::DOCKER_URL, 'missing-image:1.0'); + + self::assertSame('', $result); + } + + public function testRemoveImageReturnsEmptyStringOn409(): void { + $this->guzzleClient->method('delete')->willThrowException( + new ClientException('Conflict', new Request('DELETE', ''), new Response(409)) + ); + + $this->logger->expects(self::once()) + ->method('warning') + ->with(self::stringContains('in use, skipping removal')); + + $result = $this->dockerActions->removeImage(self::DOCKER_URL, 'in-use-image:1.0'); + + self::assertSame('', $result); + } + + public function testRemoveImageReturnsErrorOnServerError(): void { + $this->guzzleClient->method('delete')->willThrowException( + new ClientException('Server Error', new Request('DELETE', ''), new Response(500)) + ); + + $this->logger->expects(self::once()) + ->method('error') + ->with(self::stringContains('Failed to remove image')); + + $result = $this->dockerActions->removeImage(self::DOCKER_URL, 'some-image:1.0'); + + self::assertNotEmpty($result); + self::assertStringContainsString('Failed to remove image', $result); + } + + public function testRemoveImageReturnsErrorOnUnexpectedStatusCode(): void { + $this->guzzleClient->method('delete')->willReturn(new Response(204)); + + $result = $this->dockerActions->removeImage(self::DOCKER_URL, 'test-image:1.0'); + + self::assertNotEmpty($result); + self::assertStringContainsString('Unexpected status 204', $result); + } + + // --- pruneImages() tests --- + + public function testPruneImagesReturnsResultWithFilters(): void { + $body = json_encode([ + 'SpaceReclaimed' => 104857600, + 'ImagesDeleted' => [ + ['Untagged' => 'old-image:1.0'], + ['Deleted' => 'sha256:abc123'], + ], + ], JSON_THROW_ON_ERROR); + $this->guzzleClient->expects(self::once()) + ->method('post') + ->with( + self::DOCKER_URL . '/' . self::API_VERSION . '/images/prune', + self::callback(function (array $options) { + return isset($options['query']['filters']) + && $options['query']['filters'] === '{"dangling":["true"]}'; + }) + ) + ->willReturn(new Response(200, [], $body)); + + $result = $this->dockerActions->pruneImages(self::DOCKER_URL, ['dangling' => ['true']]); + + self::assertSame(104857600, $result['SpaceReclaimed']); + self::assertCount(2, $result['ImagesDeleted']); + } + + public function testPruneImagesWithNoFiltersOmitsQueryParam(): void { + $body = json_encode(['SpaceReclaimed' => 0, 'ImagesDeleted' => null], JSON_THROW_ON_ERROR); + $this->guzzleClient->expects(self::once()) + ->method('post') + ->with( + self::DOCKER_URL . '/' . self::API_VERSION . '/images/prune', + self::callback(function (array $options) { + return !isset($options['query']['filters']); + }) + ) + ->willReturn(new Response(200, [], $body)); + + $result = $this->dockerActions->pruneImages(self::DOCKER_URL); + + self::assertSame(0, $result['SpaceReclaimed']); + } + + public function testPruneImagesReturnsErrorOnGuzzleException(): void { + $this->guzzleClient->method('post')->willThrowException( + new ClientException('Forbidden', new Request('POST', ''), new Response(403)) + ); + + $result = $this->dockerActions->pruneImages(self::DOCKER_URL, ['dangling' => ['true']]); + + self::assertArrayHasKey('error', $result); + } + + public function testPruneImagesLogsInfoWithImageCount(): void { + $body = json_encode([ + 'SpaceReclaimed' => 1048576, + 'ImagesDeleted' => [['Deleted' => 'sha256:abc']], + ], JSON_THROW_ON_ERROR); + $this->guzzleClient->method('post')->willReturn(new Response(200, [], $body)); + + $this->logger->expects(self::once()) + ->method('info') + ->with(self::stringContains('1 images removed')); + + $this->dockerActions->pruneImages(self::DOCKER_URL); + } + + public function testPruneImagesLogsErrorOnFailure(): void { + $this->guzzleClient->method('post')->willThrowException( + new ClientException('Connection refused', new Request('POST', ''), new Response(500)) + ); + + $this->logger->expects(self::once()) + ->method('error') + ->with(self::stringContains('Failed to prune Docker images')); + + $this->dockerActions->pruneImages(self::DOCKER_URL); + } + + public function testPruneImagesHandlesNullImagesDeleted(): void { + $body = json_encode(['SpaceReclaimed' => 0, 'ImagesDeleted' => null], JSON_THROW_ON_ERROR); + $this->guzzleClient->method('post')->willReturn(new Response(200, [], $body)); + + $this->logger->expects(self::once()) + ->method('info') + ->with(self::stringContains('0 images removed')); + + $result = $this->dockerActions->pruneImages(self::DOCKER_URL); + + self::assertSame(0, $result['SpaceReclaimed']); + } +}