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

When activate=true, always read and clone from the active version #423

Merged
merged 1 commit into from
Jun 17, 2021
Merged

When activate=true, always read and clone from the active version #423

merged 1 commit into from
Jun 17, 2021

Conversation

bengesoff
Copy link
Contributor

This PR implements the proposed solution in #416, and fixes the bug reported in #419.

There are two key points in the service resource lifecycle where the service version is particularly important. The first is when reading the state, and the second is when cloning a version to make updates to the service. The same version must be used at both of these points to ensure that the plan, based on the diff between the state and the configuration, is applied to the same version that the plan assumed it would be. Violating this rule leads to bugs such as the one in #419.

Updates are always based off cloned_version, and cloned_version is always the last version that Terraform made changes to. When the provider has finished making updates, it sets cloned_version to the new version it has just created, so this is always true.

However, the version that the state is read from depends on whether activate is true. If so, then state is read from the active_version, and it was previously assumed that this would always be the same as cloned_version as the cloned_version was always activated after it was created. If activate was false, then the state would be read from cloned_version. The rationale behind this is laid out in the second paragraph of #416, and a snippet is given below:

The alternative of always reading and cloning the active version doesn't work for when activate is false as it means Terraform can never read its own changes until someone activates the service externally, and there will be constant non-empty plans until this happens. The other alternative of always reading and cloning the most recent version (highest version number) also probably doesn't fit as it doesn't allow anyone to experiment with new drafts outside of Terraform without Terraform attempting to revert their changes in the next apply.

However, the assumption that cloned_version and active_version would always match was incorrect. It is valid when activate is false, but when activate is true, it does not account for the fact that the version could be created and activated outside of Terraform. In this case, the version that state is read from, and the one the updates are applied to, can be different, and errors can occur. The following diagram illustrates this case:

image

Note that this sequence is only the case when activate is true, as the cloned_version is always read when it is false.

The green section shows the case where the assumption is valid (no changes outside of Terraform). The red section shows how the mismatch occurs when a new version is activated externally.

This commit changes the behaviour so that, when activate is true, the value of cloned_version is updated on every state read to match the currently active version. This ensures that the above assumption is valid, even when a version is cloned and activated outside of Terraform. The following diagram shows how the previous sequence is changed.

image

With this fix in place, it should now always be true that the same version will be used for reading the state and making updates to. It should also mean that the behaviour is more in line with what users expect as, when activate is true, they can make changes to the active version and see them reflected in the next Terraform plan, as you would see with a resource that does not use versioning.

There is also an acceptance test added to replicate the behaviour in #419, to validate that this has been fixed.

Before, updates would all be based off `cloned_version`, and `cloned_version` would always be the last version that Terraform made changes to.
However, the version that the state was read from depended on whether `activate` was `true`.
If so, then state was read from the `active version`, and it was assumed that this would be the same as `cloned_version` as `activate` was `true` and the `cloned_version` was always activated after it was created.
If `activate` was `false`, then the state would be read from `cloned_version`.

However, this assumption was incorrect.
It is valid when `activate` is `false`, but when `activate` is `true`, it does not account for the fact that the version could be created and activated outside of Terraform.
In this case, the version that state is read from, and the one the updates are applied to, can be different, and errors can occur.

This commit changes the behaviour so that, when `activate` is `true`, the value of `cloned_version` is updated on every state read to match the currently active version.
This ensures that the above assumption is valid, even when a version is cloned and activated outside of Terraform.

There is also an acceptance test added to replicate the behaviour that led to original bug, to validate that it has been fixed.

With this fix in place, it should now always be true that the same version will be used for reading the state and making updates to.
@Integralist Integralist merged commit 7325c2c into fastly:main Jun 17, 2021
@bengesoff bengesoff deleted the cloned-version-follow-active-version branch June 17, 2021 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants