Merge lp:~bac/lpsetup/mv-initialize-lxc into lp:lpsetup
| Status: | Merged |
|---|---|
| Approved by: | Brad Crittenden on 2012-07-10 |
| Approved revision: | 57 |
| Merged at revision: | 49 |
| Proposed branch: | lp:~bac/lpsetup/mv-initialize-lxc |
| Merge into: | lp:lpsetup |
| Diff against target: | 0 lines |
| To merge this branch: | bzr merge lp:~bac/lpsetup/mv-initialize-lxc |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Benji York (community) | code | 2012-07-10 | Approve on 2012-07-10 |
|
Review via email:
|
|||
Commit Message
Move most of initialize_lxc to init-host. Rename lxc-install to install-lxc. Add commands.rst. Updated the docstrings, and thus the help strings, for the subcommands. NOTE: finish-init-host and install-lxc are broken for now.
Description of the Change
Move most of initialize_lxc to init-host.
Rename lxc-install to install-lxc.
Add commands.rst, which lays out what each subcommand does, its relationships to other commands, the environment in which it is to be run, and whether it performs actions as root or the user.
Updated the docstrings, and thus the help strings, for the subcommands. Also updated the module-level docstrings for each subcommand.
NOTE:
finish-init-host and install-lxc are broken as a result of these changes and need to be refactored. Their tests have been disabled.
Since I modified setup.cfg for nosetest options to ignore 'disabled_' tests, I decided to move with-doctest to that file from the pre-commit.sh. For some reason this is now triggering a warning about the version of distribute being used.
| Benji York (benji) wrote : | # |
| Brad Crittenden (bac) wrote : | # |
Thanks for the review Benji.
The tips for better formatting the reST file are appreciated and I've made the changes. The style, formatting, and grammar fixes have been made.
The XXX needs to stay but I have provided an explanatory comment.
As for the running-
gary_poster: how can code determine if it is being run within an LXC container?
gary_poster asked Serge Hallyn if there were a reliable way for code to determine if it is being run within an LXC container. Serge said yes, and gave these steps (note that this may be Ubuntu-specific; Ubuntu-specific is good enough for us right now).
if /bin/running-
else, if lxc-is-container is not present, assume lxcguest is not installed and you're not in a container (or are in trimmed container)
else, run lxc-is-container, if 0, you're in a container, if 1 you're not
(http://
So, according to Serge, the absence of either of those test executables should indicate false.
| Launchpad QA Bot (lpqabot) wrote : | # |
There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.
| Launchpad QA Bot (lpqabot) wrote : | # |
Voting does not meet specified criteria. Required: Approve >= 1, Disapprove == 0. Got: 1 Pending.
| Benji York (benji) wrote : | # |
These changes look good. It might be a good idea to add a comment about the running-
| Launchpad QA Bot (lpqabot) wrote : | # |
There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.
| Launchpad QA Bot (lpqabot) wrote : | # |
The attempt to merge lp:~bac/lpsetup/mv-initialize-lxc into lp:lpsetup failed. Below is the output from the failed tests.
nose.plugins.cover: ERROR: Coverage not available: unable to import coverage module
.......
and can't be installed while this script is running. Please
install a more recent version first, using
'easy_install -U distribute'.
(Currently using distribute 0.6.24dev-r0 (/usr/lib/
- 57. By Brad Crittenden on 2012-07-10
-
Work-around for ignore-files

Since commands.rst is reST, we can reST-style definition lists for the docutils. sourceforge. net/docs/ ref/rst/ restructuredtex t.html# definition- lists).
"Terminology" and "install-lxc" sections (see
http://
The line length in commands.rst seems inconsistent. The longest line
looks to be 76 characters wide, which would suggest a 79 character max.
In that case there are several lines that seem to be prematurely
wrapped. I suggest 79 character lines to match our source files.
The steps list in lpsetup/ subcommands/ inithost. py (line 381 of the diff)
should end with a comma after the last item in the list and the closing
parenthesis on its own line in order to be consistent with the other
subcommands.
The docstring for call_initialize_lxc can't decide if "lxc" should be
capitalized (and the subject and verb agreement is off, too).
You have a XXX comment on line 592 of the diff.
In running_ in_container, if neither of the running- in-container or
lxc-is-container commands exist it will just return false. It seems to
me that if that happens an exception should be raised instead.