Added conjure-up support. #107

Merged
merged 5 commits into from Mar 28, 2017

Conversation

Projects
None yet
4 participants
Collaborator

petevg commented Mar 23, 2017

This is basic support for calling out to conjure-up on the command line,
and having it handle the deploy. It requires that you have a version of
conjure-up in your environment that has the patch referenced in
conjure-up/conjure-up#760

Note that this only works if you point matrix at a spell.

@johnsca @seman @abentley This is ready for review!

petevg added some commits Mar 23, 2017

[WIP] Added conjure-up support.
This is basic support for calling out to conjure-up on the command line,
and having it handle the deploy. It requires that you have a version of
conjure-up in your environment that has the patch referenced in
conjure-up/conjure-up#760

Note that this only works if you point matrix at a spell.
Added test coverage for conjure-up deploys.
Added a functional test, and a bit of unit testing.

Also took a moment to improve the functional testing logging. Functional
tests no longer sit there silently staring at you while they do stuff!

@petevg petevg changed the title from [WIP] Added conjure-up support. to Added conjure-up support. Mar 24, 2017

Collaborator

petevg commented Mar 24, 2017

Bleh. Looks like I don't know how to install snapd on trusty. @johnsca any advice on tweaking .travis.yaml so that it gives me a working conjure-up snap?

petevg added some commits Mar 24, 2017

Skip conjure-up functional test if conjure-up not installed.
Gets around conjure-up issue in Travis.
Collaborator

petevg commented Mar 24, 2017

Got tests to pass by just adding a conditional skipptest around the conjure-up functional test. It works in my local environment, and it will magically start working when/if we figure out how to install snaps in the travis env.

@petevg petevg referenced this pull request Mar 27, 2017

Closed

Resources in testing #102

Pretty much seems fine to me, but I included a few comments below.

matrix/main.py
+ " Note that if you choose conjure-up, --path "
+ "should point to a conjure-up spell "
+ "(with metadata.yaml) "
+ "instead of to a vanilla juju bundle.")
@johnsca

johnsca Mar 27, 2017

Owner

Could we auto-detect this instead of requiring it to be explicit?

@petevg

petevg Mar 28, 2017

Collaborator

@johnsca The tricky bit about doing it automatically is that a spell is basically just a bundle with a metadata.yaml file in it. Branching just on there being (or not being) a metadata.yaml felt iffy, especially as the conjure-up docs were vague about what exactly needed to be in the file for it to be a legit spell.

I went with the arg, though technically I'd be happier with automagic.

@mikemccracken

mikemccracken Mar 28, 2017

A spell is a directory with a metadata.yaml, and either a 'bundle-location' key in that file, or a separate bundle.yaml file next to it. The only required keys in metadata.yaml are 'friendly-name' and 'version'. I think just checking for a metadata.yaml with a 'friendly-name' key would do for matrix's purposes, but maybe I'm missing something?

Also - line 137 has a typo - "congure-up".

@battlemidget

battlemidget Mar 28, 2017

Yea every spell at least has to have a metadata.yaml file with a friendly-name attribute so you could check for that automatically

@petevg

petevg Mar 28, 2017

Collaborator

@mikemccracken @battlemidget I added logic (which I will pushed shortly) so that we detect whether or not something is a spell based on it having a metadata.yaml with a friendly-name attribute (version doesn't seem to be required). Thx for the clarification on what is and isn't a spell.

matrix/tasks/deploy.py
+ controller_id = controller_id[11:]
+
+ success, controllers, _ = await execute_process(
+ ["juju", "list-controllers", "--format", "yaml"],
@petevg

petevg Mar 28, 2017

Collaborator

@johnsca Yes. That's why I was asking about the best way to do it in python-libjuju. You told me there wasn't a way, which I suppose means that I should read the code, rather than trusting you :-p

@johnsca

johnsca Mar 28, 2017

Owner

I thought you meant a way to do it via the API. The JujuData helper does more or less what you're doing, though I think it reads the yaml files directly.

tests/functional/spell_test.py
+
+ if not find_executable("conjure-up"):
+ test_basic_spell = unittest.skip("Conjure up not installed")(
+ test_basic_spell)
@johnsca

johnsca Mar 27, 2017

Owner

This would be easier to read if it used @unittest.skipIf. I'd even recommend decorating the entire class, to avoid any expensive setup.

@petevg

petevg Mar 28, 2017

Collaborator

Hey cool. I didn't know that skipIf was a thing. That makes everything simpler :-)

Addressed PR comments.
conjure-up deploy now works via automagic.

@petevg petevg merged commit 7ea60cf into master Mar 28, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment