Skip to content

Commit c5ba205

Browse files
committed
Fix update_skus bug when a single gen is set
This commit fixes a bug on `update_skus` which happens whenever a plan has just a single generation set on SKU. In the previous versions it was looking only for the SKU ID suffix to determine the default and alternate gen, which was enough for all cases with 2 generations defined but would cause a bug (renaming the SKU ID) whenever a single generation was originally set. With this fix it looks not only in the SKU ID suffix but also on SKU image type to determine which generation is the default and which is the alternate to prevent renaming the default. Refers to SPSTRAT-462
1 parent 2b4f034 commit c5ba205

File tree

2 files changed

+61
-9
lines changed

2 files changed

+61
-9
lines changed

cloudpub/ms_azure/utils.py

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ def update_skus(
280280
disk_versions (list)
281281
List of existing DiskVersion in the technical config
282282
generation (str)
283-
The main generation for publishing
283+
The main generation for publishing when there are no old_skus
284284
plan-name (str)
285285
The destination plan name.
286286
old_skus (list, optional)
@@ -306,14 +306,26 @@ def update_skus(
306306
# The alternate plan name ends with the suffix "-genX" and we can't change that once
307307
# the offer is live, otherwise it will raise "BadRequest" with the message:
308308
# "The property 'PlanId' is locked by a previous submission".
309-
default_gen = "V2"
310-
alt_gen = "V1"
311-
for osku in old_skus:
312-
if osku.security_type is not None:
313-
security_type = osku.security_type
314-
if osku.id.endswith("-gen2"): # alternate is gen2 hence V1 is the default.
315-
default_gen = "V1"
316-
alt_gen = "V2"
309+
osku = old_skus[0]
310+
# Get the security type for all gens
311+
if osku.security_type is not None:
312+
security_type = osku.security_type
313+
314+
# Default Gen2 cases
315+
if osku.image_type.endswith("Gen1") and osku.id.endswith("gen1"):
316+
default_gen = "V2"
317+
alt_gen = "V1"
318+
elif osku.image_type.endswith("Gen2") and not osku.id.endswith("gen2"):
319+
default_gen = "V2"
320+
alt_gen = "V1"
321+
322+
# Default Gen1 cases
323+
elif osku.image_type.endswith("Gen1") and not osku.id.endswith("gen1"):
324+
default_gen = "V1"
325+
alt_gen = "V2"
326+
elif osku.image_type.endswith("Gen2") and osku.id.endswith("gen2"):
327+
default_gen = "V1"
328+
alt_gen = "V2"
317329

318330
return _build_skus(
319331
disk_versions,

tests/ms_azure/test_utils.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,46 @@ def test_update_existing_skus_gen1_default(
277277
]
278278
]
279279

280+
@pytest.mark.parametrize("generation", ["V1", "V2"])
281+
def test_update_existing_skus_gen1_single(
282+
self, generation: str, technical_config_obj: VMIPlanTechConfig
283+
) -> None:
284+
skus = [VMISku.from_json({"imageType": "x64Gen1", "skuId": "plan1"})]
285+
technical_config_obj.skus = skus
286+
res = update_skus(
287+
disk_versions=technical_config_obj.disk_versions,
288+
generation=generation,
289+
plan_name="plan1",
290+
old_skus=technical_config_obj.skus,
291+
)
292+
assert res == [
293+
VMISku.from_json(x)
294+
for x in [
295+
{"imageType": "x64Gen1", "skuId": "plan1", "securityType": None},
296+
{"imageType": "x64Gen2", "skuId": "plan1-gen2", "securityType": None},
297+
]
298+
]
299+
300+
@pytest.mark.parametrize("generation", ["V1", "V2"])
301+
def test_update_existing_skus_gen2_single(
302+
self, generation: str, technical_config_obj: VMIPlanTechConfig
303+
) -> None:
304+
skus = [VMISku.from_json({"imageType": "x64Gen2", "skuId": "plan1"})]
305+
technical_config_obj.skus = skus
306+
res = update_skus(
307+
disk_versions=technical_config_obj.disk_versions,
308+
generation=generation,
309+
plan_name="plan1",
310+
old_skus=technical_config_obj.skus,
311+
)
312+
assert res == [
313+
VMISku.from_json(x)
314+
for x in [
315+
{"imageType": "x64Gen2", "skuId": "plan1", "securityType": None},
316+
{"imageType": "x64Gen1", "skuId": "plan1-gen1", "securityType": None},
317+
]
318+
]
319+
280320
def test_create_disk_version_from_scratch(
281321
self,
282322
disk_version_obj: DiskVersion,

0 commit comments

Comments
 (0)