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

feat(examples): add azure-ark example #107

Merged
merged 6 commits into from Jan 16, 2019

Conversation

vdice
Copy link
Member

@vdice vdice commented Jan 2, 2019

Adds Porter example utilizing Azure Storage and a Helm-based Ark installation

Uses porter-azure mixin functionality PR'ed in getporter/arm-mixin#6

For development/testing, I built the porter-azure mixin binaries (from branch above) and copied them into the default porter location before a porter build, duffle install, etc.:

cd path/to/porter-azure && make build && cp bin/mixins/azure/azure* path/to/porter/bin/mixins/azure/
cd path/to/porter/examples/azure-ark && porter build
duffle bundle sign -f bundle.json
duffle creds generate ark-creds porter-azure-ark
duffle install ark porter-azure-ark -c ark-creds

TODOs (assuming PR a candidate for merge):

  • update image repo to deislabs

examples/azure-ark/porter.yaml Outdated Show resolved Hide resolved
examples/azure-ark/porter.yaml Outdated Show resolved Hide resolved
examples/azure-mysql-wordpress/porter.yaml Show resolved Hide resolved
Copy link
Member

@carolynvs carolynvs left a comment

Choose a reason for hiding this comment

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

Oops, meant to make this a comment, not approve.

@vdice
Copy link
Member Author

vdice commented Jan 8, 2019

One note: I figured since the porter-azure mixin PR that this example depends on (getporter/arm-mixin#6) was merged, I would be able to make build and get the latest; however, the latest still doesn't have these changes, due to a combo of the following issues:

  1. The make targets invoked by build that correspond to actual mixin filenames (e.g. bin/mixins/azure/azure) won't run if the namesake file already exists (as targets aren't PHONY). Perhaps we can revisit how we want to manage pulling in latest (external) mixin dependencies at time of build (and/or, judging from the issue queue, it appears this mixin management may migrate to porter's domain). Anyways, for my purposes, no matter: workaround is to rm these files before a subsequent make build

  2. It seems the porter-azure mixin from the most recent master commit may not have been published or was not published to the latest URL used by the Makefile in this repo for fetching. Therefore, the latest mixin binaries fetched apparently aren't truly the most recent.

Wanted to make sure we're aware of these two gotchas if not already...

@carolynvs
Copy link
Member

It seems the porter-azure mixin from the most recent master commit may not have been published or was not published to the latest URL

The way the builds are setup here's what get published when:

  • canary: tip of master
  • vX.Y.Z: tagged commits
  • latest: most recent tagged commit pushed

So we can either tag a commit on porter-azure, then porter will pick it up. Or we can say that really porter should be relying on canary (tip of master) from porter-azure in its makefile, not latest (last tag).

What do you think?

@carolynvs
Copy link
Member

Anyways, for my purposes, no matter: workaround is to rm these files before a subsequent make build

I didn't want to re-download the mixins on every build. Suggestions welcome on how to manage that better. I do have plans to make it easier to install plugins for users, but didn't have much on the backlog for managing this for ourselves (beyond what you just did, rm and redo the build).

How about clean should remove the cached mixins?

This was all put together 6 hours before KubeCon in the hotel lounge when we found out that we were going to show porter to people, so don't assume anything was by design. 😉

@carolynvs
Copy link
Member

@vdice Thank you for trying this! Please take all my replies as happy, confused attempts to help. 😀

@carolynvs
Copy link
Member

I've turned my comment explaining the release tags into documentation on the readme. #110

@vdice
Copy link
Member Author

vdice commented Jan 8, 2019

Ah, thank you @carolynvs for the clarification; indeed, I'm not surprised to hear both cases had been considered and that the defaults make sense!

So we can either tag a commit on porter-azure, then porter will pick it up. Or we can say that really porter should be relying on canary (tip of master) from porter-azure in its makefile, not latest (last tag).

This totally makes sense -- we might not want porter to grab the very latest (canary here). I agree, I think porter should grab the latest release of the azure mixin that we're confident in -- so, the first route of tagging when ready, to trigger the proper publishing. Good call.

How about clean should remove the cached mixins?

I agree here, too -- when playing around with PHONY-izing, indeed, it didn't feel right to re-fetch external dependencies on every build. I think just adding a few lines to make clean sounds perfect! I'll PR. edit: make clean alreadyrm's /bin so we're good!

@carolynvs
Copy link
Member

MAGIC! 🎩 ✨

Let me know when you feel good about a tag on porter-azure. In the meantime you can manually grab the canary binaries and use them locally to test. Maybe we need a new makefile target to make that easier to do?

@vdice
Copy link
Member Author

vdice commented Jan 9, 2019

Let me know when you feel good about a tag on porter-azure

In my testing of this example, master branch of porter-azure looks good (newly-added storage template/functionality working swimmingly). If test automation (test-cli in this repo -- other?) agrees, I'd say let's tag it!

@vdice vdice added the review label Jan 16, 2019
@ghost ghost assigned jeremyrickard Jan 16, 2019
Copy link
Contributor

@jeremyrickard jeremyrickard left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the example!

@jeremyrickard jeremyrickard merged commit a34870e into getporter:master Jan 16, 2019
@ghost ghost removed the review label Jan 16, 2019
@vdice vdice deleted the feat/examples-azure-ark branch January 16, 2019 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants