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

fix: delay before using policy #518

Merged
merged 2 commits into from
Jan 26, 2024
Merged

fix: delay before using policy #518

merged 2 commits into from
Jan 26, 2024

Conversation

shemau
Copy link
Contributor

@shemau shemau commented Jan 24, 2024

Description

Issue: 7308

Release required?

  • No release
  • Patch release (x.x.X)
  • Minor release (x.X.x)
  • Major release (X.x.x)
Release notes content

Upgrades from earlier to version to this version may show a time_sleep.wait_for_authorization_policy being deleted if they are skipping authorisation policy creation. This is expected, since there is no need to delay if the authorisation policy already exists.

Run the pipeline

If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.

Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:

/run pipeline

Checklist for reviewers

  • If relevant, a test for the change is included or updated with this PR.
  • If relevant, documentation for the change is included or updated with this PR.

For mergers

  • Use a conventional commit message to set the release level. Follow the guidelines.
  • Include information that users need to know about the PR in the commit message. The commit message becomes part of the GitHub release notes.
  • Use the Squash and merge option.

@shemau
Copy link
Contributor Author

shemau commented Jan 24, 2024

/run pipeline

@shemau
Copy link
Contributor Author

shemau commented Jan 24, 2024

To make the code more stream lined, a count was added to time_sleep.wait_for_authorization_policy so that the delay is ONLY introduced when a policy is created. Previously this resource was always created, so in the upgrade test fails because it is being deleted.

This will only be detected during this upgrade, so skip upgrade test will be required.

@shemau
Copy link
Contributor Author

shemau commented Jan 25, 2024

The destroy error is here: https://github.com/terraform-ibm-modules/terraform-ibm-cos/actions/runs/7643532125/job/20825912071#step:7:3586

It is a destroy on the time_sleep for cos_bucket2 which has skip_iam_authorization_policy set to true. So it is never going to create a policy, so it should never have had a delay in the first place.

Since this is the only change breaking the upgrade test, and it only affects the upgrade for this PR, the upgrade tests will be skipped.

@shemau
Copy link
Contributor Author

shemau commented Jan 25, 2024

/run pipeline

@shemau
Copy link
Contributor Author

shemau commented Jan 25, 2024

This failure looks like IBM-Cloud/terraform-provider-ibm#5002.

@shemau
Copy link
Contributor Author

shemau commented Jan 25, 2024

/run pipeline

@ocofaigh ocofaigh merged commit 472a353 into main Jan 26, 2024
2 checks passed
@ocofaigh ocofaigh deleted the policy-delay branch January 26, 2024 14:10
@terraform-ibm-modules-ops
Copy link
Contributor

🎉 This PR is included in version 7.2.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

3 participants