From abaa8a42b0616e06d1f7d745aaaac57499fd4971 Mon Sep 17 00:00:00 2001 From: Ramgopal Nagaboina Date: Wed, 2 Sep 2026 17:55:15 -0400 Subject: [PATCH] fix: remove deleted resources from state instead of erroring in Read When a resource is deleted out of band, Read returned the No match found error from GetByID, failing the whole plan instead of letting Terraform recreate it. GetByID reports count 0 for a genuinely missing resource and count -1 for a transport error, so Read now clears the id and returns nil only when count is 0, and still returns the error otherwise. Applied to attach_volume, cluster, loadbalancer, physical_network, pod, secondary_storage, storage_pool, vlan_ip_range and zone. Tests cover both the deleted case (id cleared) and a transport error (id preserved). --- .../resource_cloudstack_attach_volume.go | 6 +- cloudstack/resource_cloudstack_cluster.go | 6 +- .../resource_cloudstack_loadbalancer.go | 6 +- .../resource_cloudstack_physical_network.go | 6 +- cloudstack/resource_cloudstack_pod.go | 6 +- .../resource_cloudstack_secondary_storage.go | 6 +- .../resource_cloudstack_storage_pool.go | 6 +- .../resource_cloudstack_vlan_ip_range.go | 6 +- cloudstack/resource_cloudstack_zone.go | 6 +- .../resource_cloudstack_zone_unit_test.go | 69 +++++++++++++++++++ 10 files changed, 114 insertions(+), 9 deletions(-) create mode 100644 cloudstack/resource_cloudstack_zone_unit_test.go diff --git a/cloudstack/resource_cloudstack_attach_volume.go b/cloudstack/resource_cloudstack_attach_volume.go index 5880c912..fa1d5113 100644 --- a/cloudstack/resource_cloudstack_attach_volume.go +++ b/cloudstack/resource_cloudstack_attach_volume.go @@ -80,8 +80,12 @@ func resourceCloudStackAttachVolumeCreate(d *schema.ResourceData, meta interface func resourceCloudStackAttachVolumeRead(d *schema.ResourceData, meta interface{}) error { cs := meta.(*cloudstack.CloudStackClient) - r, _, err := cs.Volume.GetVolumeByID(d.Id()) + r, count, err := cs.Volume.GetVolumeByID(d.Id()) if err != nil { + if count == 0 { + d.SetId("") + return nil + } return err } diff --git a/cloudstack/resource_cloudstack_cluster.go b/cloudstack/resource_cloudstack_cluster.go index 48375359..a080d66e 100644 --- a/cloudstack/resource_cloudstack_cluster.go +++ b/cloudstack/resource_cloudstack_cluster.go @@ -209,8 +209,12 @@ func resourceCloudStackClusterCreate(d *schema.ResourceData, meta interface{}) e func resourceCloudStackClusterRead(d *schema.ResourceData, meta interface{}) error { cs := meta.(*cloudstack.CloudStackClient) - r, _, err := cs.Cluster.GetClusterByID(d.Id()) + r, count, err := cs.Cluster.GetClusterByID(d.Id()) if err != nil { + if count == 0 { + d.SetId("") + return nil + } return err } diff --git a/cloudstack/resource_cloudstack_loadbalancer.go b/cloudstack/resource_cloudstack_loadbalancer.go index 5ec015d9..8e6f4722 100644 --- a/cloudstack/resource_cloudstack_loadbalancer.go +++ b/cloudstack/resource_cloudstack_loadbalancer.go @@ -151,8 +151,12 @@ func resourceCloudStackLoadBalancerCreate(d *schema.ResourceData, meta interface func resourceCloudStackLoadBalancerRead(d *schema.ResourceData, meta interface{}) error { cs := meta.(*cloudstack.CloudStackClient) - r, _, err := cs.LoadBalancer.GetLoadBalancerByID(d.Id()) + r, count, err := cs.LoadBalancer.GetLoadBalancerByID(d.Id()) if err != nil { + if count == 0 { + d.SetId("") + return nil + } return err } diff --git a/cloudstack/resource_cloudstack_physical_network.go b/cloudstack/resource_cloudstack_physical_network.go index 0bc74286..1e433987 100644 --- a/cloudstack/resource_cloudstack_physical_network.go +++ b/cloudstack/resource_cloudstack_physical_network.go @@ -132,8 +132,12 @@ func resourceCloudStackPhysicalNetworkCreate(d *schema.ResourceData, meta interf func resourceCloudStackPhysicalNetworkRead(d *schema.ResourceData, meta interface{}) error { cs := meta.(*cloudstack.CloudStackClient) - p, _, err := cs.Network.GetPhysicalNetworkByID(d.Id()) + p, count, err := cs.Network.GetPhysicalNetworkByID(d.Id()) if err != nil { + if count == 0 { + d.SetId("") + return nil + } return err } diff --git a/cloudstack/resource_cloudstack_pod.go b/cloudstack/resource_cloudstack_pod.go index efbaddd7..d3a01347 100644 --- a/cloudstack/resource_cloudstack_pod.go +++ b/cloudstack/resource_cloudstack_pod.go @@ -112,8 +112,12 @@ func resourceCloudStackPodCreate(d *schema.ResourceData, meta interface{}) error func resourceCloudStackPodRead(d *schema.ResourceData, meta interface{}) error { cs := meta.(*cloudstack.CloudStackClient) - r, _, err := cs.Pod.GetPodByID(d.Id()) + r, count, err := cs.Pod.GetPodByID(d.Id()) if err != nil { + if count == 0 { + d.SetId("") + return nil + } return err } diff --git a/cloudstack/resource_cloudstack_secondary_storage.go b/cloudstack/resource_cloudstack_secondary_storage.go index ea2d6439..f44c74a9 100644 --- a/cloudstack/resource_cloudstack_secondary_storage.go +++ b/cloudstack/resource_cloudstack_secondary_storage.go @@ -93,8 +93,12 @@ func resourceCloudStackSecondaryStorageCreate(d *schema.ResourceData, meta inter func resourceCloudStackSecondaryStorageRead(d *schema.ResourceData, meta interface{}) error { cs := meta.(*cloudstack.CloudStackClient) - r, _, err := cs.ImageStore.GetImageStoreByID(d.Id()) + r, count, err := cs.ImageStore.GetImageStoreByID(d.Id()) if err != nil { + if count == 0 { + d.SetId("") + return nil + } return err } diff --git a/cloudstack/resource_cloudstack_storage_pool.go b/cloudstack/resource_cloudstack_storage_pool.go index b83e8319..e639c17a 100644 --- a/cloudstack/resource_cloudstack_storage_pool.go +++ b/cloudstack/resource_cloudstack_storage_pool.go @@ -147,8 +147,12 @@ func resourceCloudStackStoragePoolCreate(d *schema.ResourceData, meta interface{ func resourceCloudStackStoragePoolRead(d *schema.ResourceData, meta interface{}) error { cs := meta.(*cloudstack.CloudStackClient) - r, _, err := cs.Pool.GetStoragePoolByID(d.Id()) + r, count, err := cs.Pool.GetStoragePoolByID(d.Id()) if err != nil { + if count == 0 { + d.SetId("") + return nil + } return err } diff --git a/cloudstack/resource_cloudstack_vlan_ip_range.go b/cloudstack/resource_cloudstack_vlan_ip_range.go index ff829fb5..720cf65e 100644 --- a/cloudstack/resource_cloudstack_vlan_ip_range.go +++ b/cloudstack/resource_cloudstack_vlan_ip_range.go @@ -214,8 +214,12 @@ func resourceCloudstackVlanIpRangeCreate(d *schema.ResourceData, meta interface{ func resourceCloudstackVlanIpRangeRead(d *schema.ResourceData, meta interface{}) error { cs := meta.(*cloudstack.CloudStackClient) - r, _, err := cs.VLAN.GetVlanIpRangeByID(d.Id()) + r, count, err := cs.VLAN.GetVlanIpRangeByID(d.Id()) if err != nil { + if count == 0 { + d.SetId("") + return nil + } return err } diff --git a/cloudstack/resource_cloudstack_zone.go b/cloudstack/resource_cloudstack_zone.go index 110f2fbe..10613096 100644 --- a/cloudstack/resource_cloudstack_zone.go +++ b/cloudstack/resource_cloudstack_zone.go @@ -176,8 +176,12 @@ func resourceCloudStackZoneCreate(d *schema.ResourceData, meta interface{}) erro func resourceCloudStackZoneRead(d *schema.ResourceData, meta interface{}) error { cs := meta.(*cloudstack.CloudStackClient) - z, _, err := cs.Zone.GetZoneByID(d.Id()) + z, count, err := cs.Zone.GetZoneByID(d.Id()) if err != nil { + if count == 0 { + d.SetId("") + return nil + } return err } diff --git a/cloudstack/resource_cloudstack_zone_unit_test.go b/cloudstack/resource_cloudstack_zone_unit_test.go new file mode 100644 index 00000000..22afc854 --- /dev/null +++ b/cloudstack/resource_cloudstack_zone_unit_test.go @@ -0,0 +1,69 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package cloudstack + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/apache/cloudstack-go/v2/cloudstack" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +// When a resource has been deleted out of band, GetByID reports count 0 (as +// opposed to a transport error, which reports -1). Read must clear the id and +// return nil so Terraform plans a recreate, instead of returning an error. +func TestZoneReadRemovesDeletedFromState(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"listzonesresponse":{"count":0}}`)) + })) + defer server.Close() + + cs := cloudstack.NewClient(server.URL, "key", "secret", false) + + d := schema.TestResourceDataRaw(t, resourceCloudStackZone().Schema, map[string]interface{}{}) + d.SetId("deleted-zone") + + if err := resourceCloudStackZoneRead(d, cs); err != nil { + t.Fatalf("read of a deleted zone should not error, got: %s", err) + } + if d.Id() != "" { + t.Fatalf("read of a deleted zone should clear the id, got: %q", d.Id()) + } +} + +// A transport or API error (count -1, as opposed to a genuinely absent +// resource) must be returned and must not clear the id, so a temporary outage +// does not drop the resource from state. +func TestZoneReadPreservesStateOnAPIError(t *testing.T) { + cs := cloudstack.NewClient("http://127.0.0.1:1", "key", "secret", false) + + d := schema.TestResourceDataRaw(t, resourceCloudStackZone().Schema, map[string]interface{}{}) + d.SetId("zone-1") + + if err := resourceCloudStackZoneRead(d, cs); err == nil { + t.Fatal("expected an error from the unreachable endpoint, got nil") + } + if d.Id() != "zone-1" { + t.Fatalf("a transport error must not clear the id, got: %q", d.Id()) + } +}