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

Allow multiple packs images #166

Merged
merged 1 commit into from
Mar 13, 2021
Merged

Allow multiple packs images #166

merged 1 commit into from
Mar 13, 2021

Conversation

moonrail
Copy link
Contributor

Hello Maintainers,

this PR adds the option to use multiple st2 packs-images.

This is a breaking change, as value st2.packs.image is replaced by st2.packs.images. Datatype changes from dict/hash to list of dicts/hashes.

This is useful, when a lot of different packs are being used, as a single container can result in multiple GB very quickly, mostly due to virtualenvs.

When using multiple st2 pack-images, the User can reduce possible downtimes due to reduced image size of new/changed containers. Also the build process can be a lot faster, if the User is providing custom packs.

Currently there is a fail message implemented in templates/deployments.yaml to prevent Users from accidential removal of their custom st2packs-image. It triggers, when st2.packs.image is defined and not empty. This should notify all Users about this change.

In #118 & #160 a centralization of packs-specific definitions for this chart was shown. As these PRs are still being discussed for other reasons, I've decided to include these centralizations in this PR.
This should make these PRs a little smaller and more compatible to each other, once they will or will not be finalized.

@pull-request-size pull-request-size bot added the size/L PR that changes 100-499 lines. Requires some effort to review. label Dec 23, 2020
@moonrail
Copy link
Contributor Author

Rebased to current master, should now be mergeable again :)

@arm4b arm4b added the enhancement New feature or request label Feb 25, 2021
Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

Thanks for PR! 👍 Considering its breaking change nature looks good to mix with the other major behavior changes PRs.

Can you please sync the PR with the master one more time?

@@ -1,3 +1,8 @@
# Notify users about breaking change regarding packs, to not destroy current installations
{{- if and .Values.st2.packs.image }}
{{- fail "Value st2.packs.image was renamed to st2.packs.images and is now a list of images" }}
Copy link
Member

Choose a reason for hiding this comment

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

Nice idea here! 👍

@moonrail
Copy link
Contributor Author

moonrail commented Mar 8, 2021

Hey @armab
Finally rebased and tested it again - should be mergable again :)

Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

Looks good!

Thanks a lot for contributing these changes! 👍

@arm4b arm4b merged commit 049ec74 into StackStorm:master Mar 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change enhancement New feature or request size/L PR that changes 100-499 lines. Requires some effort to review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants