koji

Clone

#344 runroot: Support directly configuring the mock arguments
Closed 7 years ago by walters. Opened 7 years ago by walters.
walters/koji config-mock  into  master

runroot: Support nspawn_args
Colin Walters • 7 years ago  
18f23a9
file modified
+4 -1 
@@ -176,7 +176,8 @@ 
 

          self.config = self.session.getBuildConfig(self.tag_id, event=self.event_id)
 

  

      def _new(self, tag, arch, task_id, repo_id=None, install_group='build',
 

-              setup_dns=False, bind_opts=None, maven_opts=None, maven_envs=None, deps=None):
 

+              setup_dns=False, bind_opts=None, maven_opts=None, maven_envs=None, deps=None,
 

+              nspawn_args=[]):
 

          """Create a brand new repo"""
 

          if not repo_id:
 

              raise koji.BuildrootError("A repo id must be provided")
 
@@ -215,6 +216,7 @@ 
 

          self.install_group = install_group
 

          self.setup_dns = setup_dns
 

          self.bind_opts = bind_opts
 

+         self.nspawn_args = nspawn_args
 

          self.maven_opts = maven_opts
 

          self.maven_envs = maven_envs
 

          self.deps = deps
 
@@ -239,6 +241,7 @@ 
 

          opts['maven_opts'] = self.maven_opts
 

          opts['maven_envs'] = self.maven_envs
 

          opts['bind_opts'] = self.bind_opts
 

+         opts['nspawn_args'] = self.nspawn_args
 

          opts['target_arch'] = self.target_arch
 

          if 'mock.package_manager' in self.config['extra']:
 

              opts['package_manager'] = self.config['extra']['mock.package_manager']

file modified
+4 
@@ -7111,6 +7111,8 @@ 
 

              help=_("Run command through a shell, otherwise uses exec"))
 

      parser.add_option("--new-chroot", action="store_true", default=False,
 

              help=_("Run command with the --new-chroot (systemd-nspawn) option to mock"))
 

+     parser.add_option("--nspawn-arg", action="append", dest="nspawn_args", default=[],
 

+             help=_("Configure nspawn arguments for mock"))
 

      parser.add_option("--repo-id", type="int", help=_("ID of the repo to use"))
 

  

      (opts, args) = parser.parse_args(args)
 
@@ -7136,6 +7138,8 @@ 
 

          # builders with a different function signature
 

          if opts.new_chroot:
 

              kwargs['new_chroot'] = True
 

+         if opts.nspawn_args:
 

+             kwargs['nspawn_args'] = opts.nspawn_args
 

  

          task_id = session.runroot(tag, arch, command, **kwargs)
 

      except koji.GenericError, e:

file modified
+3 -2 
@@ -92,7 +92,7 @@ 
 

              if not path.startswith('/'):
 

                  raise koji.GenericError("bad config: all paths (default_mounts, safe_roots, path_subs) needs to be absolute: %s" % path)
 

  

-     def handler(self, root, arch, command, keep=False, packages=[], mounts=[], repo_id=None, skip_setarch=False, weight=None, upload_logs=None, new_chroot=False):
 

+     def handler(self, root, arch, command, keep=False, packages=[], mounts=[], repo_id=None, skip_setarch=False, weight=None, upload_logs=None, new_chroot=False, nspawn_args=[]):
 

          """Create a buildroot and run a command (as root) inside of it
 

  

          Command may be a string or a list.
 
@@ -154,7 +154,8 @@ 
 

          if compat_mode:
 

              broot = BuildRoot(root, br_arch, self.id, repo_id=repo_info['id'], setup_dns=True)
 

          else:
 

-             broot = BuildRoot(self.session, self.options, root, br_arch, self.id, repo_id=repo_info['id'], setup_dns=True)
 

+             broot = BuildRoot(self.session, self.options, root, br_arch, self.id, repo_id=repo_info['id'], setup_dns=True,
 

+                               nspawn_args=opts.nspawn_args)
 

              broot.workdir = self.workdir
 

          broot.init()
 

          rootdir = broot.rootdir()

For rpm-ostree we need

config_opts['nspawn_args'] = ['--capability=CAP_NET_ADMIN', '--as-pid2']

Which is now exposed via https://github.com/rpm-software-management/mock/pull/35

Hm, we may need a way to inject into the config, not the command line. Will look.

rebased
7 years ago

Reworked to just do nspawn_args, and write to the mock config. Still not tested but seems likely to work!

Please, alter also tests/test_cli/test_runroot.py (just adding new option in the last call).

You are passing a new option to genMockConfig, but genMockConfig does not know to do anything with it.

Also, I am little wary of directly exposing this. I don't think runroot users should need to specify arbitrary nspawn args.

Do you have an alternative suggestion? The pungi rpm-ostree task should be moved into koji, and the nspawn arguments as well?

Do we expect much variation in these args? The minimal allowance would be a flag that adds these specific options rather than passing through arbitrary options.

Seems like --as-pid2 ought to be the default in this situation anyway. How often is mock going to run something in the chroot that is really prepared to act like init?

Yes; https://github.com/rpm-software-management/mock/pull/36

As far as the networking one - so rpm-ostree needs access to the network to fetch RPMs, but uses it to explicitly turn off networking for scripts. It's somewhat optional, but it's a stronger and more reliable mechanism than what people do today with Koji using firewalling rules.

So...we could make it optional, it wouldn't be hard but not entirely trivial either.

So...maybe we could just unconditionally turn on --capability CAP_NET_ADMIN for runroot?

(What things are using runroot other than rpm-ostree?)

commenting so that I can get notifications

@walters Other than the ostree, pungi is also using runroot to create isos (running genisoimage, isohybrid, implantmd5 etc.; this only uses filesystem access, no network needed). Lorax is run in runroot as well. I think it should not be doing network access either, the compose directory is mounted in the chroot. Lastly ostree installer is done in runroot, but that's just a sligtly different run of lorax.

Actually nevermind, since pungi runs rpm-ostree as a subprocess, we don't need the --as-pid2 (though it's still a good idea in the future, since pungi-make-ostree is not designed to be pid 1).

I'm going to change rpm-ostree to detect and work around the lack of CAP_NET_ADMIN.

Pull-Request has been closed by walters
7 years ago

Actually nevermind, since pungi runs rpm-ostree as a subprocess, we don't need the --as-pid2 (though it's still a good idea in the future, since pungi-make-ostree is not designed to be pid 1).

I will point out that mock now sets --as-pid2 by default for systemd-nspawn.

Metadata
None
No Tags
Changes Summary 3
+4 -1
file changed
builder/kojid
+4 -0
file changed
cli/koji
+3 -2
file changed
plugins/builder/runroot.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.