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

Upgrade LavinMQ instances #296

Merged
merged 6 commits into from
Sep 4, 2024
Merged

Upgrade LavinMQ instances #296

merged 6 commits into from
Sep 4, 2024

Conversation

viktorerlingsson
Copy link
Member

@viktorerlingsson viktorerlingsson commented Aug 30, 2024 •
edited
Loading

WHY are these changes introduced?

To upgrade LavinMQ instances.

WHAT is this pull request doing?

Adds upgrade_lavinmq resource and tests.

HOW can this pull request be tested?

Run VCR tests.

@viktorerlingsson viktorerlingsson changed the title Add upgrade_lavinmq resource WIP: Add upgrade_lavinmq resource Aug 30, 2024
@tbroden84
Copy link
Contributor

Great. We can also look into creating a fixtures for this. So the resource will get a VCR test and be tested on each commit.

@viktorerlingsson
Copy link
Member Author

viktorerlingsson commented Sep 2, 2024 •
edited
Loading

Great. We can also look into creating a fixtures for this. So the resource will get a VCR test and be tested on each commit.

Something like this? 139e110

Edit: and this 8a59686 🙂

@tbroden84
Copy link
Contributor

Looks good. Could also remove TestAccUpgradeLavinMQ_Latest test, should be the same as TestAccUpgradeLavinMQ_Specific. The latter we want to test.

@tbroden84
Copy link
Contributor

We can also remove test/configuration/insatnce_lavinmq_*.txt files and remove the default values for subscription plan in test/configuration/instance_*.txt. This will force all other tests to specify the subscription plan to and we only need to have one configuration for cloudamqp_instance resouce..

@tbroden84 tbroden84 mentioned this pull request Sep 3, 2024
Merged
@tbroden84
Copy link
Contributor

Sweet. Also found a duplicated dependency that I removed.

@viktorerlingsson viktorerlingsson marked this pull request as ready for review September 3, 2024 14:34
@viktorerlingsson viktorerlingsson changed the title WIP: Add upgrade_lavinmq resource Upgrade LavinMQ instances Sep 3, 2024
tbroden84
tbroden84 reviewed Sep 4, 2024
View reviewed changes
docs/resources/upgrade_lavinmq.md Outdated
There is two different ways to trigger the version upgrade

> - Specify LavinMQ version to upgrade to
> - Upgrade to latest LavinMQ version
Copy link
Contributor

Choose a reason for hiding this comment

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

@viktorerlingsson Do we want to support upgrade to latest for LavinMQ? Been testing around a bit and will require some more changes to support that. If not we can remove some part of the documentation describing this.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, you're right. I removed all text about upgrading to latest and re-phrased some stuff

tbroden84
tbroden84 approved these changes Sep 4, 2024
View reviewed changes
Copy link
Contributor

@tbroden84 tbroden84 left a comment

Choose a reason for hiding this comment

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

Looking good.

@viktorerlingsson viktorerlingsson merged commit 704081c into main Sep 4, 2024
2 checks passed
@viktorerlingsson viktorerlingsson deleted the lavinmq-upgrade branch September 4, 2024 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tbroden84 tbroden84 tbroden84 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@viktorerlingsson @tbroden84