Merge lp://staging/~ev/ubuntu-ci-services-itself/better-structure-and-logging into lp://staging/ubuntu-ci-services-itself

Proposed by Evan
Status: Needs review
Proposed branch: lp://staging/~ev/ubuntu-ci-services-itself/better-structure-and-logging
Merge into: lp://staging/ubuntu-ci-services-itself
Diff against target: 1763 lines (+1264/-121)
22 files modified
charms/precise/gunicorn/README.md (+52/-0)
charms/precise/gunicorn/config.yaml (+81/-0)
charms/precise/gunicorn/copyright (+17/-0)
charms/precise/gunicorn/hooks/hooks.py (+492/-0)
charms/precise/gunicorn/icon.svg (+377/-0)
charms/precise/gunicorn/metadata.yaml (+15/-0)
charms/precise/gunicorn/revision (+1/-0)
charms/precise/gunicorn/templates/upstart.tmpl (+34/-0)
charms/precise/python-django/hooks/hooks.py (+67/-46)
charms/precise/python-django/templates/settings.tmpl (+37/-6)
charms/precise/python-django/templates/urls.tmpl (+9/-0)
juju-deployer/branch-source-builder.yaml.tmpl (+0/-1)
juju-deployer/deploy.py (+2/-1)
juju-deployer/image-builder.yaml.tmpl (+0/-1)
juju-deployer/lander.yaml.tmpl (+0/-1)
juju-deployer/ppa-assigner.yaml.tmpl (+10/-3)
juju-deployer/production-only.yaml (+1/-1)
juju-deployer/test-runner.yaml.tmpl (+0/-1)
juju-deployer/ticket-system.yaml.tmpl (+12/-5)
ppa-assigner/ppa_assigner/ppa_sync.py (+5/-2)
ppa-assigner/ppa_assigner/settings.py (+30/-28)
ticket_system/ticket_system/settings.py (+22/-25)
To merge this branch: bzr merge lp://staging/~ev/ubuntu-ci-services-itself/better-structure-and-logging
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Evan (community) Needs Resubmitting
Andy Doan (community) Approve
Chris Johnston (community) Needs Fixing
Review via email: mp+208581@code.staging.launchpad.net

Commit message

- Teach gunicorn to properly write gunicorn access and error logs under /srv/${charm_name}/logs/gunicorn-{access,error}.log.
- Store all the Django configuration under /srv/${charm_name}/conf and keep it read-only.
- Store lp:ubuntu-ci-services-itself under /srv/${charm_name}/code and keep it read-only.
- Store variable data under /srv/${charm_name}/var and set it read-write.
- Log Django errors to /srv/${charm_name}/logs/django.log.

Description of the change

Deploy django/gunicorn with the following structure:

/srv/${charm_name}/{logs,code,conf}/

- Teach gunicorn to properly write gunicorn access and error logs under /srv/${charm_name}/logs/gunicorn-{access,error}.log.
- Store all the Django configuration under /srv/${charm_name}/conf and keep it read-only.
- Store lp:ubuntu-ci-services-itself under /srv/${charm_name}/code and keep it read-only.
- Log Django errors to /srv/${charm_name}/logs/django.log.

We can later add /srv/${charm_name}/var as needed for runtime data.

My intention is to move onto rabbitmq-worker and the other charms to provide the same structure. We should have a consistent place to look when problems occur.

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

FAILED: Continuous integration, rev:295
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/~ev/ubuntu-ci-services-itself/better-structure-and-logging/+merge/208581/+edit-commit-message

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

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

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:297
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/263/
Executed test runs:

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

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

1402 +# Ticket System
1403 +BASE_IMAGE_DEFAULT = _cfg.get('base_image', '')
1404 +SERIES_DEFAULT = _cfg.get('series', '')
1405 +MASTER_PPA_DEFAULT = _cfg.get('master_ppa', '')
1406 +
1407 +# PPA Assigner
1408 +LAUNCHPAD_PPA_USER = _cfg.get('launchpad_user', None)
1409 +LAUNCHPAD_API_BASE = _cfg.get('launchpad_api_base',
1410 + 'https://api.launchpad.net/1.0')
1411 +OAUTH_CONSUMER_KEY = _cfg.get('oauth_consumer_key', None)
1412 +OAUTH_TOKEN = _cfg.get('oauth_token', None)
1413 +OAUTH_TOKEN_SECRET = _cfg.get('oauth_token_secret', None)
1414 +OAUTH_REALM = _cfg.get('oauth_realm', 'https://api.launchpad.net/')
1415 +PPA_PATTERN = _cfg.get('ppa_pattern', r'ci-pool-\d+')

