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

Jump to bottom

feat: move deprecated expiry & archive blocks to new resource #769

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open

feat: move deprecated expiry & archive blocks to new resource #769

wants to merge 35 commits into from

Conversation

jor2
Copy link
Member

@jor2 jor2 commented Nov 4, 2024

Description

#720

Release required?

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

Add support for new COS bucket resource ibm_cos_lifecycle_configuration

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.

@jor2 jor2 self-assigned this Nov 4, 2024
@jor2
Copy link
Member Author

jor2 commented Nov 4, 2024

/run pipeline

@jor2
Copy link
Member Author

jor2 commented Nov 4, 2024

/run pipeline

@jor2
Copy link
Member Author

jor2 commented Nov 4, 2024

/run pipeline

@jor2
Copy link
Member Author

jor2 commented Nov 4, 2024

/run pipeline

@jor2
Copy link
Member Author

jor2 commented Nov 4, 2024

/run pipeline

@jor2
Copy link
Member Author

jor2 commented Nov 4, 2024

/run pipeline

@jor2
Copy link
Member Author

jor2 commented Nov 4, 2024

/run pipeline

@jor2
Copy link
Member Author

jor2 commented Nov 4, 2024 •
edited
Loading

Needed to skip upgrade tests

        	Test:       	TestRunUpgradeExample
        	Messages:   	Resource(s) identified to be updated 
        	            	Name: cos_bucket1
        	            	Address: module.cos_bucket3.ibm_cos_bucket.cos_bucket1[0]
        	            	Actions: [update]
        	            	DIFF:
        	            	  Before: 
        	            		{"expire_rule":"SECURE_VALUE_HIDDEN_HASH:-4cf6caddc09cb4ce559fc25363ef69efdbbe8fd2ab67b4dc2cf0ccf7"}
        	            	  After: 
        	            		{"expire_rule":"SECURE_VALUE_HIDDEN_HASH:-9b5b9d914c438f4518ad4bd062897469f1c330d2f93e1dc3b379e26d"}
Test:       	TestRunUpgradeExample
        	Messages:   	Resource(s) identified to be updated 
        	            	Name: cos_bucket
        	            	Address: module.cos_bucket1.ibm_cos_bucket.cos_bucket[0]
        	            	Actions: [update]
        	            	DIFF:
        	            	  Before: 
        	            		{"archive_rule":"SECURE_VALUE_HIDDEN_HASH:-4ca10f669917125f6f6e586a38145a8d80f4e47fe0067b6add8cead1","expire_rule":"SECURE_VALUE_HIDDEN_HASH:-010a363deb45d94eefefc5daaf0c5cacdc2839bc45e5cfb628aa3f26"}
        	            	  After: 
        	            		{"archive_rule":"SECURE_VALUE_HIDDEN_HASH:-8d40336f166ba0bfda8f89ea810d50f4f[4944](https://github.com/terraform-ibm-modules/terraform-ibm-cos/actions/runs/11672838183/job/32502518305#step:7:4945)053fbf7db9194f9d885","expire_rule":"SECURE_VALUE_HIDDEN_HASH:-613240b74a7c58b14d01d6dceaf0648aa8ee39a758f52f26adc8d646"}

Got a few of those in this run. As far as I'm aware move blocks aren't really an option since its a parameter moving to its resource.

@jor2
Copy link
Member Author

jor2 commented Nov 5, 2024 •
edited
Loading

I ran examples/basic using the latest version:

module "cos" {
  source                 = "github.com/terraform-ibm-modules/terraform-ibm-cos?ref=v8.14.2"
  ....
}

then ran using this branch:

module "cos" {
  source                 = "github.com/terraform-ibm-modules/terraform-ibm-cos?ref=migration"
  ....
}

It resulted in an upgrade in place:
Screenshot 2024-11-05 at 13 26 29

This is what happened in the UI.

It replaced both rules, or at least did a rename as you can see the lifecycle policy has changed and archive rule is still set to 90 days.

Before:
Screenshot 2024-11-05 at 13 19 06

After:
Screenshot 2024-11-05 at 13 21 15

ocofaigh
ocofaigh requested changes Nov 6, 2024
View reviewed changes
Copy link
Member

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

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

see comments

main.tf
cos_region = compact([var.region, var.cross_region_location, var.single_site_location])[0]
}

resource "time_sleep" "wait_for_cos_bucket_lifecycle" {
Copy link
Member

Choose a reason for hiding this comment

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

why do we need a sleep? Is there a timing issue? What error do you get? If so have we created a provider bug for it? If the ibm_cos_bucket has returned as complete, I expect it to be ready for lifecycle configuration. Otherwise I think its a bug. Please leave code comments when adding workaround like this explaining why they were added (and link to any associated issues)

Copy link
Member Author

Choose a reason for hiding this comment

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

there is a timing issue because after the first apply unless I waited a couple seconds to reapply it would fail saying it couldn't find the lifecycle policy. I will create a provider issue and link in a comment above sleep.

Copy link
Member Author

Choose a reason for hiding this comment

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

TestRunAdvancedExample 2024-11-06T23:58:29Z command.go:185: Planning failed. Terraform encountered an error while generating this plan.
TestRunAdvancedExample 2024-11-06T23:58:29Z command.go:185: 
TestRunAdvancedExample 2024-11-06T23:58:29Z command.go:185: ╷
Error: AdvancedExample 2024-11-06T23:58:29Z command.go:185: │ Error: [ERROR] Error getting Lifecycle Configuration for the bucket cos-advanced-91sqap-bucket-3-3hva
TestRunAdvancedExample 2024-11-06T23:58:29Z command.go:185: │ 
TestRunAdvancedExample 2024-11-06T23:58:29Z command.go:185: │   with module.cos_bucket3.ibm_cos_bucket_lifecycle_configuration.cos_bucket_lifecycle[0],
TestRunAdvancedExample 2024-11-06T23:58:29Z command.go:185: │   on ../../main.tf line 285, in resource "ibm_cos_bucket_lifecycle_configuration" "cos_bucket_lifecycle":
TestRunAdvancedExample 2024-11-06T23:58:29Z command.go:185: │  285: resource "ibm_cos_bucket_lifecycle_configuration" "cos_bucket_lifecycle" {

The error can be seen here in this run when I removed the sleep. It doesn't occur every time, but often enough.

Copy link
Member

Choose a reason for hiding this comment

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

as per internal discussions, the issue happens on the plan AFTER apply has completed. Therefor it seems a bug with the ibm_cos_bucket_lifecycle_configuration resource. If its not fully ready, it should of been marked as complete by terraform. Lets create a provider issue, and hold off fix this migration until its fixed

Copy link
Member Author

Choose a reason for hiding this comment

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

IBM-Cloud/terraform-provider-ibm#5778

main.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
@jor2
Copy link
Member Author

jor2 commented Nov 6, 2024

/run pipeline

@jor2
Copy link
Member Author

jor2 commented Nov 7, 2024

/run pipeline

@jor2 jor2 requested a review from ocofaigh November 7, 2024 13:53
ocofaigh
ocofaigh requested changes Nov 7, 2024
View reviewed changes
Copy link
Member

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

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

You will need to expose the ability to set the prefix filter in the fscloud submodule, and the DAs in the solutions folder

@jor2
Copy link
Member Author

jor2 commented Nov 8, 2024

You will need to expose the ability to set the prefix filter in the fscloud submodule, and the DAs in the solutions folder

done

@jor2 jor2 requested a review from ocofaigh November 8, 2024 13:06
@ocofaigh
Copy link
Member

On hold until IBM-Cloud/terraform-provider-ibm#5778 is resolved

@jor2
Copy link
Member Author

jor2 commented Nov 20, 2024

/run pipeline

@jor2
Copy link
Member Author

jor2 commented Nov 20, 2024

/run pipeline

@jor2
Copy link
Member Author

jor2 commented Nov 20, 2024

/run pipeline

@jor2
Copy link
Member Author

jor2 commented Nov 20, 2024

/run pipeline

@jor2
Copy link
Member Author

jor2 commented Nov 25, 2024

/run pipeline

@jor2
Copy link
Member Author

jor2 commented Nov 27, 2024

/run pipeline

@jor2
Copy link
Member Author

jor2 commented Nov 29, 2024

/run pipeline

@jor2
Copy link
Member Author

jor2 commented Dec 2, 2024

/run pipeline

@jor2
Copy link
Member Author

jor2 commented Dec 12, 2024

/run pipeline

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shemau shemau Awaiting requested review from shemau shemau is a code owner

@daniel-butler-irl daniel-butler-irl Awaiting requested review from daniel-butler-irl daniel-butler-irl is a code owner

@ocofaigh ocofaigh Awaiting requested review from ocofaigh

Requested changes must be addressed to merge this pull request.

Assignees

@jor2 jor2

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jor2 @ocofaigh