From c6c95ac0633770382e51c58436dc98745a14c842 Mon Sep 17 00:00:00 2001 From: Lukas Peter Aldershaab Date: Fri, 8 Mar 2024 13:00:57 +0100 Subject: [PATCH 1/3] Improve resilience of service_dependency and service resources --- pagerduty/resource_pagerduty_service.go | 4 +- .../resource_pagerduty_service_dependency.go | 7 +++ ...ource_pagerduty_service_dependency_test.go | 63 ++++++++++++++++++- 3 files changed, 71 insertions(+), 3 deletions(-) diff --git a/pagerduty/resource_pagerduty_service.go b/pagerduty/resource_pagerduty_service.go index 41b51a073..99de62f13 100644 --- a/pagerduty/resource_pagerduty_service.go +++ b/pagerduty/resource_pagerduty_service.go @@ -535,7 +535,7 @@ func resourcePagerDutyServiceUpdate(d *schema.ResourceData, meta interface{}) er updatedService, _, err := client.Services.Update(d.Id(), service) if err != nil { - return err + return handleNotFoundError(err, d) } return flattenService(d, updatedService) @@ -550,7 +550,7 @@ func resourcePagerDutyServiceDelete(d *schema.ResourceData, meta interface{}) er log.Printf("[INFO] Deleting PagerDuty service %s", d.Id()) if _, err := client.Services.Delete(d.Id()); err != nil { - return err + return handleNotFoundError(err, d) } d.SetId("") diff --git a/pagerduty/resource_pagerduty_service_dependency.go b/pagerduty/resource_pagerduty_service_dependency.go index 9a6abbbdf..353bd184c 100644 --- a/pagerduty/resource_pagerduty_service_dependency.go +++ b/pagerduty/resource_pagerduty_service_dependency.go @@ -200,6 +200,10 @@ func resourcePagerDutyServiceDependencyDisassociate(ctx context.Context, d *sche return retry.NonRetryableError(err) } + if err = handleNotFoundError(err, d); err == nil { + return nil + } + // Delaying retry by 30s as recommended by PagerDuty // https://developer.pagerduty.com/docs/rest-api-v2/rate-limiting/#what-are-possible-workarounds-to-the-events-api-rate-limit time.Sleep(30 * time.Second) @@ -323,6 +327,9 @@ func findDependencySetState(ctx context.Context, depID, serviceID, serviceType s if isErrCode(err, http.StatusBadRequest) { return retry.NonRetryableError(err) } + if err = handleNotFoundError(err, d); err == nil { + return nil + } // Delaying retry by 30s as recommended by PagerDuty // https://developer.pagerduty.com/docs/rest-api-v2/rate-limiting/#what-are-possible-workarounds-to-the-events-api-rate-limit diff --git a/pagerduty/resource_pagerduty_service_dependency_test.go b/pagerduty/resource_pagerduty_service_dependency_test.go index f31cae7a1..ef8d525ee 100644 --- a/pagerduty/resource_pagerduty_service_dependency_test.go +++ b/pagerduty/resource_pagerduty_service_dependency_test.go @@ -44,6 +44,15 @@ func TestAccPagerDutyBusinessServiceDependency_Basic(t *testing.T) { ), ExpectNonEmptyPlan: true, }, + // Validating that externally removed dependent service are + // detected and gracefully handled + { + Config: testAccCheckPagerDutyBusinessServiceDependencyConfig(service, businessService, username, email, escalationPolicy), + Check: resource.ComposeTestCheckFunc( + testAccExternallyDestroyedDependentService("pagerduty_service_dependency.bar", "pagerduty_service.dependBar", "pagerduty_service.supportBar"), + ), + ExpectNonEmptyPlan: true, + }, }, }) } @@ -326,13 +335,56 @@ func testAccExternallyDestroyServiceDependency(resName, depName, suppName string return nil } } +func testAccExternallyDestroyedDependentService(resName, depName, suppName string) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[resName] + if !ok { + return fmt.Errorf("Not found: %s", resName) + } + if rs.Primary.ID == "" { + return fmt.Errorf("No Service Dependency ID is set for %q", resName) + } + + dep, ok := s.RootModule().Resources[depName] + if !ok { + return fmt.Errorf("Not found: %s", depName) + } + if dep.Primary.ID == "" { + return fmt.Errorf("No Dependent Business Service ID is set for %q", depName) + } + depServiceType := dep.Primary.Attributes["type"] + + supp, ok := s.RootModule().Resources[suppName] + if !ok { + return fmt.Errorf("Not found: %s", suppName) + } + if supp.Primary.ID == "" { + return fmt.Errorf("No Supporting Service ID is set for %q", suppName) + } + + client, _ := testAccProvider.Meta().(*Config).Client() + if depServiceType == "business_service" { + _, err := client.BusinessServices.Delete(dep.Primary.ID) + if err != nil { + return err + } + } else { + _, err := client.Services.Delete(dep.Primary.ID) + if err != nil { + return err + } + } + + return nil + } +} // Testing Technical Service Dependencies func TestAccPagerDutyTechnicalServiceDependency_Basic(t *testing.T) { dependentService := fmt.Sprintf("tf-%s", acctest.RandString(5)) supportingService := fmt.Sprintf("tf-%s", acctest.RandString(5)) username := fmt.Sprintf("tf-%s", acctest.RandString(5)) - email := fmt.Sprintf("%s@foo.test", username) + email := fmt.Sprintf("%s@lego.com", username) escalationPolicy := fmt.Sprintf("tf-%s", acctest.RandString(5)) resource.Test(t, resource.TestCase{ @@ -361,6 +413,15 @@ func TestAccPagerDutyTechnicalServiceDependency_Basic(t *testing.T) { ), ExpectNonEmptyPlan: true, }, + // Validating that externally removed dependent service are + // detected and gracefully handled + { + Config: testAccCheckPagerDutyTechnicalServiceDependencyConfig(dependentService, supportingService, username, email, escalationPolicy), + Check: resource.ComposeTestCheckFunc( + testAccExternallyDestroyedDependentService("pagerduty_service_dependency.bar", "pagerduty_service.dependBar", "pagerduty_service.supportBar"), + ), + ExpectNonEmptyPlan: true, + }, }, }) } From 8a8e4917a795af4c69a280a4e734b1e88a386ff5 Mon Sep 17 00:00:00 2001 From: Lukas Peter Aldershaab Date: Fri, 8 Mar 2024 14:34:38 +0100 Subject: [PATCH 2/3] remove lego domain and fix business services --- pagerduty/resource_pagerduty_business_service.go | 6 +++++- pagerduty/resource_pagerduty_service_dependency.go | 4 ++++ pagerduty/resource_pagerduty_service_dependency_test.go | 4 ++-- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/pagerduty/resource_pagerduty_business_service.go b/pagerduty/resource_pagerduty_business_service.go index 358403634..1480f91ba 100644 --- a/pagerduty/resource_pagerduty_business_service.go +++ b/pagerduty/resource_pagerduty_business_service.go @@ -135,6 +135,10 @@ func resourcePagerDutyBusinessServiceRead(d *schema.ResourceData, meta interface return retry.NonRetryableError(err) } + if err := handleNotFoundError(err, d); err != nil { + return retry.RetryableError(err) + } + return retry.RetryableError(err) } else if businessService != nil { d.Set("name", businessService.Name) @@ -190,7 +194,7 @@ func resourcePagerDutyBusinessServiceDelete(d *schema.ResourceData, meta interfa log.Printf("[INFO] Deleting PagerDuty business service %s", d.Id()) if _, err := client.BusinessServices.Delete(d.Id()); err != nil { - return err + return handleNotFoundError(err, d) } d.SetId("") diff --git a/pagerduty/resource_pagerduty_service_dependency.go b/pagerduty/resource_pagerduty_service_dependency.go index 353bd184c..d48a309fa 100644 --- a/pagerduty/resource_pagerduty_service_dependency.go +++ b/pagerduty/resource_pagerduty_service_dependency.go @@ -249,6 +249,10 @@ func resourcePagerDutyServiceDependencyDisassociate(ctx context.Context, d *sche return retry.NonRetryableError(err) } + if err = handleNotFoundError(err, d); err == nil { + return nil + } + // Delaying retry by 30s as recommended by PagerDuty // https://developer.pagerduty.com/docs/rest-api-v2/rate-limiting/#what-are-possible-workarounds-to-the-events-api-rate-limit time.Sleep(30 * time.Second) diff --git a/pagerduty/resource_pagerduty_service_dependency_test.go b/pagerduty/resource_pagerduty_service_dependency_test.go index ef8d525ee..a4f0edbd8 100644 --- a/pagerduty/resource_pagerduty_service_dependency_test.go +++ b/pagerduty/resource_pagerduty_service_dependency_test.go @@ -49,7 +49,7 @@ func TestAccPagerDutyBusinessServiceDependency_Basic(t *testing.T) { { Config: testAccCheckPagerDutyBusinessServiceDependencyConfig(service, businessService, username, email, escalationPolicy), Check: resource.ComposeTestCheckFunc( - testAccExternallyDestroyedDependentService("pagerduty_service_dependency.bar", "pagerduty_service.dependBar", "pagerduty_service.supportBar"), + testAccExternallyDestroyedDependentService("pagerduty_service_dependency.foo", "pagerduty_business_service.foo", "pagerduty_service.foo"), ), ExpectNonEmptyPlan: true, }, @@ -384,7 +384,7 @@ func TestAccPagerDutyTechnicalServiceDependency_Basic(t *testing.T) { dependentService := fmt.Sprintf("tf-%s", acctest.RandString(5)) supportingService := fmt.Sprintf("tf-%s", acctest.RandString(5)) username := fmt.Sprintf("tf-%s", acctest.RandString(5)) - email := fmt.Sprintf("%s@lego.com", username) + email := fmt.Sprintf("%s@foo.test", username) escalationPolicy := fmt.Sprintf("tf-%s", acctest.RandString(5)) resource.Test(t, resource.TestCase{ From 6edbea9ff67a2e4ae5199b5837a31ccbd40e80f6 Mon Sep 17 00:00:00 2001 From: Lukas Peter Aldershaab Date: Mon, 18 Mar 2024 17:40:58 +0100 Subject: [PATCH 3/3] Don't retry business service reads on 404 --- pagerduty/resource_pagerduty_business_service.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pagerduty/resource_pagerduty_business_service.go b/pagerduty/resource_pagerduty_business_service.go index 1480f91ba..1c6dd4397 100644 --- a/pagerduty/resource_pagerduty_business_service.go +++ b/pagerduty/resource_pagerduty_business_service.go @@ -135,8 +135,8 @@ func resourcePagerDutyBusinessServiceRead(d *schema.ResourceData, meta interface return retry.NonRetryableError(err) } - if err := handleNotFoundError(err, d); err != nil { - return retry.RetryableError(err) + if err := handleNotFoundError(err, d); err == nil { + return nil } return retry.RetryableError(err)