Move localloop to exec_sudo

One call in localloop requires the output of the command, so modify
exec_sudo to buffer up output and return it.  This is modelled on the
same thing in package-installs-v2 which seems to work.  Rather than
return a subprocess exception, return a dib exception which everything
should have imported anyway.

The overall reason for this is to make our external calls more
consistent for mocking in unit testing.

Change-Id: I10d23b873dee9f775daef2a4c8be5671d02c386e
This commit is contained in:
Ian Wienand 2018-06-28 10:54:40 +10:00
parent f5736f3178
commit a1a549548a
3 changed files with 44 additions and 34 deletions

View File

@ -14,12 +14,12 @@
import logging import logging
import os import os
import subprocess
from diskimage_builder.block_device.exception import \ from diskimage_builder.block_device.exception import \
BlockDeviceSetupException BlockDeviceSetupException
from diskimage_builder.block_device.plugin import NodeBase from diskimage_builder.block_device.plugin import NodeBase
from diskimage_builder.block_device.plugin import PluginBase from diskimage_builder.block_device.plugin import PluginBase
from diskimage_builder.block_device.utils import exec_sudo
from diskimage_builder.block_device.utils import parse_abs_size_spec from diskimage_builder.block_device.utils import parse_abs_size_spec
@ -41,17 +41,11 @@ def image_delete(filename):
def loopdev_attach(filename): def loopdev_attach(filename):
logger.info("loopdev attach") logger.info("loopdev attach")
logger.debug("Calling [sudo losetup --show -f %s]", filename) logger.debug("Calling [sudo losetup --show -f %s]", filename)
subp = subprocess.Popen(["sudo", "losetup", "--show", "-f", block_device = exec_sudo(["losetup", "--show", "-f", filename])
filename], stdout=subprocess.PIPE) # [:-1]: Cut of the newline
rval = subp.wait() block_device = block_device[:-1]
if rval == 0: logger.info("New block device [%s]", block_device)
# [:-1]: Cut of the newline return block_device
block_device = subp.stdout.read()[:-1].decode("utf-8")
logger.info("New block device [%s]", block_device)
return block_device
else:
logger.error("losetup failed")
raise BlockDeviceSetupException("losetup failed")
def loopdev_detach(loopdev): def loopdev_detach(loopdev):
@ -59,19 +53,16 @@ def loopdev_detach(loopdev):
# loopback dev may be tied up a bit by udev events triggered # loopback dev may be tied up a bit by udev events triggered
# by partition events # by partition events
for try_cnt in range(10, 1, -1): for try_cnt in range(10, 1, -1):
logger.debug("Calling [sudo losetup -d %s]", loopdev) try:
subp = subprocess.Popen(["sudo", "losetup", "-d", exec_sudo(["losetup", "-d", loopdev])
loopdev]) return
rval = subp.wait() except BlockDeviceSetupException as e:
if rval == 0:
logger.info("Successfully detached [%s]", loopdev)
return 0
else:
logger.error("loopdev detach failed")
# Do not raise an error - maybe other cleanup methods # Do not raise an error - maybe other cleanup methods
# can at least do some more work. # can at least do some more work.
logger.error("loopdev detach failed (%s)", e.returncode)
logger.debug("Gave up trying to detach [%s]", loopdev) logger.debug("Gave up trying to detach [%s]", loopdev)
return rval return 1
class LocalLoopNode(NodeBase): class LocalLoopNode(NodeBase):

View File

@ -13,7 +13,6 @@
# limitations under the License. # limitations under the License.
import logging import logging
import subprocess
from diskimage_builder.block_device.exception \ from diskimage_builder.block_device.exception \
import BlockDeviceSetupException import BlockDeviceSetupException
@ -313,8 +312,8 @@ class LVMUmountNode(NodeBase):
def umount(self): def umount(self):
try: try:
exec_sudo(['pvscan', '--cache']) exec_sudo(['pvscan', '--cache'])
except subprocess.CalledProcessError as cpe: except BlockDeviceSetupException as e:
logger.debug("pvscan call result [%s]", cpe) logger.info("pvscan call failed [%s]", e.returncode)
def get_edges(self): def get_edges(self):
# This node depends on all physical device(s), which is # This node depends on all physical device(s), which is

View File

@ -12,10 +12,14 @@
# License for the specific language governing permissions and limitations # License for the specific language governing permissions and limitations
# under the License. # under the License.
import locale
import logging import logging
import re import re
import subprocess import subprocess
from diskimage_builder.block_device.exception import \
BlockDeviceSetupException
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
@ -95,10 +99,16 @@ def exec_sudo(cmd):
at debug levels. at debug levels.
Arguments: Arguments:
:param cmd: str command list; for Popen()
:return: nothing
:raises: subprocess.CalledProcessError if return code != 0
:param cmd: str command list; for Popen()
:return: the stdout+stderror of the called command
:raises BlockDeviceSetupException: if return code != 0.
Exception values similar to ``subprocess.CalledProcessError``
* ``returncode`` : returncode of child
* ``cmd`` : the command run
* ``output`` : stdout+stderr output
""" """
assert isinstance(cmd, list) assert isinstance(cmd, list)
sudo_cmd = ["sudo"] sudo_cmd = ["sudo"]
@ -116,10 +126,20 @@ def exec_sudo(cmd):
stdout=subprocess.PIPE, stdout=subprocess.PIPE,
stderr=subprocess.STDOUT) stderr=subprocess.STDOUT)
for line in iter(proc.stdout.readline, b""): out = ""
logger.debug("exec_sudo: %s", line.rstrip()) with proc.stdout:
for line in iter(proc.stdout.readline, b''):
line = line.decode(encoding=locale.getpreferredencoding(False),
errors='backslashreplace')
out += line
logger.debug("exec_sudo: %s", line.rstrip())
proc.wait() proc.wait()
if proc.returncode != 0:
raise subprocess.CalledProcessError(proc.returncode, if proc.returncode:
' '.join(sudo_cmd)) e = BlockDeviceSetupException("exec_sudo failed")
e.returncode = proc.returncode
e.cmd = ' '.join(sudo_cmd)
e.output = out
raise e
return out