Skip to content

Commit cfea023

Browse files
author
ikemilian-lewis
committed
fix: small issues
1 parent 11a0e57 commit cfea023

7 files changed

Lines changed: 80 additions & 63 deletions

File tree

src/orb/providers/azure/cli/azure_cli_spec.py

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -116,14 +116,10 @@ def validate_add(self, args: argparse.Namespace) -> list[str]:
116116

117117
def generate_name(self, args: argparse.Namespace) -> str:
118118
"""Generate a provider instance name from Azure subscription and location."""
119-
try:
120-
subscription_id = self._arg(args, "azure_subscription_id") or "default"
121-
location = self._arg(args, "azure_location") or "eastus2"
122-
sanitized_subscription = re.sub(r"[^a-zA-Z0-9\-_]", "-", subscription_id)
123-
return f"azure_{sanitized_subscription}_{location}"
124-
except Exception:
125-
pass
126-
return "azure_default"
119+
subscription_id = self._arg(args, "azure_subscription_id") or "default"
120+
location = self._arg(args, "azure_location") or "eastus2"
121+
sanitized_subscription = re.sub(r"[^a-zA-Z0-9\-_]", "-", subscription_id)
122+
return f"azure_{sanitized_subscription}_{location}"
127123

128124
def format_display(self, config: dict[str, Any]) -> list[tuple[str, str]]:
129125
"""Return (label, value) pairs for display."""

