Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion js/app_api-adminSettings.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/app_api-adminSettings.js.map

Large diffs are not rendered by default.

7 changes: 7 additions & 0 deletions lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ class Application extends App implements IBootstrap {
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_ENABLED = 'image_cleanup_enabled';
public const CONF_IMAGE_CLEANUP_GRACE_HOURS = 'image_cleanup_grace_hours';
public const CONF_IMAGE_CLEANUP_DEFAULTS = [
self::CONF_IMAGE_CLEANUP_ENABLED => true,
self::CONF_IMAGE_CLEANUP_GRACE_HOURS => 24,
];

public function __construct(array $urlParams = []) {
parent::__construct(self::APP_ID, $urlParams);

Expand Down
132 changes: 132 additions & 0 deletions lib/BackgroundJob/OrphanedImageCleanupJob.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\AppAPI\BackgroundJob;

use OCA\AppAPI\DeployActions\DockerActions;
use OCA\AppAPI\Service\DaemonConfigService;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\BackgroundJob\QueuedJob;
use OCP\Util;
use Psr\Log\LoggerInterface;

/**
* Removes a Docker image that was orphaned by an ExApp uninstall or update.
*
* Scheduled via IJobList::scheduleAfter when an ExApp is removed (or updated to a new image).
* The grace period (default 24h) is configured globally; it gives a window for reinstall/rollback
* before the image is reclaimed. Docker's own DELETE-with-409 behaviour is the source of truth
* for "is the image still in use" — if anything (a fresh container, a stopped container, another
* ExApp using the same image) still references the ref, Docker refuses and the job logs and exits.
*
* QueuedJob handles self-removal automatically: the entry is removed from the job queue before
* run() is called, so a failure here is not retried by re-queuing. The next orphan event (or a
* manual --purge-now from the admin) is the next opportunity.
*/
class OrphanedImageCleanupJob extends QueuedJob {
public function __construct(
ITimeFactory $time,
private readonly DaemonConfigService $daemonConfigService,
private readonly DockerActions $dockerActions,
private readonly LoggerInterface $logger,
) {
parent::__construct($time);
}

protected function run($argument): void {
if (!is_array($argument)) {
$this->logger->warning('OrphanedImageCleanupJob received non-array argument; skipping.');
return;
}
$daemonName = (string)($argument['daemon_id'] ?? '');
$imageRef = (string)($argument['image_ref'] ?? '');
$appid = (string)($argument['appid'] ?? '');

if ($daemonName === '' || $imageRef === '') {
$this->logger->warning('OrphanedImageCleanupJob missing daemon_id or image_ref in argument; skipping.', [
'argument' => $argument,
]);
return;
}

$daemon = $this->daemonConfigService->getDaemonConfigByName($daemonName);
if ($daemon === null) {
$this->logger->info(sprintf(
'OrphanedImageCleanupJob: daemon "%s" no longer exists; skipping cleanup of image "%s" (appid=%s).',
$daemonName,
$imageRef,
$appid,
));
return;
}

if ($daemon->getAcceptsDeployId() !== DockerActions::DEPLOY_ID) {
// Kubernetes / Manual: image lifecycle isn't AppAPI's responsibility.
$this->logger->debug(sprintf(
'OrphanedImageCleanupJob: daemon "%s" is not a Docker daemon (deploy_id=%s); skipping image "%s" (appid=%s).',
$daemonName,
$daemon->getAcceptsDeployId(),
$imageRef,
$appid,
));
return;
}

try {
$result = $this->dockerActions->removeImage($daemon, $imageRef);
} catch (\Throwable $e) {
$this->logger->error(sprintf(
'OrphanedImageCleanupJob: unexpected exception removing image "%s" on daemon "%s" (appid=%s): %s',
$imageRef,
$daemonName,
$appid,
$e->getMessage(),
), ['exception' => $e]);
return;
}

if ($result['deleted'] === true) {
if (($result['reason'] ?? null) === 'not_found') {
$this->logger->info(sprintf(
'OrphanedImageCleanupJob: image "%s" already gone on daemon "%s" (appid=%s).',
$imageRef,
$daemonName,
$appid,
));
return;
}
$this->logger->info(sprintf(
'OrphanedImageCleanupJob: removed image "%s" on daemon "%s" (appid=%s, freed=%s).',
$imageRef,
$daemonName,
$appid,
Util::humanFileSize((int)($result['bytes_freed'] ?? 0)),
));
return;
}

$reason = (string)($result['reason'] ?? 'unknown');
if ($reason === 'in_use') {
$this->logger->info(sprintf(
'OrphanedImageCleanupJob: image "%s" still in use on daemon "%s" (appid=%s); leaving it in place.',
$imageRef,
$daemonName,
$appid,
));
return;
}
$this->logger->warning(sprintf(
'OrphanedImageCleanupJob: failed to remove image "%s" on daemon "%s" (appid=%s, reason=%s).',
$imageRef,
$daemonName,
$appid,
$reason,
));
}
}
42 changes: 40 additions & 2 deletions lib/Command/ExApp/Unregister.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
use OCA\AppAPI\Service\AppAPIService;
use OCA\AppAPI\Service\DaemonConfigService;
use OCA\AppAPI\Service\ExAppDeployOptionsService;
use OCA\AppAPI\Service\ExAppImageCleanupService;
use OCA\AppAPI\Service\ExAppService;
use OCA\AppAPI\Service\ImageCleanupChoice;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
Expand All @@ -31,6 +33,7 @@ public function __construct(
private readonly KubernetesActions $kubernetesActions,
private readonly ExAppService $exAppService,
private readonly ExAppDeployOptionsService $exAppDeployOptionsService,
private readonly ExAppImageCleanupService $imageCleanupService,
) {
parent::__construct();
}
Expand All @@ -53,18 +56,33 @@ protected function configure(): void {
'Continue removal even if errors.');
$this->addOption('keep-data', null, InputOption::VALUE_NONE, 'Keep ExApp data (volume) [deprecated, data is kept by default].');
$this->addOption('rm-data', null, InputOption::VALUE_NONE, 'Remove ExApp data (persistent storage volume).');
$this->addOption('purge-now', null, InputOption::VALUE_NONE, 'Delete the ExApp Docker image immediately, without the configured grace period.');
$this->addOption('keep-image', null, InputOption::VALUE_NONE, 'Do not schedule any image cleanup; admin will remove the image manually.');

$this->addUsage('test_app');
$this->addUsage('test_app --silent');
$this->addUsage('test_app --rm-data');
$this->addUsage('test_app --silent --force --rm-data');
$this->addUsage('test_app --purge-now');
$this->addUsage('test_app --keep-image');
}

protected function execute(InputInterface $input, OutputInterface $output): int {
$appId = $input->getArgument('appid');
$silent = $input->getOption('silent');
$force = $input->getOption('force');
$rmData = $input->getOption('rm-data');
$purgeNow = (bool)$input->getOption('purge-now');
$keepImage = (bool)$input->getOption('keep-image');
if ($purgeNow && $keepImage) {
$output->writeln('<error>--purge-now and --keep-image are mutually exclusive.</error>');
return 1;
}
$cleanupChoice = match (true) {
$purgeNow => ImageCleanupChoice::PURGE_NOW,
$keepImage => ImageCleanupChoice::KEEP,
default => ImageCleanupChoice::GRACE,
};

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

// Capture the image ref BEFORE the container is removed; we still need
// to know it after the removal so the cleanup job has something to delete.
$capturedImageRef = $this->imageCleanupService->captureImageRef($daemonConfig, $appId);

$containerRemoved = false;
if (boolval($exApp->getDeployConfig()['harp'] ?? false)) {
if ($this->dockerActions->removeExApp($this->dockerActions->buildDockerUrl($daemonConfig), $exApp->getAppid(), removeData: $rmData)) {
if (!$silent) {
Expand All @@ -113,6 +136,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
return 1;
}
} else {
$containerRemoved = true;
if (!$silent) {
$output->writeln(sprintf('ExApp %s successfully removed', $appId));
}
Expand All @@ -130,8 +154,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int
if (!$force) {
return 1;
}
} elseif (!$silent) {
$output->writeln(sprintf('ExApp %s container successfully removed', $appId));
} else {
$containerRemoved = true;
if (!$silent) {
$output->writeln(sprintf('ExApp %s container successfully removed', $appId));
}
}
if ($rmData) {
$volumeName = $this->dockerActions->buildExAppVolumeName($appId);
Expand All @@ -147,6 +174,17 @@ protected function execute(InputInterface $input, OutputInterface $output): int
}
}
}

// Schedule image cleanup once the container is gone. Skipped silently if
// the master toggle is off or the ref couldn't be captured.
if ($containerRemoved) {
$this->imageCleanupService->scheduleCleanup(
$capturedImageRef,
$exApp,
$daemonConfig,
$cleanupChoice,
);
}
} elseif ($daemonConfig->getAcceptsDeployId() === $this->kubernetesActions->getAcceptsDeployId()) {
$this->kubernetesActions->initGuzzleClient($daemonConfig);
$harpK8sUrl = $this->kubernetesActions->buildHarpK8sUrl($daemonConfig);
Expand Down
25 changes: 25 additions & 0 deletions lib/Command/ExApp/Update.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@
use OCA\AppAPI\Service\DaemonConfigService;

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

protected function execute(InputInterface $input, OutputInterface $output): int {
Expand All @@ -67,6 +72,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int
} elseif (!empty($appId) && $input->getOption('all')) {
$output->writeln('<error>The "--all" flag is mutually exclusive with specifying app</error>');
return 1;
} elseif ((bool)$input->getOption('purge-old-image-now') && (bool)$input->getOption('keep-old-image')) {
$output->writeln('<error>--purge-old-image-now and --keep-old-image are mutually exclusive.</error>');
return 1;
} elseif ($input->getOption('all')) {
$apps = $this->exAppFetcher->get();
$appsWithUpdates = array_filter($apps, function (array $app) {
Expand Down Expand Up @@ -209,6 +217,10 @@ private function updateExApp(InputInterface $input, OutputInterface $output, str
$harpK8sUrl = null;
$k8sRoles = [];
if ($daemonConfig->getAcceptsDeployId() === $this->dockerActions->getAcceptsDeployId()) {
// Capture the OLD image ref before the deploy replaces the container.
// We schedule cleanup for it after the new image is verified running.
$oldImageRef = $this->imageCleanupService->captureImageRef($daemonConfig, $appId);

$deployParams = $this->dockerActions->buildDeployParams($daemonConfig, $appInfo);
if (boolval($exApp->getDeployConfig()['harp'] ?? false)) {
$deployResult = $this->dockerActions->deployExAppHarp($exApp, $daemonConfig, $deployParams);
Expand All @@ -233,6 +245,19 @@ private function updateExApp(InputInterface $input, OutputInterface $output, str
return 1;
}

// Deploy + healthcheck both succeeded; safe to schedule cleanup of the old ref.
$cleanupChoice = match (true) {
(bool)$input->getOption('purge-old-image-now') => ImageCleanupChoice::PURGE_NOW,
(bool)$input->getOption('keep-old-image') => ImageCleanupChoice::KEEP,
default => ImageCleanupChoice::GRACE,
};
$this->imageCleanupService->scheduleCleanup(
$oldImageRef,
$exApp,
$daemonConfig,
$cleanupChoice,
);

$exAppUrl = $this->dockerActions->resolveExAppUrl(
$appId,
$daemonConfig->getProtocol(),
Expand Down
10 changes: 9 additions & 1 deletion lib/Controller/ConfigController.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,15 @@ public function __construct(
#[NoCSRFRequired]
public function setAdminConfig(array $values): DataResponse {
foreach ($values as $key => $value) {
$this->appConfig->setValueString(Application::APP_ID, $key, $value, lazy: true);
// Preserve native types so bool/int settings (e.g. image cleanup) round-trip
// correctly through getValueBool/getValueInt on the read side.
if (is_bool($value)) {
$this->appConfig->setValueBool(Application::APP_ID, $key, $value, lazy: true);
} elseif (is_int($value)) {
$this->appConfig->setValueInt(Application::APP_ID, $key, $value, lazy: true);
} else {
$this->appConfig->setValueString(Application::APP_ID, $key, (string)$value, lazy: true);
}
}
return new DataResponse(1);
}
Expand Down
9 changes: 8 additions & 1 deletion lib/Controller/ExAppsPageController.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
use OCA\AppAPI\Service\AppAPIService;
use OCA\AppAPI\Service\DaemonConfigService;
use OCA\AppAPI\Service\ExAppDeployOptionsService;
use OCA\AppAPI\Service\ExAppImageCleanupService;
use OCA\AppAPI\Service\ExAppService;
use OCA\AppAPI\Service\ImageCleanupChoice;
use OCP\App\IAppManager;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http;
Expand Down Expand Up @@ -55,6 +57,7 @@ public function __construct(
private readonly IAppManager $appManager,
private readonly ExAppService $exAppService,
private readonly ExAppDeployOptionsService $exAppDeployOptionsService,
private readonly ExAppImageCleanupService $imageCleanupService,
) {
parent::__construct(Application::APP_ID, $request);
}
Expand Down Expand Up @@ -432,7 +435,7 @@ public function updateApp(string $appId): JSONResponse {
* Unregister ExApp, remove container by default
*/
#[PasswordConfirmationRequired]
public function uninstallApp(string $appId, bool $removeContainer = true, bool $removeData = false): JSONResponse {
public function uninstallApp(string $appId, bool $removeContainer = true, bool $removeData = false, string $imageCleanup = 'grace'): JSONResponse {
$exApp = $this->exAppService->getExApp($appId);
if ($exApp) {
if ($exApp->getEnabled()) {
Expand All @@ -442,6 +445,8 @@ public function uninstallApp(string $appId, bool $removeContainer = true, bool $
$daemonConfig = $this->daemonConfigService->getDaemonConfigByName($exApp->getDaemonConfigName());
if ($daemonConfig->getAcceptsDeployId() === $this->dockerActions->getAcceptsDeployId()) {
$this->dockerActions->initGuzzleClient($daemonConfig);
// Capture before removal so we still have the ref to clean up afterwards.
$capturedImageRef = $this->imageCleanupService->captureImageRef($daemonConfig, $appId);
if (boolval($exApp->getDeployConfig()['harp'] ?? false)) {
$this->dockerActions->removeExApp($this->dockerActions->buildDockerUrl($daemonConfig), $exApp->getAppid(), removeData: $removeData);
} else {
Expand All @@ -452,6 +457,8 @@ public function uninstallApp(string $appId, bool $removeContainer = true, bool $
}
}
}
$choice = ImageCleanupChoice::tryFrom($imageCleanup) ?? ImageCleanupChoice::GRACE;
$this->imageCleanupService->scheduleCleanup($capturedImageRef, $exApp, $daemonConfig, $choice);
}
$this->exAppService->unregisterExApp($appId);
}
Expand Down
Loading
Loading