fix(ibm): respect configured zone for custom VPC subnets

VERSION

@@ -1 +1 @@
-1!11.1.8
+1!11.1.9

docs/clouds/ibm.md

@@ -107,6 +107,11 @@ A pre-existent VPC can be set in the config file or be passed as argument to the
 ibm = IBM(vpc="my-custom-vpc", ...)
 ```
 
+For a custom VPC, pycloudlib selects a subnet in the requested zone or creates
+one there if none exists. If no zone is provided, the zone defaults to
+`{region}-1`. Newly created subnets in an existing VPC include the zone in
+their name.
+
 Another possibility is to create a custom VPC on the fly, then one can be created
 and then later used during instance creation.
 

pycloudlib/ibm/.kb/ibm.md

@@ -14,11 +14,12 @@ Read the top-level `.kb/agents.md` file before continuing below.
 
 - **The `ibm-vpc` API version is pinned** in `ibm/cloud.py` (`VpcV1(..., version=...)` plus a `set_service_url` per region), and the SDK upper bound is constrained in `pyproject.toml` (`ibm-vpc >= 0.10, < 0.29.0`). When updating: check the SDK releases and update **both** the version string and the `<` upper bound together.
 - `resource_group_id` and `vpc` are lazy properties (looked up on first access); a missing resource group raises `IBMException`. See `ibm/cloud.py` for the `from_existing`/`from_default` VPC selection.
+- An instance's subnet must be in its zone; when resolving a custom VPC, never fall back to a subnet in another zone.
 - `ibm/_util.py` provides the iteration/wait helpers used across the backend; `ibm/errors.py` defines `IBMException`.
 - mypy: `ibm_vpc.*`/`ibm_cloud_sdk_core.*`/`ibm_platform_services.*` are in `ignore_missing_imports`; `pycloudlib.ibm.instance` has `check_untyped_defs = false` (TODO overrides in `pyproject.toml`).
 
 
 # Architecture
 
 - `IAMAuthenticator(api_key)` authenticates both `VpcV1` and `ResourceManagerV2`. The `VPC` helper (in `ibm/instance.py`) pairs the IBM VPC resource with the resolved resource-group id, region, and zone; floating IPs are selected by `floating_ip_substring` when provided.
-- `clean()` extends `BaseCloud.clean()` to tear down `created_vpcs`/`created_keys`.
+- `clean()` extends `BaseCloud.clean()` to tear down `created_subnets`/`created_vpcs`/`created_keys`.

pycloudlib/ibm/cloud.py

@@ -18,7 +18,7 @@
 from pycloudlib.ibm._util import iter_resources as _iter_resources
 from pycloudlib.ibm._util import wait_until as _wait_until
 from pycloudlib.ibm.errors import IBMException
-from pycloudlib.ibm.instance import VPC, IBMInstance
+from pycloudlib.ibm.instance import VPC, IBMInstance, _Subnet
 from pycloudlib.instance import BaseInstance
 from pycloudlib.util import UBUNTU_RELEASE_VERSION_MAP
 
@@ -56,6 +56,7 @@ def __init__(
             required_values=[resource_group, api_key, region],
         )
         self.created_vpcs: List[VPC] = []
+        self.created_subnets: List[_Subnet] = []
         self.created_keys: List[str] = []
 
         self._resource_group = (
@@ -108,7 +109,13 @@ def vpc(self) -> VPC:
             "zone": self.zone,
         }
         if self._vpc_name is not None:
-            self._vpc = VPC.from_existing(self.key_pair, name=self._vpc_name, **kwargs)
+            self._vpc = VPC.from_existing(
+                self.key_pair,
+                name=self._vpc_name,
+                **kwargs,
+            )
+            if self._vpc.created_subnet is not None:
+                self.created_subnets.append(self._vpc.created_subnet)
         else:
             self._vpc = VPC.from_default(self.key_pair, **kwargs)
 
@@ -244,11 +251,14 @@ def get_or_create_vpc(self, name: str) -> VPC:
             "zone": self.zone,
         }
         try:
-            return VPC.from_existing(*args, **kwargs)
+            vpc = VPC.from_existing(*args, **kwargs)
         except IBMException:
             vpc = VPC.create(*args, **kwargs)
             self.created_vpcs.append(vpc)
             return vpc
+        if vpc.created_subnet is not None:
+            self.created_subnets.append(vpc.created_subnet)
+        return vpc
 
     def launch(
         self,
@@ -409,6 +419,11 @@ def clean(self) -> List[Exception]:
         # Not cleaning up floating ips here because they're 1:1
         # with an instance and get cleaned up by the instance
         exceptions = super().clean()
+        for subnet in self.created_subnets:
+            try:
+                subnet.delete()
+            except Exception as error:
+                exceptions.append(error)
         for vpc in self.created_vpcs:
             try:
                 vpc.delete()

pycloudlib/ibm/instance.py

@@ -78,15 +78,17 @@ def from_default(cls, client: VpcV1, zone: str, vpc_id: str) -> "_Subnet":
         return cls.from_existing(client, f"{zone}-default-subnet", vpc_id)
 
     @classmethod
-    def discover(cls, client: VpcV1, vpc_id: str) -> "_Subnet":
-        """Discover a Subnet within a VPC."""
+    def discover(cls, client: VpcV1, vpc_id: str, zone: str) -> "_Subnet":
+        """Discover a Subnet within a VPC and zone."""
         subnet = _get_first(
             client.list_subnets,
             resource_name="subnets",
-            filter_fn=(lambda subnet: subnet["vpc"]["id"] == vpc_id),
+            filter_fn=(
+                lambda subnet: subnet["vpc"]["id"] == vpc_id and subnet["zone"]["name"] == zone
+            ),
         )
         if subnet is None:
-            raise IBMException(f"No subnet associated to vpc found: {vpc_id}")
+            raise IBMException(f"No subnet associated to vpc found: {vpc_id} in zone {zone}")
         return cls(client, subnet)
 
     @property
@@ -133,13 +135,15 @@ def __init__(
         vpc: dict,
         resource_group_id: str,
         subnet: Optional[_Subnet] = None,
+        created_subnet: Optional[_Subnet] = None,
         **_kwargs,
     ):
         """Init a `VPC`."""
         self._key_pair = key_pair
         self._client = client
         self._vpc = vpc
         self._subnet = subnet
+        self._created_subnet = created_subnet
         self._resource_group_id = resource_group_id
 
     @classmethod
@@ -200,7 +204,7 @@ def from_existing(
     ) -> "VPC":
         """Find a VPC by name.
 
-        Try to discover a Subnet within it or create it if not found.
+        Try to discover a Subnet within it in the requested zone or create it if not found.
         """
         vpc = _get_first(
             client.list_vpcs,
@@ -210,23 +214,26 @@ def from_existing(
         if vpc is None:
             raise IBMException(f"VPC not found: {name}")
 
+        created_subnet = None
         try:
-            subnet = _Subnet.discover(client, vpc_id=vpc["id"])
+            subnet = _Subnet.discover(client, vpc_id=vpc["id"], zone=zone)
         except IBMException:
             subnet = _Subnet.create(
                 client,
-                name=f"{name}-subnet",
+                name=f"{name}-{zone}-subnet",
                 zone=zone,
                 resource_group_id=resource_group_id,
                 vpc_id=vpc["id"],
             )
+            created_subnet = subnet
 
         return cls(
             *args,
             client=client,
             vpc=vpc,
             resource_group_id=resource_group_id,
             subnet=subnet,
+            created_subnet=created_subnet,
             **kwargs,
         )
 
@@ -277,6 +284,11 @@ def subnet_id(self) -> str:
             raise IBMException("No subnet available")
         return self._subnet.id
 
+    @property
+    def created_subnet(self) -> Optional[_Subnet]:
+        """Subnet created in an existing VPC, or None if one was reused."""
+        return self._created_subnet
+
     def delete(self) -> None:
         """Delete VPC.
 

tests/integration_tests/ibm/test_launch.py

@@ -5,6 +5,8 @@
 from google.cloud import compute_v1
 import time
 
+from pycloudlib.ibm._util import iter_resources
+
 
 @pytest.fixture
 def ibm_cloud():
@@ -37,6 +39,50 @@ def manage_ssh_key(ibm: IBM, key_name):
     )
 
 
+def configured_vpc_subnets(ibm_cloud: IBM):
+    """Return subnets for the configured custom VPC."""
+    vpc_name = ibm_cloud.config.get("vpc")
+    if not vpc_name:
+        pytest.skip("requires a custom VPC in the IBM test config")
+
+    vpc = next(
+        (
+            candidate
+            for candidate in iter_resources(
+                ibm_cloud._client.list_vpcs,
+                resource_name="vpcs",
+            )
+            if candidate["name"] == vpc_name
+        ),
+        None,
+    )
+    assert vpc is not None, f"Configured VPC not found: {vpc_name}"
+    return list(
+        iter_resources(
+            ibm_cloud._client.list_subnets,
+            resource_name="subnets",
+            filter_fn=lambda subnet: subnet["vpc"]["id"] == vpc["id"],
+        )
+    )
+
+
+def test_ibm_custom_vpc_selects_subnet_in_zone(ibm_cloud: IBM):
+    """Select a subnet in the cloud's zone, not the first subnet listed."""
+    subnets = configured_vpc_subnets(ibm_cloud)
+    matching_subnet = next(
+        (subnet for subnet in subnets if subnet["zone"]["name"] == ibm_cloud.zone),
+        None,
+    )
+    if matching_subnet is None:
+        pytest.skip("requires an existing matching subnet to avoid creating resources")
+    if subnets[0]["id"] == matching_subnet["id"]:
+        pytest.skip("first listed subnet must be in another zone to reproduce the bug")
+
+    selected_subnet = ibm_cloud._client.get_subnet(ibm_cloud.vpc.subnet_id).get_result()
+
+    assert selected_subnet["id"] == matching_subnet["id"]
+
+
 def test_ibm_launch(ibm_cloud: IBM):
     """
     Test launching an IBM instance.

tests/unit_tests/ibm/test_cloud.py

@@ -5,6 +5,7 @@
 from unittest import mock
 
 import pytest
+from ibm_cloud_sdk_core import ApiException
 
 from pycloudlib.errors import InvalidTagNameError, PycloudlibTimeoutError
 from pycloudlib.ibm.cloud import (
@@ -47,3 +48,77 @@ def test_validate_tag(tag: str, rules_failed: List[str]):
         assert tag in str(exc_info.value)
         for rule in rules_failed:
             assert rule in str(exc_info.value)
+
+
+@pytest.mark.mock_ssh_keys
+class TestIBM:
+    @pytest.fixture
+    def cloud(self):
+        with mock.patch("pycloudlib.ibm.cloud.VpcV1"):
+            cloud = IBM(
+                tag="test-subnet-cleanup",
+                api_key="api-key",
+                resource_group="resource-group",
+                region="us-south",
+                vpc="custom-vpc",
+            )
+        cloud._resource_group_id = "resource-group-id"
+        cloud._client.list_vpcs.return_value.get_result.return_value = {
+            "vpcs": [{"id": "vpc-id", "name": "custom-vpc"}]
+        }
+        return cloud
+
+    @pytest.mark.parametrize("lookup", ["vpc", "get_or_create_vpc"])
+    @pytest.mark.parametrize("matching_subnet", [True, False])
+    def test_clean_custom_vpc_subnets(self, cloud, lookup, matching_subnet):
+        """clean() deletes only a subnet created in an existing VPC."""
+        client = cloud._client
+        client.list_subnets.return_value.get_result.return_value = {
+            "subnets": [
+                {
+                    "id": "existing-subnet",
+                    "vpc": {"id": "vpc-id"},
+                    "zone": {"name": "us-south-1" if matching_subnet else "us-south-2"},
+                }
+            ]
+        }
+        client.create_subnet.return_value.get_result.return_value = {"id": "created-subnet"}
+        client.get_subnet.side_effect = ApiException(404)
+
+        vpc = cloud.vpc if lookup == "vpc" else cloud.get_or_create_vpc("custom-vpc")
+
+        assert vpc.subnet_id == ("existing-subnet" if matching_subnet else "created-subnet")
+        assert cloud.clean() == []
+        assert len(cloud.created_subnets) == (0 if matching_subnet else 1)
+        if matching_subnet:
+            client.create_subnet.assert_not_called()
+            client.delete_subnet.assert_not_called()
+        else:
+            client.delete_subnet.assert_called_once_with("created-subnet")
+        client.delete_vpc.assert_not_called()
+
+    def test_clean_continues_after_subnet_failure(self, cloud):
+        """A failed subnet deletion is reported and the rest of cleanup still runs."""
+        manager = mock.Mock()
+        instance = mock.Mock()
+        subnet = mock.Mock()
+        vpc = mock.Mock()
+        error = RuntimeError("subnet deletion failed")
+        subnet.delete.side_effect = error
+        cloud.created_instances.append(instance)
+        cloud.created_subnets.append(subnet)
+        cloud.created_vpcs.append(vpc)
+        cloud.created_keys.append("key-id")
+        manager.attach_mock(instance, "instance")
+        manager.attach_mock(subnet, "subnet")
+        manager.attach_mock(vpc, "vpc")
+        manager.attach_mock(cloud._client, "client")
+
+        assert cloud.clean() == [error]
+        assert cloud.created_subnets == [subnet]
+        assert manager.mock_calls == [
+            mock.call.instance.delete(),
+            mock.call.subnet.delete(),
+            mock.call.vpc.delete(),
+            mock.call.client.delete_key("key-id"),
+        ]