terraform-provider-pagerduty
terraform-provider-pagerduty copied to clipboard
resource pagerduty_service ignoring lifecycle rule for last_incident_timestamp
Terraform Version
-
Terraform v1.0.0
pagerduty provider version:
-
Installed pagerduty/pagerduty v1.9.9 (signed by a HashiCorp partner, key ID 027C6DD1F0707B45)
Affected Resource(s)
# module.external_services_plaid.pagerduty_service.this has been changed
~ resource "pagerduty_service" "this" {
id = "PG2RQLF"
~ last_incident_timestamp = "2021-09-05T16:16:07-07:00" -> "2021-09-09T17:30:16-07:00"
name = "External services Plaid (Service)"
# (8 unchanged attributes hidden)
# (1 unchanged block hidden)
}
Terraform Configuration Files
resource "pagerduty_service" "this" {
name = "${var.service_name} (Service)"
escalation_policy = var.escalation_policy.id
alert_creation = "create_alerts_and_incidents"
acknowledgement_timeout = "null"
auto_resolve_timeout = "null"
lifecycle {
ignore_changes = [last_incident_timestamp, status]
}
}
Debug Output
Panic Output
Expected Behavior
per the lifecycle
block on the resource, changes to last_incident_timestamp
should have been ignored.
Actual Behavior
changes to last_incident_timestamp
result in copious irrelevant plan output
Steps to Reproduce
-
terraform plan
Important Factoids
pretty bog-standard
References
search for last_incident_timestamp
in previous issues returned no results.
Seeing this well.
Seeing this too, very annoying with the unwanted plan changes
Similarly we also see a outside of Terraform change of status
in the pagerduty_service
rule.
# module.platform-service.pagerduty_service.service has been changed
~ resource "pagerduty_service" "service" {
id = "PAQCMT4"
+ last_incident_timestamp = "2021-12-02T10:18:57+01:00"
name = "Platform"
~ status = "active" -> "critical"
# (8 unchanged attributes hidden)
# (1 unchanged block hidden)
}
Any update on this?
Similarly we also see a outside of Terraform change of status in the pagerduty_service rule.
We see the same.
We faced the exactly same problem.
resource "pagerduty_service" "service_hosting" {
name = "Hosting"
alert_creation = "create_alerts_and_incidents"
escalation_policy = pagerduty_escalation_policy.default.id
acknowledgement_timeout = "null"
auto_resolve_timeout = "null"
incident_urgency_rule {
type = "constant"
urgency = "high"
}
lifecycle {
ignore_changes = [last_incident_timestamp,]
}
}
# pagerduty_service.service_hosting has changed
~ resource "pagerduty_service" "service_hosting" {
id = "P2L0PDG"
~ last_incident_timestamp = "2023-02-28T17:11:57-05:00" -> "2023-03-03T14:56:30-05:00"
name = "Hosting"
# (9 unchanged attributes hidden)
# (1 unchanged block hidden)
}
What's the point of tracking these attributes in TF state at all?
Proposal:
If people are relying on this attribute they should be using a data resource. Terraform is not meant to manage the state of things that are constantly changing behind the scenes. We should update this provider to no longer track this attribute in state at the resource
level.
Is that something the team would be down with?
I don't have any stake in this but figured I'd clarify the Terraform behavior a bit and what's proposed in #690.
What Terraform is raising here is drift, not planned changes as some of the comments here suggest. ignore_changes
suppresses planned changes, not the detection and reporting of drift during refresh. Because this attribute is Computed
and not also Optional
, there can never be planned changes.
There has been extensive discussion about confusion around drift reporting:
https://github.com/hashicorp/terraform/issues/28803
The Terraform core team worked on this to try to limit the drift detection reporting to increase the signal to noise:
https://github.com/hashicorp/terraform/pull/30486
The assumption made here is that extra refresh information, i.e "false positives", are a larger impediment to user's understanding of the plan than possibly hidden refresh information, which in most cases can be still inferred from the changes in the plan output. Taking this premise, we record only the specific attribute addresses that contributed to the plan, and filter the output down to those specific references.
No one's mentioned their Terraform version since the initial report, which predated this improvement.
Terraform is not meant to manage the state of things that are constantly changing behind the scenes.
This is not correct. Computed attributes exist so that Terraform can track but not manage certain attributes of a resource that are determined by the server. Including server-computed timestamps that might be valuable to the user is common/valid:
https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/db_instance#latest_restorable_time
If people are relying on this attribute they should be using a data resource.
This is definitely not correct, at least so long as it implies only data.pagerduty_service.last_incident_timestamp
should exist but not the equivalent resource attribute. Providers strive to keep the data source and resource for a given object of the same general shape for good reasons. One is that Terraform emphasizes the use of object types, e.g.:
variable "pagerduty_service" {
type = object({
id = string
name = string
last_incident_timestamp = string
})
}
Whether last_incident_timestamp
is useful or not in Terraform is certainly debatable. If you wanted logic that says "create some dependent resource if this service has had an incident in the last n months" you'd need it. Whether this is too contrived to be worthwhile isn't for me to say.
To the extent a data source is the answer, it's not diverging from the resource, it's a new one, data.pagerduty_service_status
.
And then from #690:
To avoid a backwards incompatible change, would it make sense to keep these in the resource state (and add Deprecated to them) but with a constant value (e.g. unknown and 1970-01-01T00:00:00.000Z)?
This is not any less breaking than removing the attributes entirely. If users are depending on them, breaking their behavior but preserving the structure is still breaking.
A computed attribute is generally the result of inputs though. If the inputs change that drive it, than the computed attribute should to. If computed attributes are changing constantly without any change to the inputs that's definitely an anti-pattern.
Exposing this as an attribute on the data object only and removing last_incident_timestamp
from the resource in newer versions makes the most sense to me.