Ubuntu CI Services

Merge lp://staging/~ursinha/ubuntu-ci-services-itself/add-bootstrap-check into lp://staging/ubuntu-ci-services-itself

  1. add-bootstrap-check
  2. Merge into trunk
Proposed by Ursula Junque
Status: Merged
Approved by: Chris Johnston
Approved revision: 242
Merged at revision: 243
Proposed branch: lp://staging/~ursinha/ubuntu-ci-services-itself/add-bootstrap-check
Merge into: lp://staging/ubuntu-ci-services-itself
Diff against target: 80 lines (+36/-2)
2 files modified
juju-deployer/deploy.py (+21/-2)
juju-deployer/test_deploy.py (+15/-0)
To merge this branch: bzr merge lp://staging/~ursinha/ubuntu-ci-services-itself/add-bootstrap-check
Reviewer Review Type Date Requested Status
Andy Doan (community) Approve
PS Jenkins bot (community) continuous-integration Needs Fixing
Review via email: mp+206815@code.staging.launchpad.net

Commit message

Checks if the environment is bootstrapped as it's required to deploy

Description of the change

When running juju-deployer/deploy.py and the environment isn't bootstrapped, juju-deployer throws an error that says nothing about the real failure (CalledProcessError saying command juju-deployer returned with nonzero status). This branch adds a check if juju environment is bootstrapped before performing all the branching and pre-processing that happens before the deployment. Makes easier to understand what's going on in case deploy.py fails, and avoids execution of steps that will be ignored in case there's no bootstrapped juju environment.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:242
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~ursinha/ubuntu-ci-services-itself/add-bootstrap-check/+merge/206815/+edit-commit-message

http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/162/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/162/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Andy Doan (doanac) wrote :

I seem to recall evan had a way to do this in tests/run.py that didn't
require calling juju status (which might take a while to timeout if
sshuttle isn't running)?

Revision history for this message
Ursula Junque (ursinha) wrote :

> I seem to recall evan had a way to do this in tests/run.py that didn't
> require calling juju status (which might take a while to timeout if
> sshuttle isn't running)?

Yes, you are correct, it takes a while for it to timeout if sshuttle isn't running (I added a comment in the code acknowledging that).

I originally created this branch because I was tired of guessing what was wrong when juju-deployer error message was only "returned with non-zero status". I've seen the code in tests/run.py, I think the "way to do this" you mention is needs_bootstrap in tests/run.py, which is a bit more sophisticated than the simple check I'm performing right now. It seemed a bit too much to accomplish what I wanted here but maybe we should just go ahead and make the same validations in both places, as we need similar setups to be able to deploy and test.

Revision history for this message
Ursula Junque (ursinha) wrote :

> > I seem to recall evan had a way to do this in tests/run.py that didn't
> > require calling juju status (which might take a while to timeout if
> > sshuttle isn't running)?
>
> Yes, you are correct, it takes a while for it to timeout if sshuttle isn't
> running (I added a comment in the code acknowledging that).

I meant, you are correct, there's a way to do this in tests/run.py that doesn't call juju status.

>
> I originally created this branch because I was tired of guessing what was
> wrong when juju-deployer error message was only "returned with non-zero
> status". I've seen the code in tests/run.py, I think the "way to do this" you
> mention is needs_bootstrap in tests/run.py, which is a bit more sophisticated
> than the simple check I'm performing right now. It seemed a bit too much to
> accomplish what I wanted here but maybe we should just go ahead and make the
> same validations in both places, as we need similar setups to be able to
> deploy and test.

Revision history for this message
Andy Doan (doanac) wrote :

> I originally created this branch because I was tired of guessing what was
> wrong when juju-deployer error message was only "returned with non-zero
> status". I've seen the code in tests/run.py, I think the "way to do this" you
> mention is needs_bootstrap in tests/run.py, which is a bit more sophisticated
> than the simple check I'm performing right now. It seemed a bit too much to
> accomplish what I wanted here but maybe we should just go ahead and make the
> same validations in both places, as we need similar setups to be able to
> deploy and test.

That's a fair point. Its hard to argue with a patch that still makes things better.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches