Skip to content

Commit 3b375c6

Browse files
authored
Merge pull request #45 from NVIDIA/fix/tuning-uninstall-glob-no-match
fix(tuning): tolerate unmatched systemd drop-in glob during uninstall + release 1.1.5
2 parents eac327e + f61e7bf commit 3b375c6

5 files changed

Lines changed: 145 additions & 18 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
# SPDX-License-Identifier: Apache-2.0
3+
#
4+
#
5+
# Licensed under the Apache License, Version 2.0 (the "License");
6+
# you may not use this file except in compliance with the License.
7+
# You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
17+
TEST_MATRIX = ["ubuntu:24.04"]
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
#!/usr/bin/env python3
2+
3+
# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
4+
# SPDX-License-Identifier: Apache-2.0
5+
#
6+
#
7+
# Licensed under the Apache License, Version 2.0 (the "License");
8+
# you may not use this file except in compliance with the License.
9+
# You may obtain a copy of the License at
10+
#
11+
# http://www.apache.org/licenses/LICENSE-2.0
12+
#
13+
# Unless required by applicable law or agreed to in writing, software
14+
# distributed under the License is distributed on an "AS IS" BASIS,
15+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
# See the License for the specific language governing permissions and
17+
# limitations under the License.
18+
19+
"""
20+
Tests for tuning update_settings_uninstall.sh script.
21+
22+
The uninstall script removes drop-in files written by update_settings.sh.
23+
The systemd drop-in path uses a glob (`/etc/systemd/system/*.d/...`) so on a
24+
node where no matching files exist the glob can either expand to nothing or
25+
be passed literally to `rm`. Under `set -e` this used to abort the whole
26+
uninstall. These tests guard the `rm -f` form that tolerates both cases.
27+
"""
28+
29+
from tests.helpers.assertions import (
30+
assert_exit_code,
31+
assert_file_exists,
32+
)
33+
from tests.helpers.docker_test import DockerTestRunner
34+
35+
36+
SKYHOOK_RESOURCE_ID = "1_tuning_1.1.4"
37+
PACKAGE_NAME_FROM_RESOURCE_ID = "tuning"
38+
DROP_IN_FILENAME = f"999-{PACKAGE_NAME_FROM_RESOURCE_ID}-tuning.conf"
39+
40+
41+
def test_uninstall_succeeds_when_no_service_drop_in_files_exist(base_image):
42+
"""Regression: uninstall must not fail when the systemd drop-in glob matches nothing.
43+
44+
Previously the script ran
45+
rm /etc/systemd/system/*.d/999-${package_name}-tuning.conf
46+
On a clean node with no matching files, bash leaves the glob literal and
47+
`rm` exits non-zero, which trips `set -e` and aborts the whole uninstall.
48+
The fix is `rm -f`, which silently tolerates missing operands.
49+
"""
50+
runner = DockerTestRunner(package="tuning", base_image=base_image)
51+
try:
52+
result = runner.run_script(
53+
script="update_settings_uninstall.sh",
54+
env_vars={"SKYHOOK_RESOURCE_ID": SKYHOOK_RESOURCE_ID},
55+
)
56+
assert_exit_code(result, 0)
57+
# No literal-glob diagnostic should appear in the output.
58+
assert "No such file or directory" not in result.stdout, result.stdout
59+
assert "*.d/" not in result.stdout, result.stdout
60+
finally:
61+
runner.cleanup()
62+
63+
64+
def test_uninstall_removes_existing_service_drop_in_file(base_image):
65+
"""Happy path: uninstall removes the drop-in file when one is present.
66+
67+
Seeds /etc/systemd/system/containerd.service.d/999-tuning-tuning.conf in
68+
the container before running the uninstall script, then asserts the file
69+
is gone and the script exited cleanly.
70+
"""
71+
runner = DockerTestRunner(package="tuning", base_image=base_image)
72+
try:
73+
result = runner.run_script(
74+
script="update_settings_uninstall.sh",
75+
env_vars={"SKYHOOK_RESOURCE_ID": SKYHOOK_RESOURCE_ID},
76+
)
77+
assert_exit_code(result, 0)
78+
79+
drop_in_dir = "/etc/systemd/system/containerd.service.d"
80+
drop_in_path = f"{drop_in_dir}/{DROP_IN_FILENAME}"
81+
seed = runner.container.exec_run(
82+
["/bin/bash", "-c", f"mkdir -p {drop_in_dir} && echo '[Service]' > {drop_in_path}"],
83+
workdir="/",
84+
)
85+
assert seed.exit_code == 0, seed.output.decode("utf-8", errors="replace")
86+
assert_file_exists(runner, drop_in_path)
87+
88+
uninstall_path = "/skyhook-package/skyhook_dir/update_settings_uninstall.sh"
89+
exec_result = runner.container.exec_run(
90+
["/bin/bash", "-c", f"{uninstall_path} 2>&1"],
91+
workdir="/skyhook-package",
92+
environment={
93+
"SKYHOOK_DIR": "/skyhook-package",
94+
"STEP_ROOT": "/skyhook-package/skyhook_dir",
95+
"SKYHOOK_RESOURCE_ID": SKYHOOK_RESOURCE_ID,
96+
},
97+
)
98+
assert exec_result.exit_code == 0, exec_result.output.decode("utf-8", errors="replace")
99+
assert not runner.file_exists(drop_in_path), (
100+
f"Expected {drop_in_path} to be removed by uninstall, but it still exists"
101+
)
102+
finally:
103+
runner.cleanup()

tuning/CHANGELOG.md

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,79 +2,84 @@
22

33
All notable changes to this package will be documented in this file.
44

5-
## [Unreleased]
5+
## [1.1.5] - 2026-05-20
6+
7+
### Bug Fixes
8+
9+
- *(tuning)* Tolerate unmatched systemd drop-in glob during uninstall
610

711
### New Features
812

9-
- Add check for executable bit and make all existing script have that set
13+
- Add check for executable bit and make all existing script have that set by [@ayuskauskas](https://github.com/ayuskauskas)
1014

1115
### Other Tasks
1216

13-
- Update license headers
17+
- Update license headers by [@lockwobr](https://github.com/lockwobr)
1418
- Merge pull request #19 from NVIDIA/add_validation
1519

16-
feat: add package validation for local and ci and developer docs
20+
feat: add package validation for local and ci and developer docs by [@ayuskauskas](https://github.com/ayuskauskas)
21+
- Update project to follow the template by [@lockwobr](https://github.com/lockwobr)
1722

1823
## [1.1.4] - 2025-06-03
1924

2025
### Bug Fixes
2126

22-
- Shellscript had bad post-interrupt config and tuning had wrong version in config file
27+
- Shellscript had bad post-interrupt config and tuning had wrong version in config file by [@ayuskauskas](https://github.com/ayuskauskas)
2328

2429
### Other Tasks
2530

2631
- Merge pull request #6 from NVIDIA/fix_shellscript
2732

28-
fix: shellscript had bad post-interrupt config and tuning had wrong v…
33+
fix: shellscript had bad post-interrupt config and tuning had wrong v… by [@ayuskauskas](https://github.com/ayuskauskas)
2934

3035
## [1.1.3] - 2025-05-22
3136

3237
### Bug Fixes
3338

34-
- *(tuning)* Ulimit check was broken along with readme example
39+
- *(tuning)* Ulimit check was broken along with readme example by [@ayuskauskas](https://github.com/ayuskauskas)
3540

3641
### Other Tasks
3742

3843
- Merge pull request #5 from NVIDIA/fix_tuning3
3944

40-
fix(tuning): ulimit check was broken along with readme example
45+
fix(tuning): ulimit check was broken along with readme example by [@ayuskauskas](https://github.com/ayuskauskas)
4146

4247
## [1.1.2] - 2025-05-16
4348

4449
### Bug Fixes
4550

46-
- *(tuning)* Use -gt 0 not -eq 0
51+
- *(tuning)* Use -gt 0 not -eq 0 by [@ayuskauskas](https://github.com/ayuskauskas)
4752

4853
### Other Tasks
4954

5055
- Merge pull request #4 from NVIDIA/fix_tuning_2
5156

52-
fix(tuning): use -gt 0 not -eq 0
57+
fix(tuning): use -gt 0 not -eq 0 by [@ayuskauskas](https://github.com/ayuskauskas)
5358

5459
## [1.1.1] - 2025-05-15
5560

5661
### Bug Fixes
5762

58-
- Wrong registry
59-
- *(tuning)* Don't error when no service configmaps are created
63+
- Wrong registry by [@lockwobr](https://github.com/lockwobr)
64+
- *(tuning)* Don't error when no service configmaps are created by [@ayuskauskas](https://github.com/ayuskauskas)
6065

6166
### Other Tasks
6267

6368
- Merge pull request #3 from NVIDIA/fix_tuning
6469

65-
fix(tuning): don't error when no service configmaps are created
70+
fix(tuning): don't error when no service configmaps are created by [@ayuskauskas](https://github.com/ayuskauskas)
6671

6772
## [1.1.0] - 2025-03-07
6873

6974
### New Features
7075

71-
- Initial push
72-
- Update readmes for using ghcr image and update tuning for most recent changes
76+
- Initial push by [@ayuskauskas](https://github.com/ayuskauskas)
77+
- Update readmes for using ghcr image and update tuning for most recent changes by [@ayuskauskas](https://github.com/ayuskauskas)
7378

7479
### Other Tasks
7580

7681
- Merge pull request #1 from NVIDIA/oss_final_check
7782

78-
feat: update readmes for using ghcr image and update tuning for most …
83+
feat: update readmes for using ghcr image and update tuning for most … by [@ayuskauskas](https://github.com/ayuskauskas)
7984

8085
<!-- Generated by git-cliff -->

tuning/config.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"schema_version": "v1",
33
"package_name": "tuning",
4-
"package_version": "1.1.4",
4+
"package_version": "1.1.5",
55
"expected_config_files": [],
66
"modes": {
77
"config": [

tuning/skyhook_dir/update_settings_uninstall.sh

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,9 @@ if [ -f /etc/security/limits.d/999-${package_name}-tuning.conf ]; then
3737
fi
3838

3939
# remove any service drop in files that may have been created
40-
rm /etc/systemd/system/*.d/999-${package_name}-tuning.conf
40+
# Use -f so a literal/unmatched glob (no .d/ directories or no matching file)
41+
# does not trip `set -e` during uninstall on a clean node.
42+
rm -f /etc/systemd/system/*.d/999-${package_name}-tuning.conf
4143

4244
if [ -f /etc/default/grub.d/999-${package_name}-tuning.cfg ]; then
4345
rm /etc/default/grub.d/999-${package_name}-tuning.cfg

0 commit comments

Comments
 (0)