src/orb/providers/azure/configuration/template_extension.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,9 +194,13 @@ def to_template_defaults(self) -> dict[str, Any]:
194194
defaults["placement_regions"] = self.placement_regions
195195
if self.placement_zones:
196196
defaults["placement_zones"] = self.placement_zones
197-
if self.os_disk_size_gb is not None:
198-
defaults["os_disk"] = {
197+
if self.os_disk is not None:
198+
defaults["os_disk"] = self.os_disk.model_dump(mode="json", exclude_none=True)
199+
elif self.os_disk_type is not None or self.os_disk_size_gb is not None:
200+
legacy_os_disk: dict[str, Any] = {
199201
"storage_account_type": self.os_disk_type or "Premium_LRS",
200-
"disk_size_gb": self.os_disk_size_gb,
201202
}
203+
if self.os_disk_size_gb is not None:
204+
legacy_os_disk["disk_size_gb"] = self.os_disk_size_gb
205+
defaults["os_disk"] = legacy_os_disk
202206
return defaults

src/orb/providers/azure/infrastructure/adapters/azure_validation_adapter.py

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -53,19 +53,11 @@ def validate_provider_api(self, api: str) -> bool:
5353
Returns:
5454
True if the API is supported by azure configuration
5555
"""
56-
try:
57-
supported_apis = set(get_supported_apis())
58-
is_valid = api in supported_apis
59-
60-
if not is_valid:
61-
self._logger.debug("azure API validation failed: %s not in %s", api, supported_apis)
62-
63-
return is_valid
64-
65-
except Exception as e:
66-
self._logger.error("Error validating azure provider API %s: %s", api, e)
67-
# Fall back to canonical API names so validation still works without helper wiring.
68-
return api in {"VMSS", "CycleCloud", "SingleVM", "VMSSUniform"}
56+
supported_apis = set(get_supported_apis())
57+
is_valid = api in supported_apis
58+
if not is_valid:
59+
self._logger.debug("azure API validation failed: %s not in %s", api, supported_apis)
60+
return is_valid
6961

7062
def get_supported_provider_apis(self) -> list[str]:
7163
"""
@@ -74,12 +66,7 @@ def get_supported_provider_apis(self) -> list[str]:
7466
Returns:
7567
List of supported azure provider API identifiers
7668
"""
77-
try:
78-
return sorted(get_supported_apis())
79-
except Exception as e:
80-
self._logger.error("Error getting supported azure APIs: %s", e)
81-
# Fallback to hardcoded list for safety
82-
return ["VMSS", "CycleCloud", "SingleVM", "VMSSUniform"]
69+
return sorted(get_supported_apis())
8370

8471
@staticmethod
8572
def get_api_capabilities(api: str) -> dict[str, Any]:

src/orb/providers/azure/infrastructure/handlers/single_vm_handler.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,14 +160,15 @@ def _build_status_result(
160160
"resource_id": str(vm.name),
161161
"cloud_host_id": vm.vm_id or vm.name,
162162
"vm_name": str(vm.name),
163-
"vm_id": str(vm.vm_id),
164163
"resource_group": resource_group,
165164
"location": str(vm.location),
166165
"availability_zone": availability_zone,
167166
"nic_id": network_identity["nic_id"],
168167
"nic_name": network_identity["nic_name"],
169168
"vnet_id": network_identity["vnet_id"],
170169
}
170+
if vm.vm_id is not None:
171+
provider_data["vm_id"] = vm.vm_id
171172
return {
172173
"instance_id": str(vm.name),
173174
"name": str(vm.name),

src/orb/providers/azure/strategy/azure_provider_strategy.py

Lines changed: 2 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -213,11 +213,8 @@ def deployment_service(self) -> Optional[AzureDeploymentService]:
213213
@property
214214
def handlers(self) -> dict[str, AzureHandler]:
215215
"""Get handler mapping, including any explicit test overrides."""
216-
handlers = dict(self._handlers)
217216
handler_factory = self._get_handler_factory()
218-
if handler_factory is None:
219-
return handlers
220-
handlers.update(handler_factory.get_all_handlers())
217+
handlers = handler_factory.get_all_handlers() if handler_factory is not None else {}
221218
handlers.update(self._handlers)
222219
return handlers
223220

@@ -442,10 +439,6 @@ async def execute_operation_async(self, operation: ProviderOperation) -> Provide
442439

443440
def get_capabilities(self) -> ProviderCapabilities:
444441
"""Get Azure provider capabilities."""
445-
# TODO: Keep Azure and AWS capability metadata dynamic together.
446-
# These example regions / instance types and operational heuristics are
447-
# still hard-coded in both providers, we should evaluate if they can be made
448-
# dynamic
449442
return ProviderCapabilities(
450443
provider_type="azure",
451444
supported_operations=[
@@ -457,8 +450,8 @@ def get_capabilities(self) -> ProviderCapabilities:
457450
ProviderOperationType.GET_AVAILABLE_TEMPLATES,
458451
ProviderOperationType.HEALTH_CHECK,
459452
],
453+
supported_apis=get_supported_apis(),
460454
features={
461-
"supported_apis": get_supported_apis(),
462455
"api_capabilities": get_supported_api_capabilities(),
463456
"instance_management": True,
464457
"spot_instances": True,
@@ -470,29 +463,6 @@ def get_capabilities(self) -> ProviderCapabilities:
470463
"key_pairs": True,
471464
"tags_support": True,
472465
"monitoring": True,
473-
"regions": ["eastus", "eastus2", "westus2", "westeurope", "northeurope"],
474-
"instance_types": [
475-
"Standard_D2s_v5",
476-
"Standard_D4s_v5",
477-
"Standard_D8s_v5",
478-
"Standard_E4s_v5",
479-
"Standard_F4s_v2",
480-
],
481-
"max_instances_per_request": 1000,
482-
"supports_windows": False,
483-
"supports_linux": True,
484-
},
485-
limitations={
486-
"max_concurrent_requests": 100,
487-
"rate_limit_per_second": 20,
488-
"max_instance_lifetime_hours": 8760,
489-
"requires_vpc": True,
490-
"requires_key_pair": True,
491-
},
492-
performance_metrics={
493-
"typical_create_time_seconds": 120,
494-
"typical_terminate_time_seconds": 60,
495-
"health_check_timeout_seconds": 15,
496466
},
497467
)
498468

tests/providers/azure/test_azure_config_and_infra.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,31 @@ def test_defaults_include_explicit_vm_size(self):
422422
defaults = ext.to_template_defaults()
423423
assert defaults["vm_size"] == "Standard_B1s"
424424

425+
def test_defaults_include_structured_os_disk(self):
426+
ext = AzureTemplateExtensionConfig(
427+
os_disk={
428+
"storage_account_type": "StandardSSD_LRS",
429+
"caching": "ReadOnly",
430+
"ephemeral_os_disk": True,
431+
}
432+
)
433+
434+
defaults = ext.to_template_defaults()
435+
436+
assert defaults["os_disk"] == {
437+
"storage_account_type": "StandardSSD_LRS",
438+
"caching": "ReadOnly",
439+
"ephemeral_os_disk": True,
440+
"ephemeral_placement": "CacheDisk",
441+
}
442+
443+
def test_defaults_include_legacy_os_disk_type_without_size(self):
444+
ext = AzureTemplateExtensionConfig(os_disk_type="StandardSSD_LRS")
445+
446+
defaults = ext.to_template_defaults()
447+
448+
assert defaults["os_disk"] == {"storage_account_type": "StandardSSD_LRS"}
449+
425450
def test_defaults_preserve_spot_placement_score_fields(self):
426451
ext = AzureTemplateExtensionConfig(
427452
spot_placement_score_enabled=True,

tests/providers/azure/test_single_vm_handler.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@
1414
SingleVMHandler,
1515
_format_azure_error_message,
1616
)
17+
from orb.providers.azure.infrastructure.sdk_shapes import AzureVmRuntimeStatusProtocol
18+
from orb.providers.azure.infrastructure.services.azure_network_identity_resolver import (
19+
AzureNetworkIdentity,
20+
)
1721
from tests.providers.azure.strategy_test_support import (
1822
AsyncPager,
1923
make_azure_template,
@@ -72,6 +76,36 @@ def _make_azure_client() -> MagicMock:
7276
return make_single_vm_azure_client()
7377

7478

79+
def test_status_result_omits_absent_vm_id():
80+
vm = MagicMock(spec=AzureVmRuntimeStatusProtocol)
81+
vm.name = "vm-1"
82+
vm.vm_id = None
83+
vm.location = "eastus2"
84+
vm.zones = None
85+
vm.hardware_profile = None
86+
vm.tags = None
87+
network_identity: AzureNetworkIdentity = {
88+
"private_ip": None,
89+
"public_ip": None,
90+
"subnet_id": None,
91+
"vnet_id": None,
92+
"nic_id": None,
93+
"nic_name": None,
94+
}
95+
96+
result = SingleVMHandler._build_status_result(
97+
vm=vm,
98+
resource_group="rg-1",
99+
status="running",
100+
network_identity=network_identity,
101+
)
102+
103+
provider_data = result.get("provider_data")
104+
assert provider_data is not None
105+
assert provider_data.get("cloud_host_id") == "vm-1"
106+
assert "vm_id" not in provider_data
107+
108+
75109
def test_acquire_hosts_submits_one_batched_deployment_and_returns_submitted_status():
76110
azure_client = _make_azure_client()
77111
logger = MagicMock()

0 commit comments

Comments
 (0)