From c1b0a00c607c13c52991e14056e6bcac4a49d5d5 Mon Sep 17 00:00:00 2001 From: Clark Boylan Date: Wed, 17 May 2023 13:17:50 -0700 Subject: Only check bwrap execution under the executor The reason for this is that containers for zuul services need to run privileged in order to successfully run bwrap. We currently only expect users to run the executor as privilged and the new bwrap execution checks have broken other services as a result. (Other services load the bwrap system bceause it is a normal zuul driver and all drivers are loaded by all services). This works around this by add a check_bwrap flag to connection setup and only setting it to true on the executor. A better longer term followup fixup would be to only instantiate the bwrap driver on the executor in the first place. This can probably be accomplished by overriding the ZuulApp configure_connections method in the executor and dropping bwrap creation in ZuulApp. Temporarily stop running the quick-start job since it's apparently not using speculative images. Change-Id: Ibadac0450e2879ef1ccc4b308ebd65de6e5a75ab --- .zuul.yaml | 6 +++--- tests/base.py | 2 +- tests/unit/test_bubblewrap.py | 4 ++-- zuul/cmd/__init__.py | 6 ++++-- zuul/cmd/executor.py | 2 +- zuul/driver/bubblewrap/__init__.py | 34 ++++++++++++++++++---------------- zuul/lib/connections.py | 5 +++-- 7 files changed, 32 insertions(+), 27 deletions(-) diff --git a/.zuul.yaml b/.zuul.yaml index f357c6ce6..54eb4cca4 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -389,9 +389,9 @@ nodeset: ubuntu-jammy - zuul-stream-functional-6 - zuul-nox-remote - - zuul-quick-start: - requires: nodepool-container-image - dependencies: zuul-upload-image + # - zuul-quick-start: + # requires: nodepool-container-image + # dependencies: zuul-upload-image - zuul-nox-zuul-client - zuul-build-python-release promote: diff --git a/tests/base.py b/tests/base.py index af10ebe96..d416474fd 100644 --- a/tests/base.py +++ b/tests/base.py @@ -352,7 +352,7 @@ class TestConnectionRegistry(ConnectionRegistry): self.registerDriver(SMTPDriver()) self.registerDriver(TimerDriver()) self.registerDriver(SQLDriver()) - self.registerDriver(BubblewrapDriver()) + self.registerDriver(BubblewrapDriver(check_bwrap=True)) self.registerDriver(NullwrapDriver()) self.registerDriver(MQTTDriver()) self.registerDriver(PagureDriverMock( diff --git a/tests/unit/test_bubblewrap.py b/tests/unit/test_bubblewrap.py index 822cb3ca5..33634fcd0 100644 --- a/tests/unit/test_bubblewrap.py +++ b/tests/unit/test_bubblewrap.py @@ -33,7 +33,7 @@ class TestBubblewrap(testtools.TestCase): self.useFixture(fixtures.NestedTempfile()) def test_bubblewrap_wraps(self): - bwrap = bubblewrap.BubblewrapDriver() + bwrap = bubblewrap.BubblewrapDriver(check_bwrap=True) context = bwrap.getExecutionContext() work_dir = tempfile.mkdtemp() ssh_agent = SshAgent() @@ -57,7 +57,7 @@ class TestBubblewrap(testtools.TestCase): @skipIf(sys.platform == 'darwin', 'Not supported on MacOS') def test_bubblewrap_leak(self): - bwrap = bubblewrap.BubblewrapDriver() + bwrap = bubblewrap.BubblewrapDriver(check_bwrap=True) context = bwrap.getExecutionContext() work_dir = tempfile.mkdtemp() ansible_dir = tempfile.mkdtemp() diff --git a/zuul/cmd/__init__.py b/zuul/cmd/__init__.py index 9ce342502..218556a9d 100755 --- a/zuul/cmd/__init__.py +++ b/zuul/cmd/__init__.py @@ -197,8 +197,10 @@ class ZuulApp(object): logging_config.setDebug() logging_config.apply() - def configure_connections(self, source_only=False, require_sql=False): - self.connections = zuul.lib.connections.ConnectionRegistry() + def configure_connections(self, source_only=False, + require_sql=False, check_bwrap=False): + self.connections = zuul.lib.connections.ConnectionRegistry( + check_bwrap=check_bwrap) self.connections.configure(self.config, source_only, require_sql) diff --git a/zuul/cmd/executor.py b/zuul/cmd/executor.py index eb463348e..ef0cef54e 100755 --- a/zuul/cmd/executor.py +++ b/zuul/cmd/executor.py @@ -82,7 +82,7 @@ class Executor(zuul.cmd.ZuulDaemonApp): def run(self): self.handleCommands() - self.configure_connections(source_only=True) + self.configure_connections(source_only=True, check_bwrap=True) if self.config.has_option('executor', 'job_dir'): self.job_dir = os.path.expanduser( diff --git a/zuul/driver/bubblewrap/__init__.py b/zuul/driver/bubblewrap/__init__.py index 4c85299a5..d7c6921cc 100644 --- a/zuul/driver/bubblewrap/__init__.py +++ b/zuul/driver/bubblewrap/__init__.py @@ -157,24 +157,26 @@ class BubblewrapDriver(Driver, WrapperInterface): release_file_re = re.compile(r'^\W+-release$') bwrap_version_re = re.compile(r'^(\d+\.\d+\.\d+).*') - def __init__(self): + def __init__(self, check_bwrap): self.userns_enabled = self._is_userns_enabled() self.bwrap_version = self._parse_bwrap_version() self.bwrap_command = self._bwrap_command() - # Validate basic bwrap functionality before we attempt to run - # workloads under bwrap. - context = self.getExecutionContext() - popen = context.getPopen(work_dir='/tmp', - ssh_auth_sock='/dev/null') - p = popen(['id'], - stdout=subprocess.PIPE, stderr=subprocess.STDOUT) - p.communicate() - if p.returncode != 0: - self.log.error("Non zero return code executing: %s", - " ".join(shlex.quote(c) - for c in popen.command + ['id'])) - raise Exception('bwrap execution validation failed. You can use ' - '`zuul-bwrap /tmp id` to investigate manually.') + if check_bwrap: + # Validate basic bwrap functionality before we attempt to run + # workloads under bwrap. + context = self.getExecutionContext() + popen = context.getPopen(work_dir='/tmp', + ssh_auth_sock='/dev/null') + p = popen(['id'], + stdout=subprocess.PIPE, stderr=subprocess.STDOUT) + p.communicate() + if p.returncode != 0: + self.log.error("Non zero return code executing: %s", + " ".join(shlex.quote(c) + for c in popen.command + ['id'])) + raise Exception('bwrap execution validation failed. You can ' + 'use `zuul-bwrap /tmp id` to investigate ' + 'manually.') def _is_userns_enabled(self): # This is based on the bwrap checks found here: @@ -297,7 +299,7 @@ class BubblewrapDriver(Driver, WrapperInterface): def main(args=None): logging.basicConfig(level=logging.DEBUG) - driver = BubblewrapDriver() + driver = BubblewrapDriver(check_bwrap=True) parser = argparse.ArgumentParser() parser.add_argument('--ro-paths', nargs='+') diff --git a/zuul/lib/connections.py b/zuul/lib/connections.py index b5f5e53f5..c4d458158 100644 --- a/zuul/lib/connections.py +++ b/zuul/lib/connections.py @@ -46,7 +46,7 @@ class ConnectionRegistry(object): log = logging.getLogger("zuul.ConnectionRegistry") - def __init__(self): + def __init__(self, check_bwrap=False): self.connections = OrderedDict() self.drivers = {} @@ -57,7 +57,8 @@ class ConnectionRegistry(object): self.registerDriver(zuul.driver.smtp.SMTPDriver()) self.registerDriver(zuul.driver.timer.TimerDriver()) self.registerDriver(zuul.driver.sql.SQLDriver()) - self.registerDriver(zuul.driver.bubblewrap.BubblewrapDriver()) + self.registerDriver( + zuul.driver.bubblewrap.BubblewrapDriver(check_bwrap)) self.registerDriver(zuul.driver.nullwrap.NullwrapDriver()) self.registerDriver(zuul.driver.mqtt.MQTTDriver()) self.registerDriver(zuul.driver.pagure.PagureDriver()) -- cgit v1.2.1