Skip to content

Commit 2812588

Browse files
oleksandr-ncoleksandr-nc
committed
feat(image-cleanup): event-driven cleanup of orphaned ExApp Docker images via HaRP
Signed-off-by: Oleksander Piskun <oleksandr2088@icloud.com>
1 parent 2f76f5e commit 2812588

17 files changed

Lines changed: 1212 additions & 6 deletions

‎js/app_api-adminSettings.js‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎js/app_api-adminSettings.js.map‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/AppInfo/Application.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,13 @@ class Application extends App implements IBootstrap {
4545
public const TEST_DEPLOY_INFO_XML = 'https://raw.githubusercontent.com/nextcloud/test-deploy/main/appinfo/info.xml';
4646
public const MINIMUM_HARP_VERSION = '0.3.0';
4747

48+
public const CONF_IMAGE_CLEANUP_ENABLED = 'image_cleanup_enabled';
49+
public const CONF_IMAGE_CLEANUP_GRACE_HOURS = 'image_cleanup_grace_hours';
50+
public const CONF_IMAGE_CLEANUP_DEFAULTS = [
51+
self::CONF_IMAGE_CLEANUP_ENABLED => true,
52+
self::CONF_IMAGE_CLEANUP_GRACE_HOURS => 24,
53+
];
54+
4855
public function __construct(array $urlParams = []) {
4956
parent::__construct(self::APP_ID, $urlParams);
5057

‎lib/BackgroundJob/OrphanedImageCleanupJob.php‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\AppAPI\BackgroundJob;
11+
12+
use OCA\AppAPI\DeployActions\DockerActions;
13+
use OCA\AppAPI\Service\DaemonConfigService;
14+
use OCP\AppFramework\Utility\ITimeFactory;
15+
use OCP\BackgroundJob\QueuedJob;
16+
use OCP\Util;
17+
use Psr\Log\LoggerInterface;
18+
19+
/**
20+
* Removes a Docker image that was orphaned by an ExApp uninstall or update.
21+
*
22+
* Scheduled via IJobList::scheduleAfter when an ExApp is removed (or updated to a new image).
23+
* The grace period (default 24h) is configured globally; it gives a window for reinstall/rollback
24+
* before the image is reclaimed. Docker's own DELETE-with-409 behaviour is the source of truth
25+
* for "is the image still in use" — if anything (a fresh container, a stopped container, another
26+
* ExApp using the same image) still references the ref, Docker refuses and the job logs and exits.
27+
*
28+
* QueuedJob handles self-removal automatically: the entry is removed from the job queue before
29+
* run() is called, so a failure here is not retried by re-queuing. The next orphan event (or a
30+
* manual --purge-now from the admin) is the next opportunity.
31+
*/
32+
class OrphanedImageCleanupJob extends QueuedJob {
33+
public function __construct(
34+
ITimeFactory $time,
35+
private readonly DaemonConfigService $daemonConfigService,
36+
private readonly DockerActions $dockerActions,
37+
private readonly LoggerInterface $logger,
38+
) {
39+
parent::__construct($time);
40+
}
41+
42+
protected function run($argument): void {
43+
if (!is_array($argument)) {
44+
$this->logger->warning('OrphanedImageCleanupJob received non-array argument; skipping.');
45+
return;
46+
}
47+
$daemonName = (string)($argument['daemon_id'] ?? '');
48+
$imageRef = (string)($argument['image_ref'] ?? '');
49+
$appid = (string)($argument['appid'] ?? '');
50+
51+
if ($daemonName === '' || $imageRef === '') {
52+
$this->logger->warning('OrphanedImageCleanupJob missing daemon_id or image_ref in argument; skipping.', [
53+
'argument' => $argument,
54+
]);
55+
return;
56+
}
57+
58+
$daemon = $this->daemonConfigService->getDaemonConfigByName($daemonName);
59+
if ($daemon === null) {
60+
$this->logger->info(sprintf(
61+
'OrphanedImageCleanupJob: daemon "%s" no longer exists; skipping cleanup of image "%s" (appid=%s).',
62+
$daemonName,
63+
$imageRef,
64+
$appid,
65+
));
66+
return;
67+
}
68+
69+
if ($daemon->getAcceptsDeployId() !== DockerActions::DEPLOY_ID) {
70+
// Kubernetes / Manual: image lifecycle isn't AppAPI's responsibility.
71+
$this->logger->debug(sprintf(
72+
'OrphanedImageCleanupJob: daemon "%s" is not a Docker daemon (deploy_id=%s); skipping image "%s" (appid=%s).',
73+
$daemonName,
74+
$daemon->getAcceptsDeployId(),
75+
$imageRef,
76+
$appid,
77+
));
78+
return;
79+
}
80+
81+
try {
82+
$result = $this->dockerActions->removeImage($daemon, $imageRef);
83+
} catch (\Throwable $e) {
84+
$this->logger->error(sprintf(
85+
'OrphanedImageCleanupJob: unexpected exception removing image "%s" on daemon "%s" (appid=%s): %s',
86+
$imageRef,
87+
$daemonName,
88+
$appid,
89+
$e->getMessage(),
90+
), ['exception' => $e]);
91+
return;
92+
}
93+
94+
if ($result['deleted'] === true) {
95+
if (($result['reason'] ?? null) === 'not_found') {
96+
$this->logger->info(sprintf(
97+
'OrphanedImageCleanupJob: image "%s" already gone on daemon "%s" (appid=%s).',
98+
$imageRef,
99+
$daemonName,
100+
$appid,
101+
));
102+
return;
103+
}
104+
$this->logger->info(sprintf(
105+
'OrphanedImageCleanupJob: removed image "%s" on daemon "%s" (appid=%s, freed=%s).',
106+
$imageRef,
107+
$daemonName,
108+
$appid,
109+
Util::humanFileSize((int)($result['bytes_freed'] ?? 0)),
110+
));
111+
return;
112+
}
113+
114+
$reason = (string)($result['reason'] ?? 'unknown');
115+
if ($reason === 'in_use') {
116+
$this->logger->info(sprintf(
117+
'OrphanedImageCleanupJob: image "%s" still in use on daemon "%s" (appid=%s); leaving it in place.',
118+
$imageRef,
119+
$daemonName,
120+
$appid,
121+
));
122+
return;
123+
}
124+
$this->logger->warning(sprintf(
125+
'OrphanedImageCleanupJob: failed to remove image "%s" on daemon "%s" (appid=%s, reason=%s).',
126+
$imageRef,
127+
$daemonName,
128+
$appid,
129+
$reason,
130+
));
131+
}
132+
}

‎lib/Command/ExApp/Unregister.php‎

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@
1515
use OCA\AppAPI\Service\AppAPIService;
1616
use OCA\AppAPI\Service\DaemonConfigService;
1717
use OCA\AppAPI\Service\ExAppDeployOptionsService;
18+
use OCA\AppAPI\Service\ExAppImageCleanupService;
1819
use OCA\AppAPI\Service\ExAppService;
20+
use OCA\AppAPI\Service\ImageCleanupChoice;
1921
use Symfony\Component\Console\Command\Command;
2022
use Symfony\Component\Console\Input\InputArgument;
2123
use Symfony\Component\Console\Input\InputInterface;
@@ -31,6 +33,7 @@ public function __construct(
3133
private readonly KubernetesActions $kubernetesActions,
3234
private readonly ExAppService $exAppService,
3335
private readonly ExAppDeployOptionsService $exAppDeployOptionsService,
36+
private readonly ExAppImageCleanupService $imageCleanupService,
3437
) {
3538
parent::__construct();
3639
}
@@ -53,18 +56,33 @@ protected function configure(): void {
5356
'Continue removal even if errors.');
5457
$this->addOption('keep-data', null, InputOption::VALUE_NONE, 'Keep ExApp data (volume) [deprecated, data is kept by default].');
5558
$this->addOption('rm-data', null, InputOption::VALUE_NONE, 'Remove ExApp data (persistent storage volume).');
59+
$this->addOption('purge-now', null, InputOption::VALUE_NONE, 'Delete the ExApp Docker image immediately, without the configured grace period.');
60+
$this->addOption('keep-image', null, InputOption::VALUE_NONE, 'Do not schedule any image cleanup; admin will remove the image manually.');
5661

5762
$this->addUsage('test_app');
5863
$this->addUsage('test_app --silent');
5964
$this->addUsage('test_app --rm-data');
6065
$this->addUsage('test_app --silent --force --rm-data');
66+
$this->addUsage('test_app --purge-now');
67+
$this->addUsage('test_app --keep-image');
6168
}
6269

6370
protected function execute(InputInterface $input, OutputInterface $output): int {
6471
$appId = $input->getArgument('appid');
6572
$silent = $input->getOption('silent');
6673
$force = $input->getOption('force');
6774
$rmData = $input->getOption('rm-data');
75+
$purgeNow = (bool)$input->getOption('purge-now');
76+
$keepImage = (bool)$input->getOption('keep-image');
77+
if ($purgeNow && $keepImage) {
78+
$output->writeln('<error>--purge-now and --keep-image are mutually exclusive.</error>');
79+
return 1;
80+
}
81+
$cleanupChoice = match (true) {
82+
$purgeNow => ImageCleanupChoice::PURGE_NOW,
83+
$keepImage => ImageCleanupChoice::KEEP,
84+
default => ImageCleanupChoice::GRACE,
85+
};
6886

6987
$exApp = $this->exAppService->getExApp($appId);
7088
if ($exApp === null) {
@@ -103,6 +121,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int
103121
if ($daemonConfig->getAcceptsDeployId() === $this->dockerActions->getAcceptsDeployId()) {
104122
$this->dockerActions->initGuzzleClient($daemonConfig);
105123

124+
// Capture the image ref BEFORE the container is removed; we still need
125+
// to know it after the removal so the cleanup job has something to delete.
126+
$capturedImageRef = $this->imageCleanupService->captureImageRef($daemonConfig, $appId);
127+
128+
$containerRemoved = false;
106129
if (boolval($exApp->getDeployConfig()['harp'] ?? false)) {
107130
if ($this->dockerActions->removeExApp($this->dockerActions->buildDockerUrl($daemonConfig), $exApp->getAppid(), removeData: $rmData)) {
108131
if (!$silent) {
@@ -113,6 +136,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
113136
return 1;
114137
}
115138
} else {
139+
$containerRemoved = true;
116140
if (!$silent) {
117141
$output->writeln(sprintf('ExApp %s successfully removed', $appId));
118142
}
@@ -130,8 +154,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int
130154
if (!$force) {
131155
return 1;
132156
}
133-
} elseif (!$silent) {
134-
$output->writeln(sprintf('ExApp %s container successfully removed', $appId));
157+
} else {
158+
$containerRemoved = true;
159+
if (!$silent) {
160+
$output->writeln(sprintf('ExApp %s container successfully removed', $appId));
161+
}
135162
}
136163
if ($rmData) {
137164
$volumeName = $this->dockerActions->buildExAppVolumeName($appId);
@@ -147,6 +174,17 @@ protected function execute(InputInterface $input, OutputInterface $output): int
147174
}
148175
}
149176
}
177+
178+
// Schedule image cleanup once the container is gone. Skipped silently if
179+
// the master toggle is off or the ref couldn't be captured.
180+
if ($containerRemoved) {
181+
$this->imageCleanupService->scheduleCleanup(
182+
$capturedImageRef,
183+
$exApp,
184+
$daemonConfig,
185+
$cleanupChoice,
186+
);
187+
}
150188
} elseif ($daemonConfig->getAcceptsDeployId() === $this->kubernetesActions->getAcceptsDeployId()) {
151189
$this->kubernetesActions->initGuzzleClient($daemonConfig);
152190
$harpK8sUrl = $this->kubernetesActions->buildHarpK8sUrl($daemonConfig);

‎lib/Command/ExApp/Update.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818
use OCA\AppAPI\Service\DaemonConfigService;
1919

2020
use OCA\AppAPI\Service\ExAppDeployOptionsService;
21+
use OCA\AppAPI\Service\ExAppImageCleanupService;
2122
use OCA\AppAPI\Service\ExAppService;
23+
use OCA\AppAPI\Service\ImageCleanupChoice;
2224
use Psr\Log\LoggerInterface;
2325
use Symfony\Component\Console\Command\Command;
2426
use Symfony\Component\Console\Input\InputArgument;
@@ -39,6 +41,7 @@ public function __construct(
3941
private readonly ExAppArchiveFetcher $exAppArchiveFetcher,
4042
private readonly ExAppFetcher $exAppFetcher,
4143
private readonly ExAppDeployOptionsService $exAppDeployOptionsService,
44+
private readonly ExAppImageCleanupService $imageCleanupService,
4245
) {
4346
parent::__construct();
4447
}
@@ -57,6 +60,8 @@ protected function configure(): void {
5760
$this->addOption('all', null, InputOption::VALUE_NONE, 'Updates all enabled and updatable apps');
5861
$this->addOption('showonly', null, InputOption::VALUE_NONE, 'Additional flag for "--all" to only show all updatable apps');
5962
$this->addOption('include-disabled', null, InputOption::VALUE_NONE, 'Additional flag for "--all" to also update disabled apps');
63+
$this->addOption('purge-old-image-now', null, InputOption::VALUE_NONE, 'Delete the old ExApp Docker image immediately after update, without the configured grace period.');
64+
$this->addOption('keep-old-image', null, InputOption::VALUE_NONE, 'Do not schedule any cleanup of the old ExApp image; admin will remove it manually.');
6065
}
6166

6267
protected function execute(InputInterface $input, OutputInterface $output): int {
@@ -67,6 +72,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int
6772
} elseif (!empty($appId) && $input->getOption('all')) {
6873
$output->writeln('<error>The "--all" flag is mutually exclusive with specifying app</error>');
6974
return 1;
75+
} elseif ((bool)$input->getOption('purge-old-image-now') && (bool)$input->getOption('keep-old-image')) {
76+
$output->writeln('<error>--purge-old-image-now and --keep-old-image are mutually exclusive.</error>');
77+
return 1;
7078
} elseif ($input->getOption('all')) {
7179
$apps = $this->exAppFetcher->get();
7280
$appsWithUpdates = array_filter($apps, function (array $app) {
@@ -209,6 +217,10 @@ private function updateExApp(InputInterface $input, OutputInterface $output, str
209217
$harpK8sUrl = null;
210218
$k8sRoles = [];
211219
if ($daemonConfig->getAcceptsDeployId() === $this->dockerActions->getAcceptsDeployId()) {
220+
// Capture the OLD image ref before the deploy replaces the container.
221+
// We schedule cleanup for it after the new image is verified running.
222+
$oldImageRef = $this->imageCleanupService->captureImageRef($daemonConfig, $appId);
223+
212224
$deployParams = $this->dockerActions->buildDeployParams($daemonConfig, $appInfo);
213225
if (boolval($exApp->getDeployConfig()['harp'] ?? false)) {
214226
$deployResult = $this->dockerActions->deployExAppHarp($exApp, $daemonConfig, $deployParams);
@@ -233,6 +245,19 @@ private function updateExApp(InputInterface $input, OutputInterface $output, str
233245
return 1;
234246
}
235247

248+
// Deploy + healthcheck both succeeded; safe to schedule cleanup of the old ref.
249+
$cleanupChoice = match (true) {
250+
(bool)$input->getOption('purge-old-image-now') => ImageCleanupChoice::PURGE_NOW,
251+
(bool)$input->getOption('keep-old-image') => ImageCleanupChoice::KEEP,
252+
default => ImageCleanupChoice::GRACE,
253+
};
254+
$this->imageCleanupService->scheduleCleanup(
255+
$oldImageRef,
256+
$exApp,
257+
$daemonConfig,
258+
$cleanupChoice,
259+
);
260+
236261
$exAppUrl = $this->dockerActions->resolveExAppUrl(
237262
$appId,
238263
$daemonConfig->getProtocol(),

‎lib/Controller/ConfigController.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,15 @@ public function __construct(
3030
#[NoCSRFRequired]
3131
public function setAdminConfig(array $values): DataResponse {
3232
foreach ($values as $key => $value) {
33-
$this->appConfig->setValueString(Application::APP_ID, $key, $value, lazy: true);
33+
// Preserve native types so bool/int settings (e.g. image cleanup) round-trip
34+
// correctly through getValueBool/getValueInt on the read side.
35+
if (is_bool($value)) {
36+
$this->appConfig->setValueBool(Application::APP_ID, $key, $value, lazy: true);
37+
} elseif (is_int($value)) {
38+
$this->appConfig->setValueInt(Application::APP_ID, $key, $value, lazy: true);
39+
} else {
40+
$this->appConfig->setValueString(Application::APP_ID, $key, (string)$value, lazy: true);
41+
}
3442
}
3543
return new DataResponse(1);
3644
}

‎lib/Controller/ExAppsPageController.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@
2222
use OCA\AppAPI\Service\AppAPIService;
2323
use OCA\AppAPI\Service\DaemonConfigService;
2424
use OCA\AppAPI\Service\ExAppDeployOptionsService;
25+
use OCA\AppAPI\Service\ExAppImageCleanupService;
2526
use OCA\AppAPI\Service\ExAppService;
27+
use OCA\AppAPI\Service\ImageCleanupChoice;
2628
use OCP\App\IAppManager;
2729
use OCP\AppFramework\Controller;
2830
use OCP\AppFramework\Http;
@@ -55,6 +57,7 @@ public function __construct(
5557
private readonly IAppManager $appManager,
5658
private readonly ExAppService $exAppService,
5759
private readonly ExAppDeployOptionsService $exAppDeployOptionsService,
60+
private readonly ExAppImageCleanupService $imageCleanupService,
5861
) {
5962
parent::__construct(Application::APP_ID, $request);
6063
}
@@ -432,7 +435,7 @@ public function updateApp(string $appId): JSONResponse {
432435
* Unregister ExApp, remove container by default
433436
*/
434437
#[PasswordConfirmationRequired]
435-
public function uninstallApp(string $appId, bool $removeContainer = true, bool $removeData = false): JSONResponse {
438+
public function uninstallApp(string $appId, bool $removeContainer = true, bool $removeData = false, string $imageCleanup = 'grace'): JSONResponse {
436439
$exApp = $this->exAppService->getExApp($appId);
437440
if ($exApp) {
438441
if ($exApp->getEnabled()) {
@@ -442,6 +445,8 @@ public function uninstallApp(string $appId, bool $removeContainer = true, bool $
442445
$daemonConfig = $this->daemonConfigService->getDaemonConfigByName($exApp->getDaemonConfigName());
443446
if ($daemonConfig->getAcceptsDeployId() === $this->dockerActions->getAcceptsDeployId()) {
444447
$this->dockerActions->initGuzzleClient($daemonConfig);
448+
// Capture before removal so we still have the ref to clean up afterwards.
449+
$capturedImageRef = $this->imageCleanupService->captureImageRef($daemonConfig, $appId);
445450
if (boolval($exApp->getDeployConfig()['harp'] ?? false)) {
446451
$this->dockerActions->removeExApp($this->dockerActions->buildDockerUrl($daemonConfig), $exApp->getAppid(), removeData: $removeData);
447452
} else {
@@ -452,6 +457,8 @@ public function uninstallApp(string $appId, bool $removeContainer = true, bool $
452457
}
453458
}
454459
}
460+
$choice = ImageCleanupChoice::tryFrom($imageCleanup) ?? ImageCleanupChoice::GRACE;
461+
$this->imageCleanupService->scheduleCleanup($capturedImageRef, $exApp, $daemonConfig, $choice);
455462
}
456463
$this->exAppService->unregisterExApp($appId);
457464
}

0 commit comments

Comments
 (0)