-
Notifications
You must be signed in to change notification settings - Fork 441
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
lifecycle: don't clean priming area if the snap is being tried #2143
Merged
sergiusens
merged 5 commits into
canonical:master
from
kyrofa:bugfix/clean_prime_of_tried
Jun 7, 2018
Merged
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
fda2f4d
lifecycle: don't clean priming area if the snap is being tried
kyrofa 9d7e4a3
Merge remote-tracking branch 'origin/master' into bugfix/clean_prime_…
kyrofa 88782df
Handle invalid mountinfo format
kyrofa 74f17b6
Add tests for error strings
kyrofa 6222cd4
Merge remote-tracking branch 'origin/master' into bugfix/clean_prime_…
kyrofa File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*- | ||
# | ||
# Copyright (C) 2018 Canonical Ltd | ||
# | ||
# This program is free software: you can redistribute it and/or modify | ||
# it under the terms of the GNU General Public License version 3 as | ||
# published by the Free Software Foundation. | ||
# | ||
# This program is distributed in the hope that it will be useful, | ||
# but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
# GNU General Public License for more details. | ||
# | ||
# You should have received a copy of the GNU General Public License | ||
# along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
|
||
import collections | ||
import contextlib | ||
import csv | ||
from typing import Dict, List # noqa: F401 | ||
|
||
from snapcraft.internal import errors | ||
|
||
|
||
class Mount: | ||
"""A class to provide programmatic access to a specific mountpoint""" | ||
|
||
def __init__(self, mountinfo_row: List[str]) -> None: | ||
# Parse the row according to section 3.5 of | ||
# https://www.kernel.org/doc/Documentation/filesystems/proc.txt | ||
self.mount_id = mountinfo_row[0] | ||
self.parent_id = mountinfo_row[1] | ||
self.st_dev = mountinfo_row[2] | ||
self.root = mountinfo_row[3] | ||
self.mount_point = mountinfo_row[4] | ||
self.mount_options = mountinfo_row[5] | ||
separator_index = mountinfo_row.index('-') | ||
self.optional_fields = mountinfo_row[6:separator_index] | ||
self.filesystem_type = mountinfo_row[separator_index+1] | ||
self.mount_source = mountinfo_row[separator_index+2] | ||
self.super_options = mountinfo_row[separator_index+3] | ||
|
||
|
||
class MountInfo: | ||
"""A class to provide programmatic access to /proc/self/mountinfo""" | ||
|
||
def __init__(self, *, | ||
mountinfo_file: str = '/proc/self/mountinfo') -> None: | ||
"""Create a new MountInfo instance. | ||
|
||
:param str mountinfo_file: Path to mountinfo file to be parsed. | ||
""" | ||
# Maintain two dicts pointing to the same underlying objects: | ||
# a dict of mount points to Mounts, and a dict of roots to Mounts. | ||
self._mount_point_mounts = {} # type: Dict[str, Mount] | ||
root_mounts = collections.defaultdict(list) # type: Dict[str, List[Mount]] # noqa | ||
|
||
with contextlib.suppress(FileNotFoundError): | ||
with open(mountinfo_file) as f: | ||
for row in csv.reader(f, delimiter=' '): | ||
mount = Mount(row) | ||
self._mount_point_mounts[mount.mount_point] = mount | ||
root_mounts[mount.root].append(mount) | ||
|
||
self._root_mounts = dict(root_mounts) | ||
|
||
def for_mount_point(self, mount_point: str) -> Mount: | ||
try: | ||
return self._mount_point_mounts[mount_point] | ||
except KeyError: | ||
raise errors.MountPointNotFoundError(mount_point) | ||
|
||
def for_root(self, root: str) -> List[Mount]: | ||
try: | ||
return self._root_mounts[root] | ||
except KeyError: | ||
raise errors.RootNotMountedError(root) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,114 @@ | ||
# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*- | ||
# | ||
# Copyright (C) 2018 Canonical Ltd | ||
# | ||
# This program is free software: you can redistribute it and/or modify | ||
# it under the terms of the GNU General Public License version 3 as | ||
# published by the Free Software Foundation. | ||
# | ||
# This program is distributed in the hope that it will be useful, | ||
# but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
# GNU General Public License for more details. | ||
# | ||
# You should have received a copy of the GNU General Public License | ||
# along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
|
||
from textwrap import dedent | ||
|
||
from testtools.matchers import Equals, HasLength | ||
|
||
from snapcraft.internal import ( | ||
errors, | ||
mountinfo, | ||
) | ||
|
||
from tests import unit | ||
|
||
|
||
class MountInfoTestCase(unit.TestCase): | ||
|
||
def _write_mountinfo(self, contents): | ||
path = 'mountinfo' | ||
with open(path, 'w') as f: | ||
f.write(contents) | ||
return path | ||
|
||
def test_mountinfo_by_root(self): | ||
mounts = mountinfo.MountInfo(mountinfo_file=self._write_mountinfo( | ||
dedent("""\ | ||
23 28 0:4 / /proc rw,nosuid,nodev,noexec,relatime shared:14 - proc proc rw | ||
1341 28 7:6 / /snap/snapcraft/1 ro,nodev,relatime shared:39 - squashfs /dev/loop6 ro | ||
1455 28 253:0 /test-snap/prime /snap/test-snap/x1 ro,relatime shared:1 - ext4 /dev/mapper/foo rw,errors=remount-ro,data=ordered | ||
"""))) # noqa | ||
|
||
root_mounts = mounts.for_root('/') | ||
for mount_point in ('/proc', '/snap/snapcraft/1'): | ||
self.assertTrue( | ||
any(m for m in root_mounts if m.mount_point == mount_point), | ||
'Expected {!r} to be included in root mounts'.format( | ||
mount_point)) | ||
|
||
test_snap_mounts = mounts.for_root('/test-snap/prime') | ||
self.assertThat(test_snap_mounts, HasLength(1)) | ||
self.expectThat( | ||
test_snap_mounts[0].mount_point, Equals('/snap/test-snap/x1')) | ||
|
||
def test_mountinfo_by_mount_point(self): | ||
mounts = mountinfo.MountInfo(mountinfo_file=self._write_mountinfo( | ||
dedent("""\ | ||
23 28 0:4 / /proc rw,nosuid,nodev,noexec,relatime shared:14 - proc proc rw | ||
1341 28 7:6 / /snap/snapcraft/1 ro,nodev,relatime shared:39 - squashfs /dev/loop6 ro | ||
1455 28 253:0 /test-snap/prime /snap/test-snap/x1 ro,relatime shared:1 - ext4 /dev/mapper/foo rw,errors=remount-ro,data=ordered | ||
"""))) # noqa | ||
|
||
mount = mounts.for_mount_point('/proc') | ||
self.assertThat(mount.mount_id, Equals('23')) | ||
self.assertThat(mount.parent_id, Equals('28')) | ||
self.assertThat(mount.st_dev, Equals('0:4')) | ||
self.assertThat(mount.root, Equals('/')) | ||
self.assertThat(mount.mount_point, Equals('/proc')) | ||
self.assertThat( | ||
mount.mount_options, Equals('rw,nosuid,nodev,noexec,relatime')) | ||
self.assertThat(mount.optional_fields, Equals(['shared:14'])) | ||
self.assertThat(mount.filesystem_type, Equals('proc')) | ||
self.assertThat(mount.mount_source, Equals('proc')) | ||
self.assertThat(mount.super_options, Equals('rw')) | ||
|
||
mount = mounts.for_mount_point('/snap/snapcraft/1') | ||
self.assertThat(mount.mount_id, Equals('1341')) | ||
self.assertThat(mount.parent_id, Equals('28')) | ||
self.assertThat(mount.st_dev, Equals('7:6')) | ||
self.assertThat(mount.root, Equals('/')) | ||
self.assertThat(mount.mount_point, Equals('/snap/snapcraft/1')) | ||
self.assertThat(mount.mount_options, Equals('ro,nodev,relatime')) | ||
self.assertThat(mount.optional_fields, Equals(['shared:39'])) | ||
self.assertThat(mount.filesystem_type, Equals('squashfs')) | ||
self.assertThat(mount.mount_source, Equals('/dev/loop6')) | ||
self.assertThat(mount.super_options, Equals('ro')) | ||
|
||
mount = mounts.for_mount_point('/snap/test-snap/x1') | ||
self.assertThat(mount.mount_id, Equals('1455')) | ||
self.assertThat(mount.parent_id, Equals('28')) | ||
self.assertThat(mount.st_dev, Equals('253:0')) | ||
self.assertThat(mount.root, Equals('/test-snap/prime')) | ||
self.assertThat(mount.mount_point, Equals('/snap/test-snap/x1')) | ||
self.assertThat(mount.mount_options, Equals('ro,relatime')) | ||
self.assertThat(mount.optional_fields, Equals(['shared:1'])) | ||
self.assertThat(mount.filesystem_type, Equals('ext4')) | ||
self.assertThat(mount.mount_source, Equals('/dev/mapper/foo')) | ||
self.assertThat( | ||
mount.super_options, Equals('rw,errors=remount-ro,data=ordered')) | ||
|
||
def test_mountinfo_missing_root(self): | ||
mounts = mountinfo.MountInfo(mountinfo_file=self._write_mountinfo('')) | ||
raised = self.assertRaises( | ||
errors.RootNotMountedError, mounts.for_root, 'test-root') | ||
self.assertThat(raised.root, Equals('test-root')) | ||
|
||
def test_mountinfo_missing_mount_point(self): | ||
mounts = mountinfo.MountInfo(mountinfo_file=self._write_mountinfo('')) | ||
raised = self.assertRaises( | ||
errors.MountPointNotFoundError, mounts.for_mount_point, | ||
'test-root') | ||
self.assertThat(raised.mount_point, Equals('test-root')) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can raise a KeyError, if catched, warn and continue would be my choice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thank you. Fixed.