Refactor Copr build to focus on chroot

[ehelms]

The Copr build functionality was on partially considering the chroot when looking at if a build exists. Further, there was a lot of copied code between release and scratch that is slimmed down here to allow the by chroot method to properly work.

By and large, I needed to lean more into doing everything by chroot and this includes existence. So I've removed the dedicated check for existence and instead turned copr_build into a more idempotent module that checks existence and acts based upon this.

[ehelms]

@evgeni I am needing som ehelp if you have any thoughts on why this is happening. First look at this output:

  TASK [build_package : Run build] ***********************************************
  failed: [hello] (item=epel-7-x86_64) => changed=false 
    ansible_loop_var: chroot
    chroot: epel-7-x86_64
    command: rpm --query --queryformat %{name} /tmp/SRPMs/hello-2.10-1.src.rpm --undefine dist
    msg: hello / None
    output: hello

That msg: is stdout / stderr. For some reason this is performing the right call, and returning the right information, but it's throwing a 1 exit code with no other obvious actual error. And I do not encounter this error locally.

[ehelms]

@evgeni Thanks for the fix! Now can I ask for a review too? :)

[evgeni]

I know this is copied code, but I wonder if shell=True is really needed here.

[ehelms]

Without that we get:

      File "/tmp/ansible_copr_build_payload_5hf5no4a/ansible_copr_build_payload.zip/ansible/module_utils/obal.py", line 27, in macro_lookup
      File "/usr/lib64/python3.10/subprocess.py", line 421, in check_output
        return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
      File "/usr/lib64/python3.10/subprocess.py", line 503, in run
        with Popen(*popenargs, **kwargs) as process:
      File "/usr/lib64/python3.10/subprocess.py", line 971, in __init__
        self._execute_child(args, executable, preexec_fn, close_fds,
      File "/usr/lib64/python3.10/subprocess.py", line 1847, in _execute_child
        raise child_exception_type(errno_num, err_msg, err_filename)
    FileNotFoundError: [Errno 2] No such file or directory: 'rpmquery --queryformat %{name} --package /tmp/SRPMs/hello-2.10-1.src.rpm --undefine dist'

[evgeni]

Haha. Yeah. Drop that join and pass a list, then it should work.

[ehelms]

Lets see what the tests say...

[evgeni]

I think we need to drop the quotes (when constructing the define) now, as there is no shell anymore and thus no quoting is needed

[ehelms]

That seems to have done it.

[evgeni]

Does this change mean that for a package that has multiple chroots defined, we now execute multiple copr_builds in sequence, while before it would happen in one go (as copr build … --chroot a --chroot b would just instruct Copr to "do the right thing")?

Not blocking per se, but something that can come bite us at some point when we add EL9

(We already have this "issue" today with the non-tito koji builds, so it's strictly speaking not a regression [anymore])

[ehelms]

Does this change mean that for a package that has multiple chroots defined, we now execute multiple copr_builds in sequence, while before it would happen in one go (as copr build … --chroot a --chroot b would just instruct Copr to "do the right thing")?

We do execute multiple copr_build in sequence. This was done as it's easier to think about each chroot independently when doing reporting to something like Jenkins. What is your concern of how this would bite us when we add EL9?

[evgeni]

It doubles the build time when it builds el8 and then 9

[ehelms]

It doubles the build time when it builds el8 and then 9

I attempt to reduce this by not waiting for the builds to finish and instead watching the tasks, that is it goes:

1. Kickoff build for chroot A
2. Kickoff build for chroot B
3. Wait for A and B to finish

https://github.com/theforeman/obal/pull/356/files#diff-8cc989782b3675c416f8801bcec650370b3c19ea858850bf8a76f6ad048f4cb7R60-R71

[evgeni]

Oh right right!

[ehelms]

Updated with comments