Skip to content
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.

Sign up for GitHub

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

Jump to bottom

lxd: don't inject local snaps on a different arch #1577

Merged
merged 11 commits into from Oct 16, 2017
Merged

lxd: don't inject local snaps on a different arch #1577

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
975a446
Don't inject local snaps on a different arch
Sep 29, 2017
a9c2376
Install snap from the right channel
Sep 29, 2017
3839ac2
Refresh channel if needed
Sep 29, 2017
8615321
Merge branch 'master' into lxd-arch-mismatch
Oct 5, 2017
dd9c6f8
Add channel to FakeSnapd.snaps
Oct 5, 2017
79594ca
LXD cross scenarios should not expect injects
Oct 5, 2017
f22e74d
Merge branch 'master' into lxd-arch-mismatch
Oct 8, 2017
879084f
Split off snap install into a method
Oct 10, 2017
c6b9190
Assert snap install commands in cross scenarios
Oct 10, 2017
07af59e
Clarify comments for install/ refresh calls
Oct 11, 2017
7d7a55f
Add snap command assertions in second inject test case
Oct 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 30 additions & 9 deletions snapcraft/internal/lxd.py
View file
Open in desktop
Expand Up @@ -74,11 +74,11 @@ def __init__(self, *, output, source, project_options,
kernel = server_environment['kernel_architecture']
except KeyError:
kernel = server_environment['kernelarchitecture']
deb_arch = _get_deb_arch(kernel)
if not deb_arch:
self._server_arch = _get_deb_arch(kernel)
if not self._server_arch:
raise ContainerConnectionError(
'Unrecognized server architecture {}'.format(kernel))
self._image = 'ubuntu:xenial/{}'.format(deb_arch)
self._image = 'ubuntu:xenial/{}'.format(self._server_arch)
# Use a temporary folder the 'lxd' snap can access
lxd_common_dir = os.path.expanduser(
os.path.join('~', 'snap', 'lxd', 'common'))
Expand Down Expand Up @@ -218,6 +218,13 @@ def _inject_snap(self, name):
id = json['result']['id']
# Lookup confinement to know if we need to --classic when installing
is_classic = json['result']['confinement'] == 'classic'

# If the server has a different arch we can't inject local snaps
if (self._project_options.target_arch
and self._project_options.target_arch != self._server_arch):
channel = json['result']['channel']
return self._install_snap(name, channel, is_classic=is_classic)
Copy link
Contributor

Choose a reason for hiding this comment

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

Okay so we still keep the channel here? Very good.

Copy link
Collaborator

Choose a reason for hiding this comment

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

eventually this code would need some refactoring, these return statements in the middle will bite us back later


# Revisions are unique, so we don't need to know the channel
rev = json['result']['revision']

Expand Down Expand Up @@ -246,13 +253,27 @@ def _inject_snap(self, name):
shutil.copyfile(installed, filepath)
container_filename = os.path.join(os.sep, 'run', filename)
self._push_file(filepath, container_filename)
logger.info('Installing {}'.format(container_filename))
cmd = ['snap', 'install', container_filename]
if rev.startswith('x'):
cmd.append('--dangerous')
self._install_snap(container_filename,
is_dangerous=rev.startswith('x'),
is_classic=is_classic)

def _install_snap(self, name, channel=None,
is_dangerous=False,
is_classic=False):
logger.info('Installing {}'.format(name))
# Install: will do nothing if already installed
args = []
if channel:
args.append('--channel')
args.append(channel)
if is_dangerous:
args.append('--dangerous')
if is_classic:
cmd.append('--classic')
self._container_run(cmd)
args.append('--classic')
self._container_run(['snap', 'install', name] + args)
if channel:
# Switch channel if install was a no-op
self._container_run(['snap', 'refresh', name] + args)

def _inject_assertions(self, filename, assertions):
filepath = os.path.join(self.tmp_dir, filename)
Expand Down
34 changes: 32 additions & 2 deletions snapcraft/tests/test_lxd.py
View file
Open in desktop
Expand Up @@ -43,10 +43,12 @@ class LXDTestCase(tests.TestCase):
scenarios = [
('local', dict(remote='local', target_arch=None, server='x86_64')),
('remote', dict(remote='myremote', target_arch=None, server='x86_64')),
('cross', dict(remote='local', target_arch='armhf', server='x86_64')),
('cross', dict(remote='local', target_arch='armhf', server='x86_64',
cross=True)),
('arm remote', dict(remote='pi', target_arch=None, server='armv7l')),
('arm same', dict(remote='pi', target_arch='armhf', server='armv7l')),
('arm cross', dict(remote='pi', target_arch='arm64', server='armv7l')),
('arm cross', dict(remote='pi', target_arch='arm64', server='armv7l',
cross=True)),
]

def setUp(self):
Expand Down Expand Up @@ -233,10 +235,12 @@ def test_parallel_invocation_inject_snap(self, mock_is_snap):
{'name': 'core',
'confinement': 'strict',
'id': '2kkitQurgOkL3foImG4wDwn9CIANuHlt',
'channel': 'stable',
'revision': '123'},
{'name': 'snapcraft',
'confinement': 'classic',
'id': '3lljuRvshPlM4gpJnH5xExo0DJBOvImu',
'channel': 'edge',
'revision': '345'},
]

Expand Down Expand Up @@ -318,16 +322,29 @@ def test_inject_snap(self,
{'name': 'core',
'confinement': 'strict',
'id': '2kkitQurgOkL3foImG4wDwn9CIANuHlt',
'channel': 'stable',
'revision': '123'},
{'name': 'snapcraft',
'confinement': 'classic',
'id': '3lljuRvshPlM4gpJnH5xExo0DJBOvImu',
'channel': 'edge',
'revision': '345'},
]

builder = self.make_cleanbuilder()

builder.execute()
if hasattr(self, 'cross') and self.cross:
mock_container_run.assert_has_calls([
call(['snap', 'install', 'core', '--channel', 'stable']),
call(['snap', 'refresh', 'core', '--channel', 'stable']),
call(['snap', 'install', 'snapcraft', '--channel', 'edge',
'--classic']),
call(['snap', 'refresh', 'snapcraft', '--channel', 'edge',
'--classic']),
])
return

self.fake_lxd.check_call_mock.assert_has_calls([
call(['lxc', 'file', 'push',
os.path.join(builder.tmp_dir, 'core_123.assert'),
Expand Down Expand Up @@ -367,16 +384,29 @@ def test_inject_snap_dangerous(self,
{'name': 'core',
'confinement': 'strict',
'id': '2kkitQurgOkL3foImG4wDwn9CIANuHlt',
'channel': 'stable',
'revision': '123'},
{'name': 'snapcraft',
'confinement': 'classic',
'id': '',
'channel': 'edge',
'revision': 'x1'},
]

builder = self.make_cleanbuilder()

builder.execute()
if hasattr(self, 'cross') and self.cross:
mock_container_run.assert_has_calls([
call(['snap', 'install', 'core', '--channel', 'stable']),
call(['snap', 'refresh', 'core', '--channel', 'stable']),
call(['snap', 'install', 'snapcraft', '--channel', 'edge',
'--classic']),
call(['snap', 'refresh', 'snapcraft', '--channel', 'edge',
'--classic']),
])
return

self.fake_lxd.check_call_mock.assert_has_calls([
call(['sudo', 'cp', '/var/lib/snapd/snaps/snapcraft_x1.snap',
os.path.join(builder.tmp_dir, 'snapcraft_x1.snap')]),
Expand Down