Skip to content

Commit

Permalink
Merge pull request #681 from imjaroiswebdev/improve-schedule-deletion…
Browse files Browse the repository at this point in the history
…-open-incident-handling

Improve `resource/pagerduty_schedule` open incidents handling on deletion
  • Loading branch information
imjaroiswebdev authored May 3, 2023
2 parents 5c59042 + 43cc0d4 commit ee0f0b4
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 49 deletions.
63 changes: 38 additions & 25 deletions pagerduty/resource_pagerduty_schedule.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,36 +399,47 @@ func resourcePagerDutyScheduleDelete(d *schema.ResourceData, meta interface{}) e
return err
}

// An Schedule with open incidents related can't be remove till those
// incidents have been resolved.
linksToIncidentsOpen, err := listIncidentsOpenedRelatedToSchedule(client, scheduleData, epsAssociatedToSchedule)
if err != nil {
return err
}

if len(linksToIncidentsOpen) > 0 {
var urlLinksMessage string
for _, incident := range linksToIncidentsOpen {
urlLinksMessage = fmt.Sprintf("%s\n%s", urlLinksMessage, incident)
}
return fmt.Errorf("Before Removing Schedule %q You must first resolve the following incidents related with Escalation Policies using this Schedule... %s", scheduleId, urlLinksMessage)
}

log.Printf("[INFO] Deleting PagerDuty schedule: %s", scheduleId)
// Retrying to give other resources (such as escalation policies) to delete
retryErr = resource.Retry(2*time.Minute, func() *resource.RetryError {
if _, err := client.Schedules.Delete(scheduleId); err != nil {
if !isErrCode(err, 400) {
return resource.RetryableError(err)
}
isErrorScheduleUsedByEP := func(e *pagerduty.Error) bool {
return strings.Compare(fmt.Sprintf("%v", e.Errors), "[Schedule can't be deleted if it's being used by escalation policies]") == 0
}
isErrorScheduleWOpenIncidents := func(e *pagerduty.Error) bool {
return strings.Compare(fmt.Sprintf("%v", e.Errors), "[Schedule can't be deleted if it's being used by an escalation policy snapshot with open incidents]") == 0
}

// Handling of specific http 400 errors from API call DELETE /schedules
if e, ok := err.(*pagerduty.Error); !ok || strings.Compare(fmt.Sprintf("%v", e.Errors), "[Schedule can't be deleted if it's being used by escalation policies]") != 0 {
e, ok := err.(*pagerduty.Error)
if !ok || !isErrorScheduleUsedByEP(e) && !isErrorScheduleWOpenIncidents(e) {
log.Printf("[MYDEBUG] isErrorScheduleUsedByEP: %t; isErrorScheduleWOpenIncidents: %t", isErrorScheduleUsedByEP(e), isErrorScheduleWOpenIncidents(e))
return resource.NonRetryableError(err)
}

var workaroundErr error
// An Schedule with open incidents related can't be remove till those
// incidents have been resolved.
linksToIncidentsOpen, workaroundErr := listIncidentsOpenedRelatedToSchedule(client, scheduleData, epsAssociatedToSchedule)
if workaroundErr != nil {
err = fmt.Errorf("%v; %w", err, workaroundErr)
return resource.NonRetryableError(err)
}

if len(linksToIncidentsOpen) > 0 {
var urlLinksMessage string
for _, incident := range linksToIncidentsOpen {
urlLinksMessage = fmt.Sprintf("%s\n%s", urlLinksMessage, incident)
}
return resource.NonRetryableError(fmt.Errorf("Before Removing Schedule %q You must first resolve or reassign the following incidents related with Escalation Policies using this Schedule... %s", scheduleId, urlLinksMessage))
}

// Workaround for Schedule being used by escalation policies error
log.Printf("[INFO] Dissociating Escalation Policies that use the Schedule: %s", scheduleId)
workaroundErr := dissociateScheduleFromEPs(client, scheduleId, epsAssociatedToSchedule)
workaroundErr = dissociateScheduleFromEPs(client, scheduleId, epsAssociatedToSchedule)
if workaroundErr != nil {
err = fmt.Errorf("%v; %w", err, workaroundErr)
}
Expand Down Expand Up @@ -607,18 +618,20 @@ func flattenScheFinalSchedule(finalSche *pagerduty.SubSchedule) []map[string]int

func listIncidentsOpenedRelatedToSchedule(c *pagerduty.Client, schedule *pagerduty.Schedule, epIDs []string) ([]string, error) {
var incidents []*pagerduty.Incident
retryErr := resource.Retry(10*time.Second, func() *resource.RetryError {
retryErr := resource.Retry(30*time.Second, func() *resource.RetryError {
var err error
var userIDs []string
for _, u := range schedule.Users {
userIDs = append(userIDs, u.ID)
}
incidents, err = c.Incidents.ListAll(&pagerduty.ListIncidentsOptions{
options := &pagerduty.ListIncidentsOptions{
DateRange: "all",
Statuses: []string{"triggered", "acknowledged"},
UserIDs: userIDs,
Limit: 100,
})
}
if len(schedule.Users) > 0 {
for _, u := range schedule.Users {
options.UserIDs = append(options.UserIDs, u.ID)
}
}

incidents, err = c.Incidents.ListAll(options)
if err != nil {
time.Sleep(2 * time.Second)
return resource.RetryableError(err)
Expand Down
107 changes: 83 additions & 24 deletions pagerduty/resource_pagerduty_schedule_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ func TestAccPagerDutyScheduleWithTeams_EscalationPolicyDependantWithOpenIncident
},
{
Config: testAccCheckPagerDutyScheduleWithTeamsEscalationPolicyDependantWithOpenIncidentConfigUpdated(username, email, team, escalationPolicy1, service1),
ExpectError: regexp.MustCompile("Before Removing Schedule \".*\" You must first resolve the following incidents related with Escalation Policies using this Schedule"),
ExpectError: regexp.MustCompile("Before Removing Schedule \".*\" You must first resolve or reassign the following incidents related with Escalation Policies using this Schedule"),
},
{
// Extra intermediate step with the original plan for resolving the
Expand Down Expand Up @@ -342,7 +342,8 @@ func TestAccPagerDutySchedule_EscalationPolicyDependantWithOpenIncidents(t *test
service2 := fmt.Sprintf("tf-%s", acctest.RandString(5))
username := fmt.Sprintf("tf-%s", acctest.RandString(5))
email := fmt.Sprintf("%s@foo.test", username)
schedule := fmt.Sprintf("tf-%s", acctest.RandString(5))
schedule1 := fmt.Sprintf("tf-%s", acctest.RandString(5))
schedule2 := fmt.Sprintf("tf-%s", acctest.RandString(5))
location := "America/New_York"
start := timeNowInLoc(location).Add(24 * time.Hour).Round(1 * time.Hour).Format(time.RFC3339)
rotationVirtualStart := timeNowInLoc(location).Add(24 * time.Hour).Round(1 * time.Hour).Format(time.RFC3339)
Expand All @@ -359,11 +360,11 @@ func TestAccPagerDutySchedule_EscalationPolicyDependantWithOpenIncidents(t *test
CheckDestroy: testAccCheckPagerDutyScheduleDestroy,
Steps: []resource.TestStep{
{
Config: testAccCheckPagerDutyScheduleEscalationPolicyDependantWithOpenIncidentConfig(username, email, schedule, location, start, rotationVirtualStart, escalationPolicy1, service1),
Config: testAccCheckPagerDutyScheduleEscalationPolicyDependantWithOpenIncidentConfig(username, email, schedule1, location, start, rotationVirtualStart, escalationPolicy1, service1),
Check: resource.ComposeTestCheckFunc(
testAccCheckPagerDutyScheduleExists("pagerduty_schedule.foo"),
resource.TestCheckResourceAttr(
"pagerduty_schedule.foo", "name", schedule),
"pagerduty_schedule.foo", "name", schedule1),
resource.TestCheckResourceAttr(
"pagerduty_schedule.foo", "description", "foo"),
resource.TestCheckResourceAttr(
Expand All @@ -373,12 +374,12 @@ func TestAccPagerDutySchedule_EscalationPolicyDependantWithOpenIncidents(t *test
},
{
Config: testAccCheckPagerDutyScheduleEscalationPolicyDependantWithOpenIncidentConfigUpdated(username, email, escalationPolicy1, service1),
ExpectError: regexp.MustCompile("Before Removing Schedule \".*\" You must first resolve the following incidents related with Escalation Policies using this Schedule"),
ExpectError: regexp.MustCompile("Before Removing Schedule \".*\" You must first resolve or reassign the following incidents related with Escalation Policies using this Schedule"),
},
{
// Extra intermediate step with the original plan for resolving the
// outstanding incident and retrying the schedule destroy after that.
Config: testAccCheckPagerDutyScheduleEscalationPolicyDependantWithOpenIncidentConfig(username, email, schedule, location, start, rotationVirtualStart, escalationPolicy1, service1),
Config: testAccCheckPagerDutyScheduleEscalationPolicyDependantWithOpenIncidentConfig(username, email, schedule1, location, start, rotationVirtualStart, escalationPolicy1, service1),
Check: resource.ComposeTestCheckFunc(
testAccPagerDutyScheduleResolveIncident(p_incident_id, "pagerduty_escalation_policy.foo"),
),
Expand All @@ -391,13 +392,17 @@ func TestAccPagerDutySchedule_EscalationPolicyDependantWithOpenIncidents(t *test
),
},
{
Config: testAccCheckPagerDutyScheduleEscalationPolicyDependantWithUnrelatedOpenIncidentConfig(username, email, schedule, location, start, rotationVirtualStart, escalationPolicy1, escalationPolicy2, service1, service2),
Config: testAccCheckPagerDutyScheduleEscalationPolicyDependantWithUnrelatedOpenIncidentConfig(username, email, schedule1, schedule2, location, start, rotationVirtualStart, escalationPolicy1, escalationPolicy2, service1, service2),
Check: resource.ComposeTestCheckFunc(
testAccCheckPagerDutyScheduleExists("pagerduty_schedule.foo"),
resource.TestCheckResourceAttr(
"pagerduty_schedule.foo", "name", schedule),
"pagerduty_schedule.foo", "name", schedule1),
resource.TestCheckResourceAttr(
"pagerduty_schedule.foo", "description", "foo"),
resource.TestCheckResourceAttr(
"pagerduty_schedule.bar", "name", schedule2),
resource.TestCheckResourceAttr(
"pagerduty_schedule.bar", "description", "bar"),
resource.TestCheckResourceAttr(
"pagerduty_escalation_policy.foo", "name", escalationPolicy1),
resource.TestCheckResourceAttr(
Expand All @@ -410,8 +415,12 @@ func TestAccPagerDutySchedule_EscalationPolicyDependantWithOpenIncidents(t *test
),
},
{
Config: testAccCheckPagerDutyScheduleEscalationPolicyDependantWithUnrelatedOpenIncidentConfigUpdated(username, email, schedule, location, start, rotationVirtualStart, escalationPolicy1, escalationPolicy2, service1, service2),
Config: testAccCheckPagerDutyScheduleEscalationPolicyDependantWithUnrelatedOpenIncidentConfigUpdated(username, email, schedule1, schedule2, location, start, rotationVirtualStart, escalationPolicy1, escalationPolicy2, service1, service2),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(
"pagerduty_schedule.bar", "name", schedule2),
resource.TestCheckResourceAttr(
"pagerduty_schedule.bar", "description", "bar"),
resource.TestCheckResourceAttr(
"pagerduty_escalation_policy.foo", "name", escalationPolicy1),
resource.TestCheckResourceAttr(
Expand Down Expand Up @@ -1481,7 +1490,7 @@ resource "pagerduty_service" "bar" {
`, username, email, team, schedule, location, start, rotationVirtualStart, escalationPolicy1, escalationPolicy2, service1, service2)
}

func testAccCheckPagerDutyScheduleEscalationPolicyDependantWithUnrelatedOpenIncidentConfig(username, email, schedule, location, start, rotationVirtualStart, escalationPolicy1, escalationPolicy2, service1, service2 string) string {
func testAccCheckPagerDutyScheduleEscalationPolicyDependantWithUnrelatedOpenIncidentConfig(username, email, schedule1, schedule2, location, start, rotationVirtualStart, escalationPolicy1, escalationPolicy2, service1, service2 string) string {
return fmt.Sprintf(`
resource "pagerduty_user" "foo" {
name = "%[1]s"
Expand All @@ -1491,13 +1500,34 @@ resource "pagerduty_user" "foo" {
resource "pagerduty_schedule" "foo" {
name = "%[3]s"
time_zone = "%[4]s"
time_zone = "%[5]s"
description = "foo"
layer {
name = "foo"
start = "%[5]s"
rotation_virtual_start = "%[6]s"
start = "%[6]s"
rotation_virtual_start = "%[7]s"
rotation_turn_length_seconds = 86400
users = [pagerduty_user.foo.id]
restriction {
type = "daily_restriction"
start_time_of_day = "08:00:00"
duration_seconds = 32101
}
}
}
resource "pagerduty_schedule" "bar" {
name = "%[4]s"
time_zone = "%[5]s"
description = "bar"
layer {
name = "bar"
start = "%[6]s"
rotation_virtual_start = "%[7]s"
rotation_turn_length_seconds = 86400
users = [pagerduty_user.foo.id]
Expand All @@ -1510,7 +1540,7 @@ resource "pagerduty_schedule" "foo" {
}
resource "pagerduty_escalation_policy" "foo" {
name = "%[7]s"
name = "%[8]s"
num_loops = 2
rule {
Expand All @@ -1535,7 +1565,7 @@ resource "pagerduty_escalation_policy" "foo" {
}
resource "pagerduty_escalation_policy" "bar" {
name = "%[8]s"
name = "%[9]s"
num_loops = 2
rule {
Expand All @@ -1544,26 +1574,30 @@ resource "pagerduty_escalation_policy" "bar" {
type = "user_reference"
id = pagerduty_user.foo.id
}
target {
type = "schedule_reference"
id = pagerduty_schedule.bar.id
}
}
}
resource "pagerduty_service" "foo" {
name = "%[9]s"
name = "%[10]s"
description = "foo"
auto_resolve_timeout = 1800
acknowledgement_timeout = 1800
escalation_policy = pagerduty_escalation_policy.foo.id
alert_creation = "create_incidents"
}
resource "pagerduty_service" "bar" {
name = "%[10]s"
name = "%[11]s"
description = "bar"
auto_resolve_timeout = 1800
acknowledgement_timeout = 1800
escalation_policy = pagerduty_escalation_policy.bar.id
alert_creation = "create_incidents"
}
`, username, email, schedule, location, start, rotationVirtualStart, escalationPolicy1, escalationPolicy2, service1, service2)
`, username, email, schedule1, schedule2, location, start, rotationVirtualStart, escalationPolicy1, escalationPolicy2, service1, service2)
}

func testAccCheckPagerDutyScheduleWithTeamsEscalationPolicyDependantWithUnrelatedOpenIncidentConfigUpdated(username, email, schedule, location, start, rotationVirtualStart, team, escalationPolicy1, escalationPolicy2, service1, service2 string) string {
Expand Down Expand Up @@ -1625,15 +1659,36 @@ resource "pagerduty_service" "bar" {
`, username, email, team, schedule, location, start, rotationVirtualStart, escalationPolicy1, escalationPolicy2, service1, service2)
}

func testAccCheckPagerDutyScheduleEscalationPolicyDependantWithUnrelatedOpenIncidentConfigUpdated(username, email, schedule, location, start, rotationVirtualStart, escalationPolicy1, escalationPolicy2, service1, service2 string) string {
func testAccCheckPagerDutyScheduleEscalationPolicyDependantWithUnrelatedOpenIncidentConfigUpdated(username, email, schedule1, schedule2, location, start, rotationVirtualStart, escalationPolicy1, escalationPolicy2, service1, service2 string) string {
return fmt.Sprintf(`
resource "pagerduty_user" "foo" {
name = "%[1]s"
email = "%[2]s"
}
resource "pagerduty_schedule" "bar" {
name = "%[4]s"
time_zone = "%[5]s"
description = "bar"
layer {
name = "bar"
start = "%[6]s"
rotation_virtual_start = "%[7]s"
rotation_turn_length_seconds = 86400
users = [pagerduty_user.foo.id]
restriction {
type = "daily_restriction"
start_time_of_day = "08:00:00"
duration_seconds = 32101
}
}
}
resource "pagerduty_escalation_policy" "foo" {
name = "%[7]s"
name = "%[8]s"
num_loops = 2
rule {
Expand All @@ -1646,7 +1701,7 @@ resource "pagerduty_escalation_policy" "foo" {
}
resource "pagerduty_escalation_policy" "bar" {
name = "%[8]s"
name = "%[9]s"
num_loops = 2
rule {
Expand All @@ -1655,26 +1710,30 @@ resource "pagerduty_escalation_policy" "bar" {
type = "user_reference"
id = pagerduty_user.foo.id
}
target {
type = "schedule_reference"
id = pagerduty_schedule.bar.id
}
}
}
resource "pagerduty_service" "foo" {
name = "%[9]s"
name = "%[10]s"
description = "foo"
auto_resolve_timeout = 1800
acknowledgement_timeout = 1800
escalation_policy = pagerduty_escalation_policy.foo.id
alert_creation = "create_incidents"
}
resource "pagerduty_service" "bar" {
name = "%[10]s"
name = "%[11]s"
description = "bar"
auto_resolve_timeout = 1800
acknowledgement_timeout = 1800
escalation_policy = pagerduty_escalation_policy.bar.id
alert_creation = "create_incidents"
}
`, username, email, schedule, location, start, rotationVirtualStart, escalationPolicy1, escalationPolicy2, service1, service2)
`, username, email, schedule1, schedule2, location, start, rotationVirtualStart, escalationPolicy1, escalationPolicy2, service1, service2)
}

func testAccCheckPagerDutyScheduleWithTeamsEscalationPolicyDependantWithOpenIncidentConfigUpdated(username, email, team, escalationPolicy, service string) string {
Expand Down

0 comments on commit ee0f0b4

Please sign in to comment.