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

Support for installing/upgrading charts from local files & dirs #107

Merged
merged 2 commits into from
Aug 11, 2021

Conversation

ulucinar
Copy link
Contributor

@ulucinar ulucinar commented Jul 15, 2021

Description of your changes

This PR adds support for installing/upgrading crossplane (or uxp) charts from a local chart archive or a local directory. The local chart to be processed can be specified with the --bundle command-line option.

Note: I'm planning to use the changes in this PR in the demo, and I believe it could prove to be useful for development scenarios. If this functionality is already provided by other means, or if you believe it's not appropriate, we can close this PR.

I have:

  • Read and followed Upbound's contribution process.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR, as appropriate.

How has this code been tested

Manually tested installing a downloaded UXP chart both from an archive and from a folder containing extracted contents. Also verified installations & upgrades from a Helm repo still work.

Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
Copy link
Contributor

@hasheddan hasheddan left a comment

Choose a reason for hiding this comment

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

@ulucinar thanks for working on this! We had previously been somewhat hesitant to add this type of functionality because of the potential to support other installer backends than Helm. However, I think this is a very useful feature and don't want to hold off on its implementation due to the potential of supporting something else in the future :) I left one comment below, but I was also wondering if you felt making --chart more generic, maybe something like --bundle or --src would be an improvement or just more confusing? If we kept it as --chart that is obviously specific to Helm, but the alternatives are more generic and could be supported for other installers. On the other hand, we could just say --chart is only valid if using the Helm installer. What do you think?

cmd/up/uxp/common.go Outdated Show resolved Hide resolved
- Rename "--chart" as "--bundle"
- Remove Helm from description strings

Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
@ulucinar
Copy link
Contributor Author

Hi @hasheddan,
Thanks for mentioning about the intention to hide Helm beneath. I did some changes in the wording and renamed --chart, which imposes Helm as you mentioned, as --bundle.

If, in the future, a new bundle format that we adopt does not somehow support installation/upgrades from a local path, then we may have to remove this flag but I think this is fair.

Copy link
Contributor

@hasheddan hasheddan 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 adding this awesome functionality @ulucinar and my apologies for the delayed review / merge. Excited to get this released to folks :)

@hasheddan hasheddan merged commit b812d4c into upbound:main Aug 11, 2021
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.

None yet

2 participants