Skip to content

Commit 6a7640e

Browse files
committed
fix: install non-released versions: handle space
1 parent 9bc08fa commit 6a7640e

4 files changed

Lines changed: 245 additions & 21 deletions

File tree

src/DIRAC/FrameworkSystem/Client/SystemAdministratorClientCLI.py

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#!/usr/bin/env python
22
########################################################################
3-
""" System Administrator Client Command Line Interface """
3+
"""System Administrator Client Command Line Interface"""
44

55
import atexit
66
import datetime
@@ -1066,13 +1066,11 @@ def do_update(self, args):
10661066
10671067
update <version>
10681068
"""
1069-
try:
1070-
version = args.split()[0]
1071-
except Exception as x:
1072-
gLogger.notice("ERROR: wrong input:", str(x))
1069+
version = args.strip()
1070+
if not version:
1071+
gLogger.notice("ERROR: no version specified")
10731072
gLogger.notice(self.do_update.__doc__)
10741073
return
1075-
10761074
client = SystemAdministratorClient(self.host, self.port)
10771075
gLogger.notice("Software update can take a while, please wait ...")
10781076
result = client.updateSoftware(version, timeout=600)

src/DIRAC/FrameworkSystem/Service/SystemAdministratorHandler.py

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,33 @@
11
"""SystemAdministrator service is a tool to control and monitor the DIRAC services and agents"""
22

3-
import socket
4-
import os
5-
import re
6-
import time
73
import getpass
84
import importlib
9-
import shutil
5+
import os
106
import platform
11-
import tempfile
7+
import re
8+
import shutil
9+
import socket
1210
import subprocess
11+
import tempfile
12+
import time
1313
from datetime import datetime, timedelta
14-
import requests
1514

1615
import psutil
17-
from packaging.version import Version, InvalidVersion
18-
16+
import requests
1917
from diraccfg import CFG
18+
from packaging.version import InvalidVersion, Version
2019

21-
from DIRAC import S_OK, S_ERROR, gConfig, rootPath, gLogger, convertToPy3VersionNumber
20+
from DIRAC import S_ERROR, S_OK, convertToPy3VersionNumber, gConfig, gLogger, rootPath
21+
from DIRAC.ConfigurationSystem.Client import PathFinder
2222
from DIRAC.Core.DISET.RequestHandler import RequestHandler
23+
from DIRAC.Core.Security.Locations import getHostCertificateAndKeyLocation
24+
from DIRAC.Core.Security.X509Chain import X509Chain # pylint: disable=import-error
2325
from DIRAC.Core.Utilities import Os
2426
from DIRAC.Core.Utilities.Extensions import extensionsByPriority, getExtensionMetadata
2527
from DIRAC.Core.Utilities.File import mkLink
26-
from DIRAC.Core.Utilities.TimeUtilities import fromString, hour, day
2728
from DIRAC.Core.Utilities.Subprocess import shellCall
2829
from DIRAC.Core.Utilities.ThreadScheduler import gThreadScheduler
29-
from DIRAC.Core.Security.Locations import getHostCertificateAndKeyLocation
30-
from DIRAC.Core.Security.X509Chain import X509Chain # pylint: disable=import-error
31-
from DIRAC.ConfigurationSystem.Client import PathFinder
30+
from DIRAC.Core.Utilities.TimeUtilities import day, fromString, hour
3231
from DIRAC.FrameworkSystem.Client.ComponentInstaller import gComponentInstaller
3332
from DIRAC.FrameworkSystem.Client.ComponentMonitoringClient import ComponentMonitoringClient
3433

@@ -263,6 +262,12 @@ def export_updateSoftware(self, version):
263262
- a git tag/branch like "DIRAC[server] @ git+https://github.com/fstagni/DIRAC.git@test_branch"
264263
"""
265264
# Validate and normalise the requested version
265+
# Strip surrounding whitespace and collapse any internal spaces around "@"
266+
# so that both "DIRAC[server] @ git+https://..." and
267+
# "DIRAC[server]@git+https://..." are accepted.
268+
version = version.strip()
269+
if not version:
270+
return S_ERROR("No version specified")
266271
primaryExtension = None
267272
if "==" in version:
268273
primaryExtension, version = version.split("==")
@@ -311,7 +316,7 @@ def export_updateSoftware(self, version):
311316
installer.flush()
312317
self.log.info("Downloaded DIRACOS installer to", installer.name)
313318

314-
directory = version if released_version else version.split("@")[1].split("#")[0]
319+
directory = version if released_version else version.split("@", 1)[1].strip().split("#")[0]
315320
newProPrefix = os.path.join(
316321
rootPath,
317322
"versions",

src/DIRAC/FrameworkSystem/Service/test/__init__.py

Whitespace-only changes.
Lines changed: 221 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,221 @@
1+
"""Unit tests for SystemAdministratorHandler version normalisation logic
2+
and the SystemAdministratorClientCLI do_update input validation.
3+
"""
4+
5+
import pytest
6+
from unittest.mock import MagicMock, patch
7+
8+
9+
# ---------------------------------------------------------------------------
10+
# Helpers – replicate the version-normalisation logic from
11+
# SystemAdministratorHandler.export_updateSoftware so we can test it
12+
# without standing up a full DIRAC service.
13+
# ---------------------------------------------------------------------------
14+
15+
16+
def _normalise_version(version):
17+
"""Mirror the normalisation applied at the top of export_updateSoftware.
18+
19+
Returns the normalised version string, or raises ValueError if invalid.
20+
Mirrors the side-effects relevant to the directory-name derivation and
21+
the pip command construction.
22+
"""
23+
from packaging.version import Version, InvalidVersion
24+
25+
version = version.strip()
26+
if not version:
27+
raise ValueError("No version specified")
28+
29+
released_version = True
30+
isPrerelease = False
31+
32+
if version.lower() in ["integration", "devel", "master", "main"]:
33+
released_version = False
34+
version = "DIRAC[server] @ git+https://github.com/DIRACGrid/DIRAC.git@integration"
35+
36+
if released_version:
37+
try:
38+
parsed = Version(version)
39+
isPrerelease = parsed.is_prerelease
40+
version = f"v{parsed}"
41+
except InvalidVersion:
42+
if "https://" in version:
43+
released_version = False
44+
else:
45+
raise ValueError(f"Invalid version passed {version!r}")
46+
47+
return version, released_version, isPrerelease
48+
49+
50+
def _directory_from_version(version, released_version):
51+
"""Mirror the directory-name derivation in export_updateSoftware.
52+
53+
Split on the *first* "@" only (the pip package @ URL separator), strip
54+
whitespace, then drop any "#egg=..." fragment.
55+
"""
56+
if released_version:
57+
return version
58+
return version.split("@", 1)[1].strip().split("#")[0]
59+
60+
61+
# ---------------------------------------------------------------------------
62+
# Tests: version normalisation
63+
# ---------------------------------------------------------------------------
64+
65+
66+
class TestNormaliseVersion:
67+
def test_empty_string_raises(self):
68+
with pytest.raises(ValueError, match="No version specified"):
69+
_normalise_version("")
70+
71+
def test_whitespace_only_raises(self):
72+
with pytest.raises(ValueError, match="No version specified"):
73+
_normalise_version(" ")
74+
75+
def test_released_version(self):
76+
version, released, pre = _normalise_version("9.0.18")
77+
assert released is True
78+
assert pre is False
79+
assert version == "v9.0.18"
80+
81+
def test_released_prerelease_version(self):
82+
version, released, pre = _normalise_version("9.0.18a1")
83+
assert released is True
84+
assert pre is True
85+
assert version == "v9.0.18a1"
86+
87+
@pytest.mark.parametrize("keyword", ["integration", "devel", "master", "main"])
88+
def test_special_keywords(self, keyword):
89+
version, released, pre = _normalise_version(keyword)
90+
assert released is False
91+
assert "DIRACGrid/DIRAC" in version
92+
assert "@integration" in version
93+
94+
def test_git_url_without_spaces(self):
95+
raw = "DIRAC[server]@git+https://github.com/fstagni/DIRAC.git@test_branch"
96+
version, released, pre = _normalise_version(raw)
97+
assert released is False
98+
assert version == raw
99+
100+
def test_git_url_with_spaces_around_at(self):
101+
"""The CLI now sends the raw user input; leading/trailing spaces must be stripped."""
102+
raw = " DIRAC[server] @ git+https://github.com/fstagni/DIRAC.git@test_branch "
103+
version, released, pre = _normalise_version(raw)
104+
assert released is False
105+
# Internal spaces around "@" are preserved (pip accepts them)
106+
assert version.strip() == raw.strip()
107+
108+
def test_invalid_version_no_url_raises(self):
109+
with pytest.raises(ValueError, match="Invalid version passed"):
110+
_normalise_version("not-a-valid-version")
111+
112+
113+
# ---------------------------------------------------------------------------
114+
# Tests: directory derivation from version string
115+
# ---------------------------------------------------------------------------
116+
117+
118+
class TestDirectoryFromVersion:
119+
def test_released_version_uses_version_directly(self):
120+
d = _directory_from_version("v9.0.18", released_version=True)
121+
assert d == "v9.0.18"
122+
123+
def test_git_url_without_spaces(self):
124+
"""No spaces around '@' separator — branch name must be included."""
125+
version = "DIRAC[server]@git+https://github.com/fstagni/DIRAC.git@test_branch"
126+
d = _directory_from_version(version, released_version=False)
127+
# Split on first "@" → "git+https://github.com/fstagni/DIRAC.git@test_branch"
128+
assert d == "git+https://github.com/fstagni/DIRAC.git@test_branch"
129+
130+
def test_git_url_with_spaces_around_at_separator(self):
131+
"""Spaces around the pip '@' separator must be stripped; branch part kept."""
132+
# Simulate what the handler receives after version.strip()
133+
version = "DIRAC[server] @ git+https://github.com/fstagni/DIRAC.git@test_branch"
134+
d = _directory_from_version(version, released_version=False)
135+
assert d == "git+https://github.com/fstagni/DIRAC.git@test_branch"
136+
137+
def test_git_url_with_hash_fragment(self):
138+
"""#egg= fragment must be stripped from directory name."""
139+
version = "DIRAC[server]@git+https://github.com/DIRACGrid/DIRAC.git@integration#egg=DIRAC"
140+
d = _directory_from_version(version, released_version=False)
141+
assert d == "git+https://github.com/DIRACGrid/DIRAC.git@integration"
142+
143+
144+
# ---------------------------------------------------------------------------
145+
# Tests: CLI do_update input validation
146+
# ---------------------------------------------------------------------------
147+
148+
149+
class TestDoUpdate:
150+
"""Test SystemAdministratorClientCLI.do_update input validation."""
151+
152+
def _make_cli(self):
153+
from DIRAC.FrameworkSystem.Client.SystemAdministratorClientCLI import (
154+
SystemAdministratorClientCLI,
155+
)
156+
157+
with patch("DIRAC.FrameworkSystem.Client.SystemAdministratorClientCLI.SystemAdministratorClient"):
158+
cli = SystemAdministratorClientCLI.__new__(SystemAdministratorClientCLI)
159+
cli.host = "localhost"
160+
cli.port = 9162
161+
return cli
162+
163+
def test_empty_args_prints_usage_and_returns(self):
164+
cli = self._make_cli()
165+
with (
166+
patch(
167+
"DIRAC.FrameworkSystem.Client.SystemAdministratorClientCLI.SystemAdministratorClient"
168+
) as mock_client_cls,
169+
patch("DIRAC.FrameworkSystem.Client.SystemAdministratorClientCLI.gLogger") as mock_logger,
170+
):
171+
cli.do_update("")
172+
# Client must NOT be contacted
173+
mock_client_cls.assert_not_called()
174+
# Usage should be printed
175+
assert mock_logger.notice.called
176+
177+
def test_whitespace_only_args_prints_usage_and_returns(self):
178+
cli = self._make_cli()
179+
with (
180+
patch(
181+
"DIRAC.FrameworkSystem.Client.SystemAdministratorClientCLI.SystemAdministratorClient"
182+
) as mock_client_cls,
183+
patch("DIRAC.FrameworkSystem.Client.SystemAdministratorClientCLI.gLogger"),
184+
):
185+
cli.do_update(" ")
186+
mock_client_cls.assert_not_called()
187+
188+
def test_valid_version_calls_client(self):
189+
cli = self._make_cli()
190+
with (
191+
patch(
192+
"DIRAC.FrameworkSystem.Client.SystemAdministratorClientCLI.SystemAdministratorClient"
193+
) as mock_client_cls,
194+
patch("DIRAC.FrameworkSystem.Client.SystemAdministratorClientCLI.gLogger"),
195+
):
196+
mock_instance = MagicMock()
197+
mock_instance.updateSoftware.return_value = {"OK": True, "Value": None}
198+
mock_client_cls.return_value = mock_instance
199+
200+
cli.do_update("9.0.18")
201+
202+
mock_client_cls.assert_called_once_with(cli.host, cli.port)
203+
mock_instance.updateSoftware.assert_called_once_with("9.0.18", timeout=600)
204+
205+
def test_git_url_with_spaces_passes_stripped_version(self):
206+
"""Spaces are stripped from the outer edges but the version body is preserved."""
207+
cli = self._make_cli()
208+
with (
209+
patch(
210+
"DIRAC.FrameworkSystem.Client.SystemAdministratorClientCLI.SystemAdministratorClient"
211+
) as mock_client_cls,
212+
patch("DIRAC.FrameworkSystem.Client.SystemAdministratorClientCLI.gLogger"),
213+
):
214+
mock_instance = MagicMock()
215+
mock_instance.updateSoftware.return_value = {"OK": True, "Value": None}
216+
mock_client_cls.return_value = mock_instance
217+
218+
raw = "DIRAC[server] @ git+https://github.com/fstagni/DIRAC.git@test_branch"
219+
cli.do_update(raw)
220+
221+
mock_instance.updateSoftware.assert_called_once_with(raw, timeout=600)

0 commit comments

Comments
 (0)