Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase Retry Time on Data Sources #454

Merged
merged 3 commits into from
Feb 10, 2022
+64 −47
Merged

Increase Retry Time on Data Sources #454

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9eceed6
increase retry on data sources
Feb 3, 2022
5931668
Merge branch 'master' into extend-retry
Feb 8, 2022
c4169b1
fix conflicts
Feb 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pagerduty/data_source_pagerduty_business_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func dataSourcePagerDutyBusinessServiceRead(d *schema.ResourceData, meta interfa

searchName := d.Get("name").(string)

return resource.Retry(2*time.Minute, func() *resource.RetryError {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
resp, _, err := client.BusinessServices.List()
if err != nil {
if isErrCode(err, 429) {
Expand Down
2 changes: 1 addition & 1 deletion pagerduty/data_source_pagerduty_escalation_policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func dataSourcePagerDutyEscalationPolicyRead(d *schema.ResourceData, meta interf
Query: searchName,
}

return resource.Retry(2*time.Minute, func() *resource.RetryError {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
resp, _, err := client.EscalationPolicies.List(o)
if err != nil {
if isErrCode(err, 429) {
Expand Down
2 changes: 1 addition & 1 deletion pagerduty/data_source_pagerduty_extension_schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func dataSourcePagerDutyExtensionSchemaRead(d *schema.ResourceData, meta interfa

searchName := d.Get("name").(string)

return resource.Retry(2*time.Minute, func() *resource.RetryError {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
resp, _, err := client.ExtensionSchemas.List(&pagerduty.ListExtensionSchemasOptions{Query: searchName})
if err != nil {
if isErrCode(err, 429) {
Expand Down
2 changes: 1 addition & 1 deletion pagerduty/data_source_pagerduty_priority.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func dataSourcePagerDutyPriorityRead(d *schema.ResourceData, meta interface{}) e

searchTeam := d.Get("name").(string)

return resource.Retry(2*time.Minute, func() *resource.RetryError {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
resp, _, err := client.Priorities.List()
if err != nil {
if isErrCode(err, 429) {
Expand Down
2 changes: 1 addition & 1 deletion pagerduty/data_source_pagerduty_ruleset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func dataSourcePagerDutyRulesetRead(d *schema.ResourceData, meta interface{}) er

searchName := d.Get("name").(string)

return resource.Retry(2*time.Minute, func() *resource.RetryError {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
resp, _, err := client.Rulesets.List()
if err != nil {
if isErrCode(err, 429) {
Expand Down
2 changes: 1 addition & 1 deletion pagerduty/data_source_pagerduty_schedule.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func dataSourcePagerDutyScheduleRead(d *schema.ResourceData, meta interface{}) e
Query: searchName,
}

return resource.Retry(2*time.Minute, func() *resource.RetryError {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
resp, _, err := client.Schedules.List(o)
if err != nil {
if isErrCode(err, 429) {
Expand Down
2 changes: 1 addition & 1 deletion pagerduty/data_source_pagerduty_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func dataSourcePagerDutyServiceRead(d *schema.ResourceData, meta interface{}) er
Query: searchName,
}

return resource.Retry(2*time.Minute, func() *resource.RetryError {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
resp, _, err := client.Services.List(o)
if err != nil {
if isErrCode(err, 429) {
Expand Down
2 changes: 1 addition & 1 deletion pagerduty/data_source_pagerduty_service_integration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func dataSourcePagerDutyServiceIntegrationRead(d *schema.ResourceData, meta inte
Query: searchName,
}

return resource.Retry(2*time.Minute, func() *resource.RetryError {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
resp, _, err := client.Services.List(o)
if err != nil {
return handleError(err)
Expand Down
2 changes: 1 addition & 1 deletion pagerduty/data_source_pagerduty_tag.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func dataSourcePagerDutyTagRead(d *schema.ResourceData, meta interface{}) error
Query: searchTag,
}

return resource.Retry(2*time.Minute, func() *resource.RetryError {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
resp, _, err := client.Tags.List(o)
if err != nil {
if isErrCode(err, 429) {
Expand Down
2 changes: 1 addition & 1 deletion pagerduty/data_source_pagerduty_team.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func dataSourcePagerDutyTeamRead(d *schema.ResourceData, meta interface{}) error
Query: searchTeam,
}

return resource.Retry(2*time.Minute, func() *resource.RetryError {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
resp, _, err := client.Teams.List(o)
if err != nil {
if isErrCode(err, 429) {
Expand Down
2 changes: 1 addition & 1 deletion pagerduty/data_source_pagerduty_user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func dataSourcePagerDutyUserRead(d *schema.ResourceData, meta interface{}) error
Query: searchEmail,
}

return resource.Retry(2*time.Minute, func() *resource.RetryError {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
resp, _, err := client.Users.List(o)
if err != nil {
if isErrCode(err, 429) {
Expand Down
2 changes: 1 addition & 1 deletion pagerduty/data_source_pagerduty_user_contact_method.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func dataSourcePagerDutyUserContactMethodRead(d *schema.ResourceData, meta inter
searchLabel := d.Get("label").(string)
searchType := d.Get("type").(string)

return resource.Retry(2*time.Minute, func() *resource.RetryError {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
resp, _, err := client.Users.ListContactMethods(userId)
if err != nil {
errResp := handleNotFoundError(err, d)
Expand Down
2 changes: 1 addition & 1 deletion pagerduty/data_source_pagerduty_vendor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func dataSourcePagerDutyVendorRead(d *schema.ResourceData, meta interface{}) err
o := &pagerduty.ListVendorsOptions{
Query: searchName,
}
return resource.Retry(2*time.Minute, func() *resource.RetryError {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
resp, _, err := client.Vendors.List(o)
if err != nil {
if isErrCode(err, 429) {
Expand Down
2 changes: 1 addition & 1 deletion pagerduty/resource_pagerduty_addon.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func buildAddonStruct(d *schema.ResourceData) *pagerduty.Addon {

func fetchPagerDutyAddon(d *schema.ResourceData, meta interface{}, errCallback func(error, *schema.ResourceData) error) error {
client, _ := meta.(*Config).Client()
return resource.Retry(2*time.Minute, func() *resource.RetryError {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
addon, _, err := client.Addons.Get(d.Id())
if err != nil {
log.Printf("[WARN] Service read error")
Expand Down
4 changes: 2 additions & 2 deletions pagerduty/resource_pagerduty_business_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func buildBusinessServiceStruct(d *schema.ResourceData) (*pagerduty.BusinessServ
func resourcePagerDutyBusinessServiceCreate(d *schema.ResourceData, meta interface{}) error {
client, _ := meta.(*Config).Client()

retryErr := resource.Retry(2*time.Minute, func() *resource.RetryError {
retryErr := resource.Retry(5*time.Minute, func() *resource.RetryError {

businessService, err := buildBusinessServiceStruct(d)
if err != nil {
Expand All @@ -122,7 +122,7 @@ func resourcePagerDutyBusinessServiceRead(d *schema.ResourceData, meta interface

log.Printf("[INFO] Reading PagerDuty business service %s", d.Id())

retryErr := resource.Retry(2*time.Minute, func() *resource.RetryError {
retryErr := resource.Retry(5*time.Minute, func() *resource.RetryError {
if businessService, _, err := client.BusinessServices.Get(d.Id()); err != nil {
return resource.RetryableError(err)
} else if businessService != nil {
Expand Down
4 changes: 2 additions & 2 deletions pagerduty/resource_pagerduty_business_service_subscriber.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func resourcePagerDutyBusinessServiceSubscriberCreate(d *schema.ResourceData, me
client, _ := meta.(*Config).Client()
businessServiceId := d.Get("business_service_id").(string)

retryErr := resource.Retry(2*time.Minute, func() *resource.RetryError {
retryErr := resource.Retry(5*time.Minute, func() *resource.RetryError {

businessServiceSubscriber, err := buildBusinessServiceSubscriberStruct(d)
if err != nil {
Expand Down Expand Up @@ -89,7 +89,7 @@ func resourcePagerDutyBusinessServiceSubscriberRead(d *schema.ResourceData, meta

log.Printf("[INFO] Reading PagerDuty business service %s subscriber %s type %s", businessServiceId, businessServiceSubscriber.ID, businessServiceSubscriber.Type)

return resource.Retry(30*time.Second, func() *resource.RetryError {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
if subscriberResponse, _, err := client.BusinessServiceSubscribers.List(businessServiceId); err != nil {
time.Sleep(2 * time.Second)
return resource.RetryableError(err)
Expand Down
29 changes: 21 additions & 8 deletions pagerduty/resource_pagerduty_escalation_policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,19 +103,32 @@ func buildEscalationPolicyStruct(d *schema.ResourceData) *pagerduty.EscalationPo

func resourcePagerDutyEscalationPolicyCreate(d *schema.ResourceData, meta interface{}) error {
client, _ := meta.(*Config).Client()
var readErr error

escalationPolicy := buildEscalationPolicyStruct(d)

log.Printf("[INFO] Creating PagerDuty escalation policy: %s", escalationPolicy.Name)

escalationPolicy, _, err := client.EscalationPolicies.Create(escalationPolicy)
if err != nil {
return err
}
return resource.Retry(5*time.Minute, func() *resource.RetryError {
escalationPolicy, _, err := client.EscalationPolicies.Create(escalationPolicy)
if err != nil {
if isErrCode(err, 429) {
// 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)
return resource.RetryableError(err)
}

d.SetId(escalationPolicy.ID)
return resource.NonRetryableError(err)
}

return resourcePagerDutyEscalationPolicyRead(d, meta)
d.SetId(escalationPolicy.ID)
readErr = resourcePagerDutyEscalationPolicyRead(d, meta)
if readErr != nil {
return resource.NonRetryableError(readErr)
}
return nil
})
}

func resourcePagerDutyEscalationPolicyRead(d *schema.ResourceData, meta interface{}) error {
Expand All @@ -125,7 +138,7 @@ func resourcePagerDutyEscalationPolicyRead(d *schema.ResourceData, meta interfac

o := &pagerduty.GetEscalationPolicyOptions{}

return resource.Retry(2*time.Minute, func() *resource.RetryError {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
escalationPolicy, _, err := client.EscalationPolicies.Get(d.Id(), o)
if err != nil {
time.Sleep(2 * time.Second)
Expand Down Expand Up @@ -155,7 +168,7 @@ func resourcePagerDutyEscalationPolicyUpdate(d *schema.ResourceData, meta interf

log.Printf("[INFO] Updating PagerDuty escalation policy: %s", d.Id())

retryErr := resource.Retry(2*time.Minute, func() *resource.RetryError {
retryErr := resource.Retry(5*time.Minute, func() *resource.RetryError {
if _, _, err := client.EscalationPolicies.Update(d.Id(), escalationPolicy); err != nil {
return resource.RetryableError(err)
}
Expand Down
44 changes: 24 additions & 20 deletions pagerduty/resource_pagerduty_service_dependency.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func resourcePagerDutyServiceDependencyAssociate(d *schema.ResourceData, meta in
log.Printf("[INFO] Associating PagerDuty dependency %s", serviceDependency.ID)

var dependencies *pagerduty.ListServiceDependencies
retryErr := resource.Retry(30*time.Second, func() *resource.RetryError {
retryErr := resource.Retry(5*time.Minute, func() *resource.RetryError {
if dependencies, _, err = client.ServiceDependencies.AssociateServiceDependencies(&input); err != nil {
if isErrCode(err, 404) {
return resource.RetryableError(err)
Expand Down Expand Up @@ -160,26 +160,30 @@ func resourcePagerDutyServiceDependencyDisassociate(d *schema.ResourceData, meta

log.Printf("[INFO] Disassociating PagerDuty dependency %s", dependency.DependentService.ID)

// listServiceRelationships by calling get dependencies using the serviceDependency.DependentService.ID
depResp, _, err := client.ServiceDependencies.GetServiceDependenciesForType(dependency.DependentService.ID, dependency.DependentService.Type)
if err != nil {
return err
}

var foundDep *pagerduty.ServiceDependency

// loop serviceRelationships until relationship.IDs match
for _, rel := range depResp.Relationships {
if rel.ID == d.Id() {
foundDep = rel
break
// listServiceRelationships by calling get dependencies using the serviceDependency.DependentService.ID
retryErr := resource.Retry(5*time.Minute, func() *resource.RetryError {
if dependencies, _, err := client.ServiceDependencies.GetServiceDependenciesForType(dependency.DependentService.ID, dependency.DependentService.Type); err != nil {
if isErrCode(err, 404) || isErrCode(err, 500) || isErrCode(err, 429) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stmcallister, would be a good idea to centralize the logic of what is a returnable error? I'm not sure if it is worth it to retry on 400 at all for example. I can see the benefit of increasing the retry to 5min for 429 and maybe 503/504 errors, but it might substantially hurt the experience if there is a real 404 or 400.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@drastawi Thanks for the feedback! The tricky part is that in some race condition situations we do want to retry on a 404 🤔 Good point on the centralizing of the logic.

return resource.RetryableError(err)
}
return resource.NonRetryableError(err)
} else if dependencies != nil {
for _, rel := range dependencies.Relationships {
if rel.ID == d.Id() {
foundDep = rel
break
}
}
}
}
// check if relationship not found
if foundDep == nil {
d.SetId("")
return nil
})
if retryErr != nil {
time.Sleep(5 * time.Second)
return retryErr
}

// convertType is needed because the PagerDuty API returns the 'reference' values in responses but wants the other
// values in requests
foundDep.SupportingService.Type = convertType(foundDep.SupportingService.Type)
Expand All @@ -193,17 +197,17 @@ func resourcePagerDutyServiceDependencyDisassociate(d *schema.ResourceData, meta
input := pagerduty.ListServiceDependencies{
Relationships: r,
}
retryErr := resource.Retry(30*time.Second, func() *resource.RetryError {
retryErr = resource.Retry(5*time.Minute, func() *resource.RetryError {
if _, _, err = client.ServiceDependencies.DisassociateServiceDependencies(&input); err != nil {
if isErrCode(err, 404) {
if isErrCode(err, 404) || isErrCode(err, 429) {
return resource.RetryableError(err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stmcallister I am a bit concerned the end users might have to wait for 5 min to find out a resource already exists or had already been deleted manually which are not that uncommon.

One solution that seems fairly easy to implement at a glance would be to add a max timeout to the retryable error as an extra optional parameter

Suggested change
return resource.RetryableError(err)
if isErrCode(err, 429) {
return resource.RetryableError(err)
} else if isErrCode(err, 404) {
return resource.RetryableErrorWithMaxTimeout(err, 15 * time.Second)
}

}
return resource.NonRetryableError(err)
}
return nil
})
if retryErr != nil {
time.Sleep(2 * time.Second)
time.Sleep(5 * time.Second)
return retryErr
}

Expand Down Expand Up @@ -264,7 +268,7 @@ func findDependencySetState(depID, serviceID, serviceType string, d *schema.Reso

// Pausing to let the PD API sync.
time.Sleep(1 * time.Second)
retryErr := resource.Retry(30*time.Second, func() *resource.RetryError {
retryErr := resource.Retry(5*time.Minute, func() *resource.RetryError {
if dependencies, _, err := client.ServiceDependencies.GetServiceDependenciesForType(serviceID, serviceType); err != nil {
if isErrCode(err, 404) || isErrCode(err, 500) || isErrCode(err, 429) {
return resource.RetryableError(err)
Expand Down
2 changes: 1 addition & 1 deletion pagerduty/resource_pagerduty_tag_assignment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func resourcePagerDutyTagAssignmentCreate(d *schema.ResourceData, meta interface

log.Printf("[INFO] Creating PagerDuty tag assignment with tagID %s for %s entity with ID %s", assignment.TagID, assignment.EntityType, assignment.EntityID)

retryErr := resource.Retry(10*time.Second, func() *resource.RetryError {
retryErr := resource.Retry(5*time.Minute, func() *resource.RetryError {
if _, err := client.Tags.Assign(assignment.EntityType, assignment.EntityID, assignments); err != nil {
if isErrCode(err, 400) || isErrCode(err, 429) {
return resource.RetryableError(err)
Expand Down