Skip to content

Commit c8f9830

Browse files
matejmatuskamatejmatuska
committed
repositoriesmapping: Add validation for target pesids existing
1 parent ab3024f commit c8f9830

3 files changed

Lines changed: 217 additions & 12 deletions

File tree

‎repos/system_upgrade/common/actors/repositoriesmapping/libraries/repositoriesmapping.py‎

Lines changed: 51 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,45 @@ def get_mappings(self, src_major_version, dst_major_version, dst_distro):
109109

110110
return map_list
111111

112+
def validate_target_pesids_for_ipu(self, target_distro, arch):
113+
"""
114+
Validate that all PESIDs are associated with a repository
115+
116+
If a PESID is mapped as a target PESID in the mapping section, check
117+
that the PESID is associated with some repository for the given IPU
118+
i.e. a repository matching the given source and target distro and
119+
architecture and the target major version exists in the repositories
120+
section.
121+
122+
The check doesn't really make sense for a PESID mapped as a source,
123+
because it not being present in repositories section for the given IPU
124+
can be intentional and is valid.
125+
"""
126+
# TODO maybe take into account just the current source/target version?
127+
# But we can check them without further input unlike the distro and
128+
# arch.
129+
repo_lookup = set()
130+
for repo in self.repositories:
131+
repo_lookup.add((repo.pesid, repo.major_version, repo.distro, repo.arch))
132+
133+
for upg_path, mappings in self.mapping.items():
134+
target_ver = upg_path.split(':')[1]
135+
136+
for _source_pesid, target_pesids_by_distro in mappings.items():
137+
default = target_pesids_by_distro['default']
138+
pesids_for_distro = target_pesids_by_distro.get(target_distro, default)
139+
140+
for pesid in pesids_for_distro:
141+
if (pesid, target_ver, target_distro, arch) not in repo_lookup:
142+
raise ValueError(
143+
"Target PESID {} does not have any associated repository"
144+
" for major version {} and distro {}.".format(
145+
pesid,
146+
target_ver,
147+
target_distro,
148+
)
149+
)
150+
112151
@staticmethod
113152
def load_from_dict(data):
114153
if data['version_format'] != RepoMapData.VERSION_FORMAT:
@@ -157,12 +196,14 @@ def load_from_dict(data):
157196
)
158197