do we really want this stuff in our charm? its already in the components settings.py file and just seems like it will lead to confusion if we ever need to add/remove/change one of these settings.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:297
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/272/
Executed test runs:

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Chris Johnston (cjohnston) wrote :

+ /home/ubuntu/jenkins/workspace/uci-engine-ci/uci-engine-autolanding/ppa-assigner/ppa_assigner/settings.py:16: 'glob' imported but unused

This one potentially looks valid

+ /home/ubuntu/jenkins/workspace/uci-engine-ci/uci-engine-autolanding/ppa-assigner/ppa_assigner/settings.py:150: 'from local_settings import *' used; unable to detect undefined names
+ /home/ubuntu/jenkins/workspace/uci-engine-ci/uci-engine-autolanding/ppa-assigner/ppa_assigner/settings.py:155: 'DATABASES' imported but unused

These two I think we should setup ignores for.

review: Needs Fixing
Revision history for this message
Evan (ev) wrote :

> do we really want this stuff in our charm? its already in the components
> settings.py file and just seems like it will lead to confusion if we ever
> need to add/remove/change one of these settings.

Yeah, I'm not happy about this either. Let me see what I can do.

Revision history for this message
Evan (ev) wrote :

> + > /home/ubuntu/jenkins/workspace/uci-engine-ci/uci-engine-autolanding/ppa-assigner/ppa_assigner/settings.py:16:
> 'glob' imported but unused

Fixed as r299.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:298
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/323/
Executed test runs:

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

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:299
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/324/
Executed test runs:

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

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:300
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/326/
Executed test runs:

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

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

Just nits - nothing worth not merging over:

django hook.py
1131 - p = 'PYTHONPATH=%s' % config_data.get('python_path', install_root)
1132 + def_path = '%s:%s' % (config_dir, code_dir)
1133 + p = 'PYTHONPATH=%s' % config_data.get('python_path', def_path)

1269 - relation_set({'python_path': install_root})
1270 + relation_set({'python_path': '%s:%s' % (config_dir, code_dir)})

I think the relation_set should use the same logic as line 1133 uses?

ppa-assigner.yaml:
1468 + python_path: /srv/ppa_django/conf:/srv/ppa_django/code/ci-utils:/srv/ppa_django/code/ppa-assigner

Is this needed now, or should the relation_set (now that its been updated) handle this setting properly.

Untested by me.

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

i'm getting a deployer issue. not sure the cause yet:

2014-03-07 11:04:45 Branching charm lp:~canonical-ci-engineering/charms/precise/ubuntu-ci-services-itself/rabbitmq-server @ /tmp/better-structure-and-logging/charms/precise/rabbitmq
Traceback (most recent call last):
  File "/tmp/better-structure-and-logging/branches/juju-deployer/deployer/cli.py", line 233, in <module>
    main()
  File "/tmp/better-structure-and-logging/branches/juju-deployer/deployer/cli.py", line 127, in main
    run()
  File "/tmp/better-structure-and-logging/branches/juju-deployer/deployer/cli.py", line 225, in run
    importer.Importer(env, deployment, options).run()
  File "/tmp/better-structure-and-logging/branches/juju-deployer/deployer/action/importer.py", line 182, in run
    self.get_charms()
  File "/tmp/better-structure-and-logging/branches/juju-deployer/deployer/action/importer.py", line 63, in get_charms
    no_local_mods=self.options.no_local_mods)
  File "/tmp/better-structure-and-logging/branches/juju-deployer/deployer/deployment.py", line 127, in fetch_charms
    if charm.is_modified():
  File "/tmp/better-structure-and-logging/branches/juju-deployer/deployer/charm.py", line 152, in is_modified
    return self.vcs.is_modified()
  File "/tmp/better-structure-and-logging/branches/juju-deployer/deployer/vcs.py", line 85, in is_modified
    tree = WorkingTree.open(self.path)
  File "/usr/lib/python2.7/dist-packages/bzrlib/workingtree.py", line 280, in open
    control = controldir.ControlDir.open(path, _unsupported=_unsupported)
  File "/usr/lib/python2.7/dist-packages/bzrlib/controldir.py", line 689, in open
    _unsupported=_unsupported)
  File "/usr/lib/python2.7/dist-packages/bzrlib/controldir.py", line 718, in open_from_transport
    find_format, transport, redirected)
  File "/usr/lib/python2.7/dist-packages/bzrlib/transport/__init__.py", line 1719, in do_catching_redirections
    return action(transport)
  File "/usr/lib/python2.7/dist-packages/bzrlib/controldir.py", line 706, in find_format
    probers=probers)
  File "/usr/lib/python2.7/dist-packages/bzrlib/controldir.py", line 1155, in find_format
    raise errors.NotBranchError(path=transport.base)
bzrlib.errors.NotBranchError: Not a branch: "/tmp/better-structure-and-logging/charms/precise/gunicorn/".

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:301
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/330/
Executed test runs:

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

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:302
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/331/
Executed test runs:

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

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:303
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/335/
Executed test runs:

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Evan (ev) wrote :

Fixed the deployer issue as r303.

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

thanks for the updates

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

the django charms are broke now during the postgres relation join:

2014-03-07 20:55:55 INFO juju-log pgsql:28: Running pgsql-relation-changed hook
2014-03-07 20:55:56 DEBUG juju-log pgsql:28: PYTHONPATH=/srv/ts_django/conf:/srv/ts_django/code/ci-utils:/srv/ts_django/code/ticket_system /usr/bin/django-admin syncdb --noinput --settings=ticket_system.settings
2014-03-07 20:55:57 ERROR juju-log pgsql:28: status=1, output=usage: django-admin [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
   or: django-admin --help [cmd1 cmd2 ...]
   or: django-admin --help-commands
   or: django-admin cmd --help

error: invalid command 'syncdb'

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

Another bug i'm seeing is with unit_config:

  Unable to use unit_config(../unit_config), defaulting values

I think our settings.py files don't know how to locate this under its new location.

Revision history for this message
Evan (ev) wrote :

Man, this is just fail all over the place. Doing a deploy now to track
these down.

Revision history for this message
Chris Johnston (cjohnston) wrote :
Download full text (101.6 KiB)

The attempt to merge lp:~ev/ubuntu-ci-services-itself/better-structure-and-logging into lp:ubuntu-ci-services-itself failed. Below is the output from the failed tests.

New python executable in /tmp/tmp.9PczZfUOrt/bin/python
Installing distribute.............................................................................................................................................................................................done.
Installing pip...............done.
== Testing ci-utils ....
Unpacking /tmp/tarmac/branch.KOOFkz/.deps/Babel-1.3.tar.gz
  Running setup.py egg_info for package from file:///tmp/tarmac/branch.KOOFkz/.deps/Babel-1.3.tar.gz

    warning: no previously-included files matching '*' found under directory 'docs/_build'
    warning: no previously-included files matching '*.pyc' found under directory 'tests'
    warning: no previously-included files matching '*.pyo' found under directory 'tests'
Installing collected packages: Babel
  Running setup.py install for Babel

    warning: no previously-included files matching '*' found under directory 'docs/_build'
    warning: no previously-included files matching '*.pyc' found under directory 'tests'
    warning: no previously-included files matching '*.pyo' found under directory 'tests'
    Installing pybabel script to /tmp/tmp.9PczZfUOrt/bin
Successfully installed Babel
Cleaning up...
Using saved parent location: bzr+ssh://bazaar.launchpad.net/~canonical-ci-engineering/ubuntu-ci-services-itself/deps/
No revisions or tags to pull.
Unpacking /tmp/tarmac/branch.KOOFkz/.deps/pbr-0.6.tar.gz
  Running setup.py egg_info for package from file:///tmp/tarmac/branch.KOOFkz/.deps/pbr-0.6.tar.gz
    [pbr] Processing SOURCES.txt
    warning: LocalManifestMaker: standard file '-c' not found

    warning: no previously-included files found matching '.gitignore'
    warning: no previously-included files found matching '.gitreview'
    warning: no previously-included files matching '*.pyc' found anywhere in distribution
    warning: no previously-included files found matching '.gitignore'
    warning: no previously-included files found matching '.gitreview'
    warning: no previously-included files matching '*.pyc' found anywhere in distribution
Installing collected packages: pbr
  Running setup.py install for pbr
    [pbr] Reusing existing SOURCES.txt
Successfully installed pbr
Cleaning up...
Using saved parent location: bzr+ssh://bazaar.launchpad.net/~canonical-ci-engineering/ubuntu-ci-services-itself/deps/
No revisions or tags to pull.
Unpacking /tmp/tarmac/branch.KOOFkz/.deps/iso8601-0.1.8.tar.gz
  Running setup.py egg_info for package from file:///tmp/tarmac/branch.KOOFkz/.deps/iso8601-0.1.8.tar.gz

Installing collected packages: iso8601
  Running setup.py install for iso8601

Successfully installed iso8601
Cleaning up...
Using saved parent location: bzr+ssh://bazaar.launchpad.net/~canonical-ci-engineering/ubuntu-ci-services-itself/deps/
No revisions or tags to pull.
Unpacking /tmp/tarmac/branch.KOOFkz/.deps/prettytable-0.7.2.zip
  Running setup.py egg_info for package from file:///tmp/tarmac/branch.KOOFkz/.deps/prettytable-0.7.2.zip

Installing collected package...

304. By Evan

Fix setting CONFDIR and LOGDIR.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:304
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/338/
Executed test runs:

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

review: Needs Fixing (continuous-integration)
305. By Evan

Safely write the lpcreds file. Put it under a writable directory.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:305
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/341/
Executed test runs:

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

review: Needs Fixing (continuous-integration)
306. By Evan

Actually setting the VARDIR would help.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:306
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/342/
Executed test runs:

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

review: Needs Fixing (continuous-integration)
307. By Evan

Missing import.

308. By Evan

Include timestamps in django log messages.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:307
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/343/
Executed test runs:

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

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:308
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/344/
Executed test runs:

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Evan (ev) wrote :

Finally!

  % curl http://15.125.108.29:8080/api/v1/status/\?format\=json
[{"label": "launchpad configured", "status": "okay", "value": true}, {"label": "total ppas", "status": "okay", "value": 2}, {"label": "available ppas", "status": "okay", "value": 2}]

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

This is close, but I just noticed the ppa-cleaner isn't running:

Unable to use unit_config(/srv/ppa_django/code/ppa-assigner/../unit_config), defaulting values
ERROR: oauth settings are required by this command:
  OAUTH_CONSUMER_KEY
  OAUTH_TOKEN
  OAUTH_TOKEN_SECRET

I think this is because this type of deployment requires CONFDIR to get get set. Is CONFDIR a gunicorn specific thing or something, I'm not sure where that gets set at?

Revision history for this message
Evan (ev) wrote :

On 10 March 2014 03:06, Andy Doan <email address hidden> wrote:
> This is close, but I just noticed the ppa-cleaner isn't running:
>
> Unable to use unit_config(/srv/ppa_django/code/ppa-assigner/../unit_config), defaulting values
> ERROR: oauth settings are required by this command:
> OAUTH_CONSUMER_KEY
> OAUTH_TOKEN
> OAUTH_TOKEN_SECRET
>
> I think this is because this type of deployment requires CONFDIR to get get set. Is CONFDIR a gunicorn specific thing or something, I'm not sure where that gets set at?

CONFDIR is something I invented to let us set a variable configuration
path rather than having it hardcoded everywhere, which would've made
our charms not mergeable with trunk. I think we'll need to teach the
upstart charm to support custom stanzas or specifically the
environment variables to set. We can then tell the ppa-cleaner to try
looking under CONFIDR for the unit_config first.

My rationale is this: we shouldn't be deploying configuration on top
of code. That's a recipe for disaster.

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

> CONFDIR is something I invented to let us set a variable configuration
> path rather than having it hardcoded everywhere, which would've made
> our charms not mergeable with trunk. I think we'll need to teach the
> upstart charm to support custom stanzas or specifically the
> environment variables to set. We can then tell the ppa-cleaner to try
> looking under CONFIDR for the unit_config first.
>
> My rationale is this: we shouldn't be deploying configuration on top
> of code. That's a recipe for disaster.

Another thing we could do (take your pick) would be to add a little more
logic to our settings.py module like:

try:
     # This should get set by local_settings.
     path = os.path.join(CONFDIR, '../unit_config')
except NameError:
     path = os.path.join(BASEDIR, '../unit_config')
     if not os.path.exists(path):
         print('hmm, maybe its one level up...')
         path = os.path.join(BASEDIR, '../../conf/unit_config')

or just throw in the towel and do:

  subprocess.check_out(['find', '/', '-name', unit_config])

:)

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:308
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/402/
Executed test runs:

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

review: Needs Fixing (continuous-integration)

Unmerged revisions

308. By Evan

Include timestamps in django log messages.

307. By Evan

Missing import.

306. By Evan

Actually setting the VARDIR would help.

305. By Evan

Safely write the lpcreds file. Put it under a writable directory.

304. By Evan

Fix setting CONFDIR and LOGDIR.

303. By Evan

How'd this sneak in here. Gunicorn lives in the branch for now.

302. By Evan

Merge with trunk.

301. By Evan

Set the PYTHONPATH appropriately on the relation.

300. By Evan

Move the parts specific to the PPA Assigner and Ticket System back into those components. This code need not live in the python-django charm so long as we can pass a reference to the config file root.

299. By Evan

Unused import. Thanks Chris.

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

to all changes: