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

makefile: integrate xpkg publishing process #142

Merged
merged 4 commits into from
Sep 27, 2022

Conversation

muvaf
Copy link
Member

@muvaf muvaf commented Sep 26, 2022

Description of your changes

This adds the consumption of xpkg build lib so that we push XPKG to Upbound marketplace. It needs XPKG_ACCESS_ID and XPKG_TOKEN secret vars that can push to xpkg.upbound.io/crossplane but I don't have access to that org @hasheddan

I borrowed most of the logic from https://github.com/crossplane-contrib/provider-aws/blob/master/Makefile

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable to ensure this PR is ready for review.

How has this code been tested

make -j2 build.all and see a multi-arch xpkg is built. e2e-test target of the PR.

…Upbound marketplace, too

Signed-off-by: Muvaffak Onus <[email protected]>
Copy link
Member

@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.

@muvaf thanks for updating this! We are going to need some more changes for it to work fully. crossplane-contrib/provider-aws#1460 may be useful to model after

@muvaf
Copy link
Member Author

muvaf commented Sep 26, 2022

The integration test fails with the following event on Release:

  Warning  CannotCreateExternalResource  2s (x8 over 27s)  managed/release.helm.crossplane.io  (combined from similar events): failed to install release: failed to pull chart: looks like "https://charts.bitnami.com/bitnami" is not a valid chart repository or cannot be reached: open /.cache/helm/repository/Bo0QkEonMrLNTmMqzq8ah5GbsCc=-index.yaml: no such file or directory

@hasheddan @turkenh does this ring a bell? It works as expected when I run the controller locally with make run.

@muvaf
Copy link
Member Author

muvaf commented Sep 26, 2022

Alright the problem turned out to be that distroless image didn't allow Helm to call mkdir -p on a root level folder, i.e. /.cache since $HOME is empty. But the error is very misleading since Helm code didn't have error handling helm/helm#11388

Another thing is that bitnami wordpress image does not yet support ARM apparently bitnami/charts#7305

Makefile Outdated
@@ -2,7 +2,6 @@
PROJECT_NAME := provider-helm
PROJECT_REPO := github.com/crossplane-contrib/$(PROJECT_NAME)

PLATFORMS ?= linux_amd64 linux_arm64
Copy link
Member

Choose a reason for hiding this comment

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

@muvaf does this need to be dropped?

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't see it in provider-aws and thought we probably don't need it here either. Is there a reason to keep it? The default in submodule seems to include darwin and windows as well.

Copy link
Member

Choose a reason for hiding this comment

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

I believe that it is present in all providers (example in provider-aws). The only issue it would cause is in image construction, but it looks like it is being skipped correctly. I think building the extra binaries here is not really needed, but I'm not strongly against it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh I missed that. I'd prefer not to deviate, reverted that part. Thanks!

Copy link
Member

@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.

@muvaf I'll let you decide about the arch question 👍🏻 Thanks for your work here!

try to create /.cache folder which is not allowed in distroless.

Signed-off-by: Muvaffak Onus <[email protected]>
@muvaf muvaf merged commit b368192 into crossplane-contrib:master Sep 27, 2022
@muvaf muvaf deleted the integrate-xpkg branch September 27, 2022 07:17
@github-actions
Copy link

Successfully created backport PR #143 for release-0.11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants