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

Feature/update resolve conflicts #192

Merged
merged 5 commits into from Oct 6, 2023
Merged

Feature/update resolve conflicts #192

merged 5 commits into from Oct 6, 2023

Conversation

tyu0912
Copy link
Contributor

@tyu0912 tyu0912 commented Aug 18, 2023

what

Updating the addon to use resolve_conflicts_on_create and resolve_conflicts_on_update.

why

Per reference below, resolve_conflicts is deprecated.

references

https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/eks_addon

@tyu0912 tyu0912 requested review from a team as code owners August 18, 2023 20:46
@tyu0912
Copy link
Contributor Author

tyu0912 commented Aug 21, 2023

@Gowiem / @korenyoni - is this PR helpful?

@tyu0912
Copy link
Contributor Author

tyu0912 commented Oct 4, 2023

@Gowiem @korenyoni?

@Gowiem
Copy link
Member

Gowiem commented Oct 4, 2023

@tyu0912 definitely helpful. Because this was an addition to the AWS provider API following a specific version of the provider, we'll need to ensure we bump the minimum version of the AWS provider as well. Can you track down that version and do that update in versions.tf?

@Gowiem
Copy link
Member

Gowiem commented Oct 4, 2023

/terratest

@tyu0912
Copy link
Contributor Author

tyu0912 commented Oct 5, 2023

Thanks @Gowiem . Done.

@tyu0912
Copy link
Contributor Author

tyu0912 commented Oct 5, 2023

/terratest

@tyu0912
Copy link
Contributor Author

tyu0912 commented Oct 5, 2023

Ah, that command doesn't work me. Will leave it to you guys then. Let me know if there's anything else.

Command 'terratest' is not configured for the user's permission level 'none'.

@Gowiem
Copy link
Member

Gowiem commented Oct 5, 2023

/terratest

@Gowiem
Copy link
Member

Gowiem commented Oct 5, 2023

@tyu0912 Please run the following commands and commit the changes

make init
make github/init
make readme

@tyu0912
Copy link
Contributor Author

tyu0912 commented Oct 6, 2023

@Gowiem , Done.

@Gowiem
Copy link
Member

Gowiem commented Oct 6, 2023

/terratest

@Gowiem
Copy link
Member

Gowiem commented Oct 6, 2023

@tyu0912 this looks good from my end, but because we're fixing a v5 deprecation and bumping the required provider to be v5 this would mean we should do a major version rev. I'm not sure if we actually want to do that at this time, so I am polling the rest of the contributor team to make sure that we're all on the same page before pulling the trigger here. I'll circle back here with a status update once we've discussed. Thanks for the patience!

@tyu0912
Copy link
Contributor Author

tyu0912 commented Oct 6, 2023

No problem. Thanks for all the help and putting this on the radar @Gowiem . My team and I are very much looking forward to this update and can understand the need for planning as well. Looking forward to the release when it happens. Let me know if there's anything else I can do to support.

@aknysh aknysh added the major Breaking changes (or first stable release) label Oct 6, 2023
Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

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

thanks @tyu0912

@aknysh aknysh merged commit bf58df1 into cloudposse:main Oct 6, 2023
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major Breaking changes (or first stable release)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants