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

Make the workspace path relative to the manifest file #444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4fc6400
Make the workspace path relative to the manifest file (resolves #424)
mengelmann Jan 23, 2018
dfe7b8a
debootstrap --unpack-tarball requires an absolute path to a tarball
mengelmann Jan 23, 2018
50c54af
Cosmetic - debootstrap creates tarball that is actually gzipped tar, …
mengelmann Jan 23, 2018
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions bootstrapvz/common/tasks/bootstrap.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion bootstrapvz/common/tasks/image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions bootstrapvz/plugins/minimize_size/tasks/mounts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:

bootstrap-vz/bootstrapvz/common/tasks/filesystem.py

Lines 59 to 67 in a4e4ad9

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):
Expand All @@ -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'])
Expand Down
4 changes: 2 additions & 2 deletions bootstrapvz/plugins/prebootstrapped/tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion bootstrapvz/plugins/vagrant/tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions bootstrapvz/providers/gce/tasks/image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
4 changes: 2 additions & 2 deletions bootstrapvz/providers/oracle/tasks/image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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])


Expand Down