Skip to content
This repository has been archived by the owner on Jun 10, 2019. It is now read-only.

Make the workspace path relative to the manifest file #444

Closed
wants to merge 3 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion bootstrapvz/base/bootstrapinfo.py
Original file line number Diff line number Diff line change
@@ -22,7 +22,9 @@ def __init__(self, manifest=None, debug=False):

# Define the path to our workspace
import os.path
self.workspace = os.path.join(manifest.bootstrapper['workspace'], self.run_id)
from bootstrapvz.common.tools import rel_path
self.workspace_root = rel_path(manifest.path, os.path.join(manifest.bootstrapper['workspace']))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, this adds some semantic info to the code, I like it.

self.workspace = os.path.join(self.workspace_root, self.run_id)

# Load all the volume information
from fs import load_volume
4 changes: 2 additions & 2 deletions bootstrapvz/common/tasks/bootstrap.py
Original file line number Diff line number Diff line change
@@ -38,8 +38,8 @@ def get_tarball_filename(info):
# Filter info.root which points at /target/volume-id, we won't ever hit anything with that in there.
hash_args = [arg for arg in arguments if arg != info.root]
tarball_id = sha1(repr(frozenset(options + hash_args))).hexdigest()[0:8]
tarball_filename = 'debootstrap-' + tarball_id + '.tar'
return os.path.join(info.manifest.bootstrapper['workspace'], tarball_filename)
tarball_filename = 'debootstrap-' + tarball_id + '.tgz'
return os.path.join(os.path.abspath(info.workspace_root), tarball_filename)


class MakeTarball(Task):
2 changes: 1 addition & 1 deletion bootstrapvz/common/tasks/image.py
Original file line number Diff line number Diff line change
@@ -12,7 +12,7 @@ def run(cls, info):
filename = image_name + '.' + info.volume.extension

import os.path
destination = os.path.join(info.manifest.bootstrapper['workspace'], filename)
destination = os.path.join(info.workspace_root, filename)
import shutil
shutil.move(info.volume.image_path, destination)
info.volume.image_path = destination
5 changes: 2 additions & 3 deletions bootstrapvz/plugins/minimize_size/tasks/mounts.py
Original file line number Diff line number Diff line change
@@ -22,7 +22,7 @@ def run(cls, info):

full_path = os.path.join(info.root, folder)
os.chmod(temp_path, os.stat(full_path).st_mode)
info.volume.partition_map.root.add_mount(temp_path, full_path, ['--bind'])
info.volume.partition_map.root.add_mount(temp_path, folder, ['--bind'])
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mount call here is not executed in the chroot, we need the full path.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

full path is not required as it is possible to mount directory with relative paths, like this:

service:/tmp/mount-test$ mkdir src dst
service:/tmp/mount-test$ touch src/test
service:/tmp/mount-test$ sudo mount --bind src dst
service:/tmp/mount-test$ ls -1 dst
test

Actually with full_path I got errors like this:

[89365.6539917] INFO: Mounting folders for writing temporary and cache data
[89365.9210205] DEBUG: Executing: mount --bind dist/6287dc50/minimize_size/tmp dist/6287dc50/root/dist/6287dc50/root/tmp
[89369.1380024] ERROR: mount: mount point dist/6287dc50/root/dist/6287dc50/root/tmp does not exist
[89469.5329666] ERROR: Command 'mount --bind dist/6287dc50/minimize_size/tmp dist/6287dc50/root/dist/6287dc50/root/tmp' returned non-zero exit status 32
Traceback (most recent call last):
  File "/home/users/mengelmann/diskimage-builder-imagin/bootstrap-vz/bootstrapvz/base/main.py", line 111, in run
    tasklist.run(info=bootstrap_info, dry_run=dry_run)
  File "/home/users/mengelmann/diskimage-builder-imagin/bootstrap-vz/bootstrapvz/base/tasklist.py", line 44, in run
    task.run(info)
  File "/home/users/mengelmann/diskimage-builder-imagin/bootstrap-vz/bootstrapvz/plugins/minimize_size/tasks/mounts.py", line 27, in run
    info.volume.partition_map.root.add_mount(temp_path, full_path, ['--bind'])
  File "/home/users/mengelmann/diskimage-builder-imagin/bootstrap-vz/bootstrapvz/base/fs/partitions/abstract.py", line 123, in add_mount
    mount.mount(self.mount_dir)
  File "/home/users/mengelmann/diskimage-builder-imagin/bootstrap-vz/bootstrapvz/base/fs/partitions/mount.py", line 29, in mount
    log_check_call(['mount'] + self.opts + [self.source, mount_dir])
  File "/home/users/mengelmann/diskimage-builder-imagin/bootstrap-vz/bootstrapvz/common/tools.py", line 13, in log_check_call
    raise e
CalledProcessError: Command 'mount --bind dist/6287dc50/minimize_size/tmp dist/6287dc50/root/dist/6287dc50/root/tmp' returned non-zero exit status 32
[89470.1631069] ERROR: Rolling back

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait. This suggests info.root is not absolute any longer. It's set here:

class CreateMountDir(Task):
description = 'Creating mountpoint for the root partition'
phase = phases.volume_mounting
@classmethod
def run(cls, info):
import os
info.root = os.path.join(info.workspace, 'root')
os.makedirs(info.root)

Are you sure info.workspace is still absolute?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, info.root is not absolute any longer, it becomes relative to the manifest file. Should it be absolute path?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be, yes. Can you check if this was the case before the change?



class RemoveFolderMounts(Task):
@@ -35,9 +35,8 @@ def run(cls, info):
import shutil
for folder in folders:
temp_path = os.path.join(info._minimize_size['foldermounts'], folder.replace('/', '_'))
full_path = os.path.join(info.root, folder)

info.volume.partition_map.root.remove_mount(full_path)
info.volume.partition_map.root.remove_mount(folder)
shutil.rmtree(temp_path)

os.rmdir(info._minimize_size['foldermounts'])
4 changes: 2 additions & 2 deletions bootstrapvz/plugins/prebootstrapped/tasks.py
Original file line number Diff line number Diff line change
@@ -54,7 +54,7 @@ class CopyImage(Task):
@classmethod
def run(cls, info):
loopback_backup_name = 'volume-{id}.{ext}.backup'.format(id=info.run_id, ext=info.volume.extension)
destination = os.path.join(info.manifest.bootstrapper['workspace'], loopback_backup_name)
destination = os.path.join(info.workspace_root, loopback_backup_name)

with unmounted(info.volume):
copyfile(info.volume.image_path, destination)
@@ -84,7 +84,7 @@ class CopyFolder(Task):
@classmethod
def run(cls, info):
folder_backup_name = '{id}.{ext}.backup'.format(id=info.run_id, ext=info.volume.extension)
destination = os.path.join(info.manifest.bootstrapper['workspace'], folder_backup_name)
destination = os.path.join(info.workspace_root, folder_backup_name)
log_check_call(['cp', '-a', info.volume.path, destination])
msg = 'A copy of the bootstrapped volume was created. Path: ' + destination
log.info(msg)
2 changes: 1 addition & 1 deletion bootstrapvz/plugins/vagrant/tasks.py
Original file line number Diff line number Diff line change
@@ -16,7 +16,7 @@ class CheckBoxPath(Task):
def run(cls, info):
box_basename = info.manifest.name.format(**info.manifest_vars)
box_name = box_basename + '.box'
box_path = os.path.join(info.manifest.bootstrapper['workspace'], box_name)
box_path = os.path.join(info.workspace_root, box_name)
if os.path.exists(box_path):
from bootstrapvz.common.exceptions import TaskError
msg = 'The vagrant box `{name}\' already exists at `{path}\''.format(name=box_name, path=box_path)
4 changes: 2 additions & 2 deletions bootstrapvz/providers/gce/tasks/image.py
Original file line number Diff line number Diff line change
@@ -19,11 +19,11 @@ def run(cls, info):
image_name = image_name.replace(".", "-")
info._gce['image_name'] = image_name
tarball_name = image_name + '.tar.gz'
tarball_path = os.path.join(info.manifest.bootstrapper['workspace'], tarball_name)
tarball_path = os.path.join(info.workspace_root, tarball_name)
info._gce['tarball_name'] = tarball_name
info._gce['tarball_path'] = tarball_path
# GCE requires that the file in the tar be named disk.raw, hence the transform
log_check_call(['tar', '--sparse', '-C', info.manifest.bootstrapper['workspace'],
log_check_call(['tar', '--sparse', '-C', info.workspace_root,
'-caf', tarball_path,
'--transform=s|.*|disk.raw|',
filename])
4 changes: 2 additions & 2 deletions bootstrapvz/providers/oracle/tasks/image.py
Original file line number Diff line number Diff line change
@@ -16,9 +16,9 @@ def run(cls, info):
filename = image_name + '.' + info.volume.extension

tarball_name = image_name + '.tar.gz'
tarball_path = os.path.join(info.manifest.bootstrapper['workspace'], tarball_name)
tarball_path = os.path.join(info.workspace_root, tarball_name)
info._oracle['tarball_path'] = tarball_path
log_check_call(['tar', '--sparse', '-C', info.manifest.bootstrapper['workspace'],
log_check_call(['tar', '--sparse', '-C', info.workspace_root,
'-caf', tarball_path, filename])