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.

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

spurious diff on pagerduty_schedule #200

Open
rblumen-desk opened this issue Apr 9, 2020 · 14 comments
Open

spurious diff on pagerduty_schedule #200

rblumen-desk opened this issue Apr 9, 2020 · 14 comments

Comments

@rblumen-desk
Copy link

Terraform Version

v0.12.24

Affected Resource(s)

Please list the resources as a list, for example:

  • pagerduty_schedule

If this issue appears to affect multiple resources, it may be an issue with Terraform's core, so please mention this.

Terraform Configuration Files

terraform {
  required_version = ">= 0.12.20"

  backend local {
    path = "tfstate"
  }
}

provider pagerduty {}
variable team_members {
  type        = list(string)
  description = "list of emails of the team members in the on call rotations"
  default     = ["<redacted>"]
}

variable time_zone {
  description = "time zone in which the schedule is defined.  the value must be a string from the [tz database](https://en.wikipedia.org/wiki/List_of_tz_database_time_zones)"
  default     = "America/Los_Angeles"
}

variable rotation_start_time {
  description = "all on-call rotations will start at ths time (in RFC3339 format)"
  default     = "2019-12-03T00:00:00.00-08:00"
}
data pagerduty_user team_members {
  for_each = toset(var.team_members)
  email    = each.value
}

resource pagerduty_team team {
  name        = "BUG team"
  description = "BUG team"
}

resource pagerduty_team_membership membership {
  for_each = data.pagerduty_user.team_members
  team_id  = pagerduty_team.team.id
  user_id  = each.value.id
}

resource pagerduty_schedule on_call {
  name        = "BUG on call"
  time_zone   = var.time_zone
  description = "BUG on call"

  layer {
    name                         = "layer 1"
    start                        = var.rotation_start_time
    rotation_virtual_start       = var.rotation_start_time
    rotation_turn_length_seconds = 604800
    users                        = values(data.pagerduty_user.team_members)[*].id
  }
}

Debug Output

Not available.

Panic Output

N/A

Expected Behavior

The plan should not show any changes

Actual Behavior

Spurious diffs were shown

Terraform will perform the following actions:

  # pagerduty_schedule.on_call will be updated in-place
  ~ resource "pagerduty_schedule" "on_call" {
        description = "BUG on call"
        id          = "<redacted>"
        name        = "BUG on call"
        time_zone   = "America/Los_Angeles"

      ~ layer {
            id                           = "<redacted>"
            name                         = "layer 1"
            rotation_turn_length_seconds = 604800
          ~ rotation_virtual_start       = "2019-12-03T00:00:00-08:00" -> "2019-12-03T00:00:00.00-08:00"
          ~ start                        = "2020-04-09T15:37:39-07:00" -> "2019-12-03T00:00:00.00-08:00"
            users                        = [
                "<redacted>",
            ]
        }
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Steps to Reproduce

Please list the steps required to reproduce the issue, for example:

  1. terraform apply -auto-approve
  2. terraform plan

Important Factoids

N/A

@stmcallister
Copy link
Contributor

stmcallister commented Apr 22, 2020

Hi @rblumen-desk! In looking at the diff I'm seeing that you're trying to set timestamps with a decimal value on the time. 00:00:00.00. The PagerDuty API only supports DateTime values in the ISO 8601 format. You can read more about that on Types page of the PagerDuty Developer docs.

@rblumen-desk
Copy link
Author

@stmcallister I am not understanding your comment. I don't think that we are on the same page about how I see the problem. On the first run of apply, the provider reads the default value of the rotation start time which is in RFC3339 format, which I believe is compatible with or the same as ISO 8601. You can see the value that we set which in the default value. But it doesn't matter what the default is for the purpose of this ticket because the bug occurs on the second pass. The default could be any value. If the provider could not parse the default value there would be a failure on the first pass. The first time through, the apply completes and creates a schedule. The issue comes on the second run of plan with no code changes. After an apply has completed a second plan with no source code changes should show zero diffs. A diff on a plan against unchanged infrastructure with unchanged code is prima facie a bug.

@stmcallister
Copy link
Contributor

Hi @rblumen-desk. I'm still unable to replicate this issue. One of the common issues that I (and other users) run into is not matching the time zone offset with that of your PagerDuty account.

Also, in relation to the differing datetime formats, in the schedule resource those rotation_virtual_start and start fields are of the string type. So, Terraform is comparing those values as strings. If the time stamp you have set in the var.rotation_start_time differs as a string than the value in the Terraform state then Terraform will want to make a change.

@rblumen-desk
Copy link
Author

@stmcallister based on your comments I have done some more experiments on this. I think that there are two different issues here. One is that there is something to do with time zones and possibly DST. If I use a timezone offset of -NN the difference between UTC and Pacific is not stable. To take this off the table I changed the time zone to UTC and a virtual start time of "2020-05-02T17:00:00Z". With that configuration the spurious diff on the rotation_virtual_start field is no longer happening.

With a time zone of UTC and setting the value of the date values like this
variable start { default = "2020-05-07T00:00:Z" } variable virtual { default = "2020-05-02T17:00:00Z" }
then I in some cases still get a diff on the start field. The diff looks like this:
~ start = "2020-05-07T19:11:04Z" -> "2020-05-07T00:00:Z"
I found the following in your documentation

start - (Required) The start time of the schedule layer. This value will not be read back from the PagerDuty API because the API will always return a new start time, which represents the last updated time of the schedule layer.

I am finding this a bit confusing - if the field is being updated internally then the provider should not be comparing it to the value in the terraform file other than on creation. It looks like it is being updated, and is being read back and is being compared. Can you find out if the documentation is correct or if this is working as expected?

I am seeing the following behavior which is also confusing. When I start from a zero state, run an apply it builds the resources. Then I run a plan, I get a diff on the start field as described above. If I apply that and run another plan, then I get no diffs. The spurious diff happens once and then does not happen again even with no code changes.

@sprutton1
Copy link

I believe that I am seeing the same issue. I see the start time value changing any time I modify a schedule. This makes sense from the perspective of the start time I have in my .tf files being different than what is in PagerDuty, but I'm not sure why I'm required to set a start time in tf if PagerDuty is going to determine one for me anyways?

@JeremieBethmont
Copy link

I confirm that I am seeing the same issue too. The Pagerduty API sets the start time at creation time and TF provider doesn't cater for that. And looking at the Pagerduty API docs the start attribute is actually optional... shouldn't that be the same for TF provider?

As a temporary workaround @rblumen-desk @sprutton1 I was able to use the Lifecycle Customizations: ignore changes resource block in order to force TF to ignore the changes.

resource pagerduty_schedule my_schedule {
  ...
  layer {
    start                        = "2020-01-01T00:00:00Z"
    ...
  }

  lifecycle {
    ignore_changes = [
      layer[0].start
    ]
  }
}

@2rs2ts
Copy link

2rs2ts commented Apr 13, 2021

The docs say this, but it's obviously not true:

start - (Required) The start time of the schedule layer. This value will not be read back from the PagerDuty API because the API will always return a new start time, which represents the last updated time of the schedule layer.

@stmcallister
Copy link
Contributor

@2rs2ts Good point! Added that fix to #322 which contains a few other random doc fixes.

@Padorax
Copy link
Contributor

Padorax commented Apr 14, 2021

I've upgraded to 1.9.6 and still have this start diff issue. I have no changes in the configuration file, but PagerDuty updated the start field internally, then terraform keeps diffing against this. After applying that plan, the diff disappears.

The #321 seems to suppress "Equal Diff" when the time is in different format (e.g., timezone) but is the same time. But it cannot fix the above case where the start actually changes due to PagerDuty updates. But this is also a bit confusing as why the provider would reads from the PagerDuty API some value that could be internally updated?

@stmcallister
Copy link
Contributor

@Padorax The start value can't be set to a time in the past. So, if the value you pass to the PagerDuty API is before the current time then PagerDuty will set start to the current time.

So, in Robert's case at the top of this thread, he was trying to set start = 2019-12-03T00:00:00.00-08:00, however it appears that he ran terraform apply (ie, called the PagerDuty API) at 2020-04-09T15:37:39-07:00 because that's the time PagerDuty set the start for the schedule layer.

Does that help?

@drastawi
Copy link
Contributor

@stmcallister it does not seem like a sustainable solution. If you are running terraform apply in automation on a regular basis or even if your want to make an unrelated change in your schedule you definitely do not want to maintain the start time for all the schedules in the same project.

@noqcks
Copy link

noqcks commented May 11, 2021

@stmcallister this is still a problem for us with v1.9.6.

In 1.9.5 we get

No changes. Infrastructure is up-to-date.

In 1.9.6 we get this on every single apply

  ~ pagerduty_schedule.test      
        layer.0.start: "2021-05-03T16:04:24Z" => "2020-08-03T08:00:00Z"
        layer.1.start: "2021-05-03T16:04:24Z" => "2021-04-05T08:00:00Z"
        layer.2.start: "2021-05-03T16:04:24Z" => "2020-07-20T08:00:00Z"

The only workaround right now as mentioned above is to add a lifecycle ignore_changes after you have initially created your schedule. However, if you need to actually update your schedule start time, you need to remove this lifecycle ignore, apply, and then add back the lifecycle ignore.

Are there any other suggested workarounds right now?

@chenrui333
Copy link
Contributor

chenrui333 commented Jun 1, 2021

^^ I have also seen the above issue on my end as well (I ended up sync the configurations back into code).

Padorax added a commit to Padorax/terraform-provider-pagerduty that referenced this issue Jun 8, 2021
Issue: PagerDuty#200

When both schedule layer start time (the old one read from PagerDuty API, and the new one read
from configuration files) are in the past, there is no need to show Diff as the acutual start
time will be updated by PagerDuty API to current time.

Otherwise, users will have to sync the configuration files from time to time, when they make
irrelevant changes to their schedules (e.g., adding a user) or even no changes at all.
@2rs2ts
Copy link

2rs2ts commented Feb 22, 2024

Did #343 fix this? I'm not seeing this behavior anymore, but I'm a bit worried that it's only due to some sort of dumb luck. Would have to go change all our codebase only to have this recur.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants