PR#1717: Don't merge - just triggering CI - copr/copr

copr / copr

#1717 Don't merge - just triggering CI

Closed 3 years ago by praiskup. Opened 3 years ago by praiskup.

Unknown source dont-merge-me into main

cli, frontend: build --enable-net not set by default

Pavel Raiskup • 3 years ago

5756b41

build_aux: skip symlink files in linter

Pavel Raiskup • 3 years ago

ef43f40

frontend, backend: prepare for the centos-stream-8 rename

Pavel Raiskup • 3 years ago

eff7667

backend/run/copr-rename-chroot

file added

+93

		`@@ -0,0 +1,93 @@`
		`+ #! /usr/bin/python3`
		`+`
		`+ """`
		`+ Rename chroots, and keep backward compatibility link.`
		`+ """`
		`+`
		`+ import logging`
		`+ import os`
		`+ import sys`
		`+ import pwd`
		`+ import argparse`
		`+`
		`+ from copr_backend.helpers import BackendConfigReader`
		`+`
		`+ logging.basicConfig(level=logging.DEBUG)`
		`+ LOG = logging.getLogger()`
		`+`
		`+ parser = argparse.ArgumentParser(`
		`+ description=(`
		`+ "Sometimes we need to rename chroots both in Frontend DB and on "`
		`+ "Backend, synchronously. For example we removed 'centos-stream' to "`
		`+ "'centos-stream-8'. This script helps with the Backend part."))`
		`+ parser.add_argument(`
		`+ "--real-run",`
		`+ action='store_true',`
		`+ help=("Also perform the changes, not just checks"))`
		`+ parser.add_argument(`
		`+ "--pair",`
		`+ dest="pairs",`
		`+ action="append",`
		`+ help=("Add FROM:TO chroot name pairs, e.g. "`
		`+ "centos-stream-x86_64:centos-stream-8-x86_64"))`
		`+`
		`+`
		`+ def rename_in(path, pairs, dry_run=True):`
		`+ """`
		`+ Perform the rename of directories relatively to the PATH directory.`
		`+ PAIRS is dictionary for chroot_from => chroot_to mapping. When dry_run is`
		`+ True we only log.`
		`+ """`
		`+ success = True`
		`+ path = os.path.normpath(path)`
		`+ for directory, _, _ in os.walk(path):`
		`+ subdir = os.path.relpath(directory, path)`
		`+ parts = subdir.split(os.sep)`
		`+ depth = len(parts)`
		`+ if depth != 3:`
		`+ continue`
		`+`
		`+ chroot = parts[-1]`
		`+ if chroot in pairs:`
		`+ new_chroot = pairs[chroot]`
		`+ parent, _ = os.path.split(directory)`
		`+ new_directory = os.path.join(parent, new_chroot)`
		`+ if os.path.exists(new_directory):`
		`+ LOG.warning("'%s' already exists", new_directory)`
		`+ success = False`
		`+ try:`
		`+ LOG.info("Renaming %s to %s", directory, new_directory)`
		`+ if not dry_run:`
		`+ os.rename(directory, new_directory)`
		`+`
		`+ LOG.info("Linking %s -> %s", directory, new_chroot)`
		`+ if not dry_run:`
		`+ os.symlink(new_chroot, directory)`
		`+`
		`+ except OSError:`
		`+ LOG.exception("Failed to rename or link")`
		`+`
		`+ return success`
		`+`
		`+`
		`+ def _main():`
		`+ args = parser.parse_args()`
		`+ if not args.real_run:`
		`+ LOG.warning("Just doing dry run, run with --real-run")`
		`+`
		`+ config_file = os.environ.get("BACKEND_CONFIG", "/etc/copr/copr-be.conf")`
		`+ opts = BackendConfigReader(config_file).read()`
		`+ pairs = {}`
		`+ for pair in args.pairs:`
		`+ from_name, to_name = pair.split(":")`
		`+ pairs[from_name.strip()] = to_name.strip()`
		`+`
		`+ rename_in(opts.destdir, pairs, not args.real_run)`
		`+`
		`+`
		`+ if __name__ == "__main__":`
		`+ if pwd.getpwuid(os.getuid())[0] != "copr":`
		+ print("This script should be executed under the `copr` user")
		`+ sys.exit(1)`
		`+ else:`
		`+ _main()`

build_aux/linter

file modified

		`@@ -41,6 +41,10 @@`
		`"""`
		`if filename.endswith(".py"):`
		`return 'python'`
		`+`
		`+ if os.path.islink(filename):`
		`+ return 'unknown'`
		`+`
		`with open(filename) as f_d:`
		`first_line = f_d.readline()`
		`if first_line.startswith('#!') and first_line.find('python') != -1:`

cli/copr_cli/main.py

file modified

+10 -3

		`@@ -112,9 +112,12 @@`
		`"timeout": args.timeout,`
		`"chroots": args.chroots,`
		`"background": args.background,`
		`- "enable_net": ON_OFF_MAP[args.enable_net],`
		`"progress_callback": progress_callback,`
		`}`
		`+`
		`+ if args.enable_net is not None:`
		`+ buildopts["enable_net"] = ON_OFF_MAP[args.enable_net]`
		`+`
		`for opt in ["exclude_chroots", "bootstrap", "after_build_id", "with_build_id", "isolation"]:`
		`value = getattr(args, opt)`
		`if value is not None:`
		`@@ -1188,8 +1191,12 @@`
		`help="Mark the build as a background job. It will have lesser priority than regular builds.")`
		`parser_build_parent.add_argument("--isolation", choices=["simple", "nspawn", "default"], default="unchanged",`
		`help="Choose the isolation method for running commands in buildroot.")`
		`- parser_build_parent.add_argument("--enable-net", choices=["on", "off"], default="off",`
		`- help="If net should be enabled for this build (default is off)")`
		`+`
		`+ parser_build_parent.add_argument(`
		`+ "--enable-net",`
		`+ choices=["on", "off"],`
		`+ help=("If net should be enabled for this build (by default "`
		`+ "the project configuration is used, see 'modify' command)."))`

		`parser_build_parent.add_argument(`
		`"--bootstrap",`

cli/tests/test_distgit.py

file modified

+5 -5

		`@@ -42,7 +42,6 @@`
		`'background': False,`
		`'progress_callback': None,`
		`'isolation': 'unchanged',`
		`- 'enable_net': False,`
		`},`
		`'packagename': 'test',`
		`'distgit': None,`
		`@@ -69,13 +68,14 @@`
		`call = f_patch_create_from_distgit.call_args_list[0]`
		`assert call[1] == self.default_build_call`

		`- def test_full_featured_distgit_build(self, f_patch_create_from_distgit,`
		`- capsys):`
		`+ @pytest.mark.parametrize('enable_net', ["on", "off"])`
		`+ def test_full_featured_distgit_build(self, enable_net,`
		`+ f_patch_create_from_distgit, capsys):`
		`_assert_output(`
		`['build-distgit', '--name', 'test', '@group/project', '--nowait',`
		`'--timeout', "3600", '--chroot', 'fedora-rawhide-x86_64',`
		`'--distgit', 'centos', '--commit', 'f19', '--namespace',`
		`- 'rpms', "--background"],`
		`+ 'rpms', "--background", "--enable-net", enable_net],`
		`self.build_1, "",`
		`capsys)`
		`assert len(f_patch_create_from_distgit.call_args_list) == 1`
		`@@ -92,7 +92,7 @@`
		`"background": True,`
		`"progress_callback": None,`
		`'isolation': 'unchanged',`
		`- 'enable_net': False,`
		`+ "enable_net": enable_net == "on",`
		`},`
		`})`
		`assert call[1] == result`

frontend/coprs_frontend/alembic/versions/6b48324e9264_disallow_dash_in_os_release.py

file added

+24

		`@@ -0,0 +1,24 @@`
		`+ """`
		`+ disallow dash in os_release`
		`+`
		`+ Revision ID: 6b48324e9264`
		`+ Revises: 6866cd91c3c6`
		`+ Create Date: 2021-03-15 15:54:24.870411`
		`+ """`
		`+`
		`+ from alembic import op`
		`+`
		`+ revision = '6b48324e9264'`
		`+ down_revision = '6866cd91c3c6'`
		`+`
		`+ def upgrade():`
		`+ op.execute("""`
		`+ ALTER TABLE "mock_chroot"`
		`+ ADD CONSTRAINT "no_dash_in_version_check"`
		`+ CHECK ("os_version" NOT LIKE '%-%')`
		`+ """)`
		`+`
		`+ def downgrade():`
		`+ op.execute("""`
		`+ ALTER TABLE mock_chroot DROP CONSTRAINT "no_dash_in_version_check"`
		`+ """)`

frontend/coprs_frontend/coprs/forms.py

file modified

+18 -1

		`@@ -142,6 +142,23 @@`
		`)`


		`+ class BooleanFieldOptional(wtforms.BooleanField):`
		`+ """`
		`+ The same as BooleanField, but we make sure that None is used for self.data`
		`+ instead of False when no data were submitted in the form for this field.`
		`+ From web-ui it isn't normal situation, but from command-line client and`
		`+ Python API it is pretty normal that some fields are not set in POST data.`
		`+ And sometimes it is convenient to have three-state checkbox`
		`+ (True\|False\|None).`
		`+ """`
		`+ def process_formdata(self, valuelist):`
		`+ """ override parent's self.data decision when no value is sent """`
		`+ super().process_formdata(valuelist)`
		`+ if not valuelist:`
		`+ # pylint: disable=attribute-defined-outside-init`
		`+ self.data = None`
		`+`
		`+`
		`class MultiCheckboxField(wtforms.SelectMultipleField):`
		`widget = wtforms.widgets.ListWidget(prefix_label=False)`
		`option_widget = wtforms.widgets.CheckboxInput()`
		`@@ -1109,7 +1126,7 @@`
		`max=app.config["MAX_BUILD_TIMEOUT"])],`
		`default=app.config["DEFAULT_BUILD_TIMEOUT"])`

		`- F.enable_net = wtforms.BooleanField(false_values=FALSE_VALUES)`
		`+ F.enable_net = BooleanFieldOptional(false_values=FALSE_VALUES)`
		`F.background = wtforms.BooleanField(default=False, false_values=FALSE_VALUES)`
		`F.project_dirname = wtforms.StringField(default=None)`
		`F.bootstrap = create_mock_bootstrap_field("build")`

frontend/coprs_frontend/coprs/logic/coprs_logic.py

file modified

+9 -5

		`@@ -962,11 +962,15 @@`
		`@classmethod`
		`def tuple_from_name(cls, name, noarch=False):`
		`"""`
		`- input should be os-version-architecture, e.g. fedora-rawhide-x86_64`
		`-`
		`- the architecture could be optional with noarch=True`
		`-`
		`- returns ("os", "version", "arch") or ("os", "version", None)`
		`+ Input is either 'NAME-VERSION-ARCH' string or just 'NAME-VERSION',`
		`+ depending on the NOARCH input argument.`
		`+`
		`+ Note that to deterministically split the string into tuple using comma`
		`+ symbol, we can either allow comma to be part of the OS_NAME or`
		`+ OS_VERSION but not both. We somewhat artificially decided to allow`
		`+ dashes in name instead of version (i.e. that we interpret the string`
		`+ 'centos-stream-8-x86_64' as ("centos-stream", "8") instead of`
		`+ ("centos", "stream-8").`
		`"""`
		`split_name = name.rsplit("-", 1) if noarch else name.rsplit("-", 2)`

frontend/coprs_frontend/coprs/models.py

file modified

+3 -1

		`@@ -1428,12 +1428,14 @@`

		`__table_args__ = (`
		`db.UniqueConstraint('os_release', 'os_version', 'arch', name='mock_chroot_uniq'),`
		`+ db.CheckConstraint("os_version not like '%-%'",`
		`+ name="no_dash_in_release_check"),`
		`)`

		`id = db.Column(db.Integer, primary_key=True)`
		`# fedora/epel/..., mandatory`
		`os_release = db.Column(db.String(50), nullable=False)`
		`- # 18/rawhide/..., optional (mock chroot doesn"t need to have this)`
		`+ # 18/rawhide/..., can't contain '-' symbol, see tuple_from_name`
		`os_version = db.Column(db.String(50), nullable=False)`
		`# x86_64/i686/..., mandatory`
		`arch = db.Column(db.String(50), nullable=False)`

frontend/coprs_frontend/coprs/static/chroot_logodir/centos-stream.png

file added

		`@@ -0,0 +1,1 @@`
		`+ ../centos_logo.png`
		`\ No newline at end of file`

frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_builds.py

file modified

+3 -1

		`@@ -283,9 +283,11 @@`
		`'isolation': form.isolation.data,`
		`'after_build_id': form.after_build_id.data,`
		`'with_build_id': form.with_build_id.data,`
		`- 'enable_net': form.enable_net.data,`
		`}`

		`+ if form.enable_net.data is not None:`
		`+ generic_build_options['enable_net'] = form.enable_net.data`
		`+`
		`# From URLs it can be created multiple builds at once`
		`# so it can return a list`
		`build = create_new_build(generic_build_options)`

frontend/coprs_frontend/tests/test_apiv3/test_builds.py

file modified

+7 -1

		`@@ -87,7 +87,9 @@`
		`{},`
		`{`
		`"chroots": ["fedora-17-x86_64"],`
		`- }`
		`+ },`
		`+ {"enable_net": True},`
		`+ {"enable_net": False},`
		`]`


		`@@ -132,6 +134,10 @@`
		`assert r.status_code == 200`
		`build = self.models.Build.query.first()`

		`+ # We inherit enable_net config from project when not explicitly`
		`+ # set in build API call.`
		`+ assert build.enable_net == buildopts.get("enable_net", build.copr.build_enable_net)`
		`+`
		`enabled_chroots = set(['fedora-17-x86_64', 'fedora-17-i386'])`
		`if not form_data.get("chroots"):`
		`assert build.chroots == []`