Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cli): do not remove persistent storage by default in ExApp unregister command #381

Merged
merged 5 commits into from
Sep 6, 2024
Merged
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
### Fixed

- TaskProcessing: fixed bug when provider wasn't removed on unregister. #370
- OCC: ExApp unregister command now doesn't remove volume by default. #381

### Removed

Expand Down
4 changes: 2 additions & 2 deletions docs/ManagingExternalApplications.rst
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ Options
Unregister
----------

Command: ``app_api:app:unregister [--keep-data] [--force] [--silent] [--] <appid>``
Command: ``app_api:app:unregister [--rm-data] [--force] [--silent] [--] <appid>``

To remove an ExApp you can use the unregister command.
There are additional options to keep the ExApp persistent storage (data volume).
Expand All @@ -58,7 +58,7 @@ Arguments
Options
*******

* ``--keep-data`` *[optional]* - keep ExApp persistent storage (data volume)
* ``--rm-data`` *[optional]* - remove ExApp persistent storage (data volume)
* ``--force`` *[optional]* - continue removal even if some error occurs.
* ``--silent`` *[optional]* - print a minimum of information, display only some errors, if any.

Expand Down
11 changes: 6 additions & 5 deletions lib/Command/ExApp/Unregister.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,20 @@ protected function configure(): void {
null,
InputOption::VALUE_NONE,
'Continue removal even if errors.');
$this->addOption('keep-data', null, InputOption::VALUE_NONE, 'Keep ExApp data (volume)');
$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->addUsage('test_app');
$this->addUsage('test_app --silent');
$this->addUsage('test_app --keep-data');
$this->addUsage('test_app --silent --force --keep-data');
$this->addUsage('test_app --rm-data');
$this->addUsage('test_app --silent --force --rm-data');
}

protected function execute(InputInterface $input, OutputInterface $output): int {
$appId = $input->getArgument('appid');
$silent = $input->getOption('silent');
$force = $input->getOption('force');
$keep_data = $input->getOption('keep-data');
$rmData = $input->getOption('rm-data');

$exApp = $this->exAppService->getExApp($appId);
if ($exApp === null) {
Expand Down Expand Up @@ -104,7 +105,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
} elseif (!$silent) {
$output->writeln(sprintf('ExApp %s container successfully removed', $appId));
}
if (!$keep_data) {
if ($rmData) {
$volumeName = $this->dockerActions->buildExAppVolumeName($appId);
$removeVolumeResult = $this->dockerActions->removeVolume(
$this->dockerActions->buildDockerUrl($daemonConfig), $volumeName
Expand Down
8 changes: 4 additions & 4 deletions tests/test_occ_commands_docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,14 @@ def deploy_register():
r = run("php occ --no-warnings app_api:app:unregister skeleton".split(), stdout=PIPE)
assert r.returncode
assert r.stdout.decode("UTF-8"), "Output should be non empty"
# testing if "--keep-data" works.
# testing if volume is kept by default
deploy_register()
r = run("php occ --no-warnings app_api:app:unregister skeleton --keep-data".split(), stdout=PIPE, check=True)
r = run("php occ --no-warnings app_api:app:unregister skeleton".split(), stdout=PIPE, check=True)
assert r.stdout.decode("UTF-8"), "Output should be non empty"
run("docker volume inspect nc_app_skeleton_data".split(), check=True)
# test if volume will be removed without "--keep-data"
# test if volume will be removed with "--rm-data"
deploy_register()
run("php occ --no-warnings app_api:app:unregister skeleton".split(), check=True)
run("php occ --no-warnings app_api:app:unregister skeleton --rm-data".split(), check=True)
r = run("docker volume inspect nc_app_skeleton_data".split())
assert r.returncode
# test "--force" option
Expand Down
Loading