cosalib: fix --build= argument passing

[dustymabe]

Passing --build= isn't working. I fixed it using this but I think
there is probably a better fix in order somewhere.

[dustymabe]

if you look at:

class QemuVariantImage(_Build):         
    def __init__(self, *args, **kwargs):

I don't see anywhere where *args is being used. It gets plumbed through in the call to _Build.__init__(self, *args, **kwargs) but I don't see that function using *args either.

[dustymabe]

to be clear the change in this PR works, but I think there is a larger scale problem with *args that needs to be fixed. There are several places in our pipelines where we call buildextend* with --build=$something and presumably right now that is being ignored, which could really bite us one day

[cgwalters]

Agree with the likely general issue.

Try rebasing this on master to fix Prow.

[dustymabe]

Agree with the likely general issue.

👍

Try rebasing this on master to fix Prow.

It says i'm already rebased on latest master

[dustymabe]

/retest

[cgwalters]

It says i'm already rebased on latest master

Did you git fetch and then git rebase origin/master?

[dustymabe]

$ git log --oneline  HEAD^^..HEAD
6b035bf3 (HEAD -> dusty-fix-build-arg, origin/dusty-fix-build-arg) WIP: fix cosa buildextend-gcp --build=
7175ed4e (upstreamorigin/master, upstreammaster) build-sys: Only build schema if source changed

7175ed4 is latest master, right?

[cgwalters]

7175ed4 is latest master, right?

Dude, that's so 8 minutes ago! Now it's ed72a92 😉

But...hmm. Maybe there's a race. Will investigate.

[jlebon]

Yeah, I think we can nuke *args entirely. The only other class that derives from it is Build in cmd-koji-upload and that one doesn't use *args either. Want to do that? If not that's fine too, we can do it as a follow up.

[dustymabe]

Yeah, I think we can nuke *args entirely. The only other class that derives from it is Build in cmd-koji-upload and that one doesn't use *args either. Want to do that? If not that's fine too, we can do it as a follow up.

Works for me. I'll update this PR. My intent was to get some expert opinions and figure out what the real fix should be.

[dustymabe]

OK. Pushed an update and updated the title of this PR. Here is the new commit message of the commit:

cosalib: fix --build= argument passing

I stumbled upon this problem because `cosa buildextend-gcp --build=xyz`
wasn't appropriately passing the build on to the underlying library.
It turns out that args was never getting used anywhere, only kwargs
were. Let's remove the *args pieces and use pass all arguments as
kwargs.

[jlebon]

+ sudo -u builder cosa buildextend-openstack
sudo: setrlimit(RLIMIT_CORE): Operation not permitted
[INFO]: CLI is a symlink for cmd-buildextends-openstack
[INFO]: Target 'OPENSTACK' is a Qemu Variant image
Traceback (most recent call last):
  File "/usr/lib/coreos-assembler/cmd-buildextend-openstack", line 114, in <module>
    artifact_cli().build_artifacts()
  File "/usr/lib/coreos-assembler/cmd-buildextend-openstack", line 91, in artifact_cli
    force=args.force, schema=args.schema)
  File "/usr/lib/coreos-assembler/cmd-buildextend-openstack", line 26, in get_builder
    return QVariants.QemuVariantImage(*args, **kargs)
TypeError: __init__() takes 1 positional argument but 3 were given
script returned exit code 1

I think we also need something like:

diff --git a/src/cmd-artifact-disk b/src/cmd-artifact-disk
index dbde1453..f3d62d37 100755
--- a/src/cmd-artifact-disk
+++ b/src/cmd-artifact-disk
@@ -14,8 +14,9 @@ import cosalib.vmware as VmwareOVA


 def get_builder(imgtype, build_root, build="latest", force=False, schema=None):
-    args = [build_root, build]
     kargs = {
+        "build": build,
+        "buildroot": build_root,
         "force": force,
         "schema": schema,
         "variant": imgtype

?

[dustymabe]

I think we also need something like:

👍 - updated

[jlebon]

Oh right, also need to drop passing *args to the QemuVariantImage and VmwareOVA constructors.

[dustymabe]

like this?

diff --git a/src/cosalib/vmware.py b/src/cosalib/vmware.py
index e9df1f4d..1b3b638c 100644
--- a/src/cosalib/vmware.py
+++ b/src/cosalib/vmware.py
@@ -52,10 +52,10 @@ class VmwareOVA(QemuVariantImage):
     https://www.dmtf.org/sites/default/files/standards/documents/DSP0243_1.1.0.pdf
     """
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, **kwargs):
         variant = kwargs.pop("variant", "vmware")
         kwargs.update(VARIANTS.get(variant, {}))
-        QemuVariantImage.__init__(self, *args, **kwargs)
+        QemuVariantImage.__init__(self, **kwargs)
         # Set the QemuVariant mutate_callback so that OVA is called.
         self.mutate_callback = self.write_ova
         # Ensure that coreos.ovf is included in the tar

[dustymabe]

yep and I found a few more: pushed an update.

[jlebon]

/lgtm

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dustymabe, jlebon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [dustymabe,jlebon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[cgwalters]

Every time I switch from a statically compiled language (particularly Rust which can be slow to compile) back to a dynamic one (python/shell) I usually have this feeling like OH MY GOD I'M GOING SO FAST then...at some point later crash -

+ coreos-assembler koji-upload --keytab /srv/.keytab/keytab --buildroot builds --owner rhcos-build/jenkins-redhat-coreos.cloud.paas.upshift.redhat.com@REDHAT.COM --profile brew-stage --tag rhaos-4.5-rhel-8-build
Traceback (most recent call last):
  File "/usr/lib/coreos-assembler/cmd-koji-upload", line 669, in <module>
    cli()
  File "/usr/lib/coreos-assembler/cmd-koji-upload", line 652, in cli
    build = Build(args.buildroot, build=args.build, arch=args.arch)
  File "/usr/lib/coreos-assembler/cmd-koji-upload", line 108, in __init__
    _Build.__init__(self, *args, **kwargs)
TypeError: __init__() takes 1 positional argument but 2 were given

[dustymabe]

Every time I switch from a statically compiled language (particularly Rust which can be slow to compile) back to a dynamic one (python/shell) I usually have this feeling like OH MY GOD I'M GOING SO FAST then...at some point later crash -

I know, right?? 🤣

+ coreos-assembler koji-upload --keytab /srv/.keytab/keytab --buildroot builds --owner rhcos-build/jenkins-redhat-coreos.cloud.paas.upshift.redhat.com@REDHAT.COM --profile brew-stage --tag rhaos-4.5-rhel-8-build
Traceback (most recent call last):
  File "/usr/lib/coreos-assembler/cmd-koji-upload", line 669, in <module>
    cli()
  File "/usr/lib/coreos-assembler/cmd-koji-upload", line 652, in cli
    build = Build(args.buildroot, build=args.build, arch=args.arch)
  File "/usr/lib/coreos-assembler/cmd-koji-upload", line 108, in __init__
    _Build.__init__(self, *args, **kwargs)
TypeError: __init__() takes 1 positional argument but 2 were given

Thanks!

Maybe #1323 will take care of it.