159198
for pesid in [entry['source']] + [id for ids in entry['target'].values() for id in ids]:
160-
# FIXME: this check isn't complete since the distro field was
161-
# introduced for repositories, because the PESID might be
162-
# associated with just repositories for a particular
163-
# distro(s) and therefore might not be for the source or
164-
# target distro.
165-
# However this is better than nothing.
199+
# NOTE: this check isn't complete for target repositories,
200+
# since the distro field was introduced for repositories,
201+
# because the PESID might be associated with just
202+
# repositories for a particular distro(s) and therefore
203+
# might not be for the target distro.
204+
# This is therefore a very simple check, for a proper one
205+
# the :py:func:`validate_target_pesids_for_ipu` can be
206+
# used.
166207
if pesid not in existing_pesids:
167208
raise ValueError(
168209
'The {} pesid is not related to any repository.'.format(pesid)
@@ -228,6 +269,9 @@ def scan_repositories(read_repofile_func=_read_repofile):
228269

229270
src_distro, dst_distro = get_source_distro_id(), get_target_distro_id()
230271
src_ver, dst_ver = get_source_major_version(), get_target_major_version()
272+
arch = api.current_actor().configuration.architecture
273+
274+
repomap_data.validate_target_pesids_for_ipu(dst_distro, arch)
231275

232276
mapping = repomap_data.get_mappings(src_ver, dst_ver, dst_distro)
233277
if mapping is None:
@@ -242,7 +286,7 @@ def scan_repositories(read_repofile_func=_read_repofile):
242286
repos = repomap_data.get_repositories(
243287
[src_ver, dst_ver],
244288
[src_distro, dst_distro],
245-
[api.current_actor().configuration.architecture],
289+
[arch],
246290
)
247291

248292
api.produce(RepositoriesMapping(mapping=mapping, repositories=repos))

‎repos/system_upgrade/common/actors/repositoriesmapping/tests/files/repomap_example.json‎

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
"source": "pesid1",
1111
"target": {
1212
"default": ["pesid2", "pesid3"],
13-
"centos": ["pesid6", "pesid7"]
13+
"centos": ["pesid7"]
1414
}
1515
}
1616
]
@@ -22,8 +22,9 @@
2222
{
2323
"source": "pesid3",
2424
"target": {
25-
"default": ["pesid4", "pesid5"],
26-
"almalinux": ["pesid6"]
25+
"default": ["pesid4"],
26+
"centos": ["pesid4"],
27+
"almalinux": ["pesid9"]
2728
}
2829
}
2930
]
@@ -69,6 +70,19 @@
6970
}
7071
]
7172
},
73+
{
74+
"pesid": "pesid2",
75+
"entries": [
76+
{
77+
"major_version": "8",
78+
"repoid": "some-rhel-8-repoid1",
79+
"arch": "x86_64",
80+
"repo_type": "rpm",
81+
"channel": "eus",
82+
"distro": "almalinux"
83+
}
84+
]
85+
},
7286
{
7387
"pesid": "pesid3",
7488
"entries": [
@@ -82,6 +96,19 @@
8296
}
8397
]
8498
},
99+
{
100+
"pesid": "pesid3",
101+
"entries": [
102+
{
103+
"major_version": "8",
104+
"repoid": "some-rhel-8-repoid2",
105+
"arch": "x86_64",
106+
"repo_type": "rpm",
107+
"channel": "eus",
108+
"distro": "almalinux"
109+
}
110+
]
111+
},
85112
{
86113
"pesid": "pesid4",
87114
"entries": [
@@ -95,6 +122,19 @@
95122
}
96123
]
97124
},
125+
{
126+
"pesid": "pesid4",
127+
"entries": [
128+
{
129+
"major_version": "9",
130+
"repoid": "some-rhel-9-repo1",
131+
"arch": "x86_64",
132+
"repo_type": "rpm",
133+
"channel": "eus",
134+
"distro": "centos"
135+
}
136+
]
137+
},
98138
{
99139
"pesid": "pesid5",
100140
"entries": [

‎repos/system_upgrade/common/actors/repositoriesmapping/tests/unit_test_repositoriesmapping.py‎

Lines changed: 123 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,26 @@ def adjust_cwd():
7878
]
7979

8080
ALMA_REPOS = [
81+
PESIDRepositoryEntry(
82+
pesid='pesid2',
83+
major_version='8',
84+
repoid='some-rhel-8-repoid1',
85+
arch='x86_64',
86+
repo_type='rpm',
87+
channel='eus',
88+
rhui='',
89+
distro='almalinux',
90+
),
91+
PESIDRepositoryEntry(
92+
pesid='pesid3',
93+
major_version='8',
94+
repoid='some-rhel-8-repoid2',
95+
arch='x86_64',
96+
repo_type='rpm',
97+
channel='eus',
98+
rhui='',
99+
distro='almalinux',
100+
),
81101
PESIDRepositoryEntry(
82102
pesid='pesid8',
83103
major_version='8',
@@ -95,12 +115,12 @@ def adjust_cwd():
95115
[
96116
('rhel', 'rhel', {'pesid2', 'pesid3'}, RHEL_REPOS),
97117
# has specific mapping
98-
('centos', 'centos', {'pesid6', 'pesid7'}, CENTOS_REPOS),
118+
('centos', 'centos', {'pesid7'}, CENTOS_REPOS),
99119
# no specific mapping, should use default, same as rhel
100120
('almalinux', 'almalinux', {'pesid2', 'pesid3'}, ALMA_REPOS),
101121
# conversions
102122
('centos', 'rhel', {'pesid2', 'pesid3'}, RHEL_REPOS + CENTOS_REPOS),
103-
('rhel', 'centos', {'pesid6', 'pesid7'}, RHEL_REPOS + CENTOS_REPOS),
123+
('rhel', 'centos', {'pesid7'}, RHEL_REPOS + CENTOS_REPOS),
104124
('almalinux', 'rhel', {'pesid2', 'pesid3'}, RHEL_REPOS + ALMA_REPOS),
105125
]
106126
)
@@ -372,3 +392,104 @@ def test_scan_repositories_with_repo_entry_mapping_target_not_a_dict(monkeypatch
372392
"The 'target' of a mapping entry for PESID source_pesid is <class 'list'>, must be a dict"
373393
in error_info.value.message
374394
)
395+
396+
397+
@pytest.mark.parametrize(
398+
"target_repo",
399+
[
400+
# mapped for a different version
401+
{
402+
"pesid": "target_pesid1",
403+
"entries": [
404+
{
405+
"major_version": "7",
406+
"repoid": "some-rhel-9-repo1",
407+
"arch": "x86_64",
408+
"repo_type": "rpm",
409+
"channel": "eus",
410+
"distro": "rhel",
411+
}
412+
],
413+
},
414+
# mapped for a different distro
415+
{
416+
"pesid": "target_pesid2",
417+
"entries": [
418+
{
419+
"major_version": "7",
420+
"repoid": "some-rhel-9-repo1",
421+
"arch": "x86_64",
422+
"repo_type": "rpm",
423+
"channel": "eus",
424+
"distro": "rhel",
425+
}
426+
],
427+
},
428+
# mapped for a different arch
429+
{
430+
"pesid": "target_pesid3",
431+
"entries": [
432+
{
433+
"major_version": "7",
434+
"repoid": "some-rhel-9-repo1",
435+
"arch": "aarch64",
436+
"repo_type": "rpm",
437+
"channel": "eus",
438+
"distro": "rhel",
439+
}
440+
],
441+
},
442+
],
443+
)
444+
def test_scan_repositories_with_non_existing_pesids_mapped(monkeypatch, target_repo):
445+
"""
446+
Tests whether a PESID present in mapping as a target, but not present in
447+
repositories with the given target distro and architecture is detected.
448+
"""
449+
450+
json_data = {
451+
'datetime': '202107141655Z',
452+
'version_format': repositoriesmapping.RepoMapData.VERSION_FORMAT,
453+
'mapping': [
454+
{
455+
'source_major_version': '7',
456+
'target_major_version': '8',
457+
'entries': [
458+
{
459+
'source': 'source_pesid',
460+
'target': {
461+
'default': [target_repo['pesid']]
462+
},
463+
}
464+
],
465+
}
466+
],
467+
'repositories': [
468+
{
469+
'pesid': 'source_pesid',
470+
'entries': [
471+
{
472+
'major_version': '7',
473+
'repoid': 'some-rhel-9-repo1',
474+
'arch': 'x86_64',
475+
"repo_type": "rpm",
476+
"channel": "eus",
477+
"distro": "rhel",
478+
}
479+
],
480+
},
481+
target_repo,
482+
],
483+
}
484+
485+
monkeypatch.setattr(api, 'current_actor', CurrentActorMocked(src_ver='7.9', dst_ver='8.4'))
486+
monkeypatch.setattr(api, 'produce', produce_mocked())
487+
488+
with pytest.raises(StopActorExecutionError) as error_info:
489+
repositoriesmapping.scan_repositories(lambda dummy: json_data)
490+
491+
msg = (
492+
"Target PESID {} does not have any associated repository"
493+
" for major version 8 and distro rhel"
494+
).format(target_repo["pesid"])
495+
assert msg in error_info.value.message

0 commit comments

Comments
 (0)