Arvados

22058-cwltest-plugin @ fcac5ad33bd215a7c7d7e15ef9f57e0ac8a02f24

This is the corresponding Arvados branch that to make use of the new file access plug-in interface implemented in cwltest here: https://github.com/common-workflow-language/cwltest/pull/233

developer-run-tests: #4564

Peter Amstutz wrote in #note-8:

developer-run-tests: #4564

def get_fsaccess():
    api_client = arvados.api()

Please pass 'v1' to arvados.api to avoid spurious warning spew.

Other than that, the code looks good to me. I have questions about process.

Is there a cwltest release that includes the support we need to be able to start using that now? If so, should this branch remove/update our cwltest version pins to make that happen? Or does it need to happen in a separate branch for some logistical reason?

If it happens in this branch, is it possible to run the CWL conformance tests on Jenkins against this update to show it all working?

Thanks.

Brett Smith wrote in #note-9:

Is there a cwltest release that includes the support we need to be able to start using that now? If so, should this branch remove/update our cwltest version pins to make that happen? Or does it need to happen in a separate branch for some logistical reason?

When I put this up for review on Friday, I'm not sure if Michael had made a new cwltest release yet, but that has since happened.

I have updated the pin in test_with_arvbox to set the minimum version to the new version.

The other pin is in the configuration for https://ci.arvados.org/view/Arvados%20build%20pipeline/job/run-tests-cwl-suite/

This gets arvados-cwl-runner from our packages and cwltest from pip. For cwltest to access Keep, an appropriate version of arvados-cwl-runner needs to be installed in the same environment as cwltest. We could get arvados-cwl-runner from git, or publish an appropriate dev package to PyPi, but these are not exactly ideal solutions as we end up with two different copies of arvados-cwl-runner which creates potential for confusion.

So it occurred to me, we could include cwltest in the arvados-cwl-runner package. What do you think?

If it happens in this branch, is it possible to run the CWL conformance tests on Jenkins against this update to show it all working?

22058-cwltest-plugin @ 522226b8f2c410a881bdba86cee02560accea2ba

arvados-cwl-conformance-tests: #1822

See note above about run-tests-cwl-suite.

Peter Amstutz wrote in #note-10:

I have updated the pin in test_with_arvbox to set the minimum version to the new version.

I think it would be a nice improvement if 4729a5ffb5ae8f5365f3522a27f9c5c161fe8cc7 used git -C instead of cd; git.

It's a maintainability thing. cd changes a global variable. When a shell script does a lot of cd it gets hard to maintain because you have to think about which steps rely on which $PWD when. Using git -C (or env -C or whatever) makes life easier long-term.

This gets arvados-cwl-runner from our packages and cwltest from pip. For cwltest to access Keep, an appropriate version of arvados-cwl-runner needs to be installed in the same environment as cwltest. We could get arvados-cwl-runner from git, or publish an appropriate dev package to PyPi, but these are not exactly ideal solutions as we end up with two different copies of arvados-cwl-runner which creates potential for confusion.

I think it would be best if we only got and installed a-c-r from Git. This would make it more consistent with the rest of our test jobs, which generally take a git_hash. It would give us the benefit of being able to test any a-c-r branch against the conformance suite; that would be a nice testing win for other large a-c-r branches. And the current Jenkins code is already checking out the a-c-r source from Git; it's just doing less with it. I don't think it would be much work to have it install and run from a virtualenv like arvbox does now.

If it helps, I'd be happy to make the Jenkins changes, it's the same darn virtualenv setup code I've written half a dozen times already.

Thanks.

Brett Smith wrote in #note-11:

This gets arvados-cwl-runner from our packages and cwltest from pip. For cwltest to access Keep, an appropriate version of arvados-cwl-runner needs to be installed in the same environment as cwltest. We could get arvados-cwl-runner from git, or publish an appropriate dev package to PyPi, but these are not exactly ideal solutions as we end up with two different copies of arvados-cwl-runner which creates potential for confusion.

I think it would be best if we only got and installed a-c-r from Git. This would make it more consistent with the rest of our test jobs, which generally take a git_hash. It would give us the benefit of being able to test any a-c-r branch against the conformance suite; that would be a nice testing win for other large a-c-r branches. And the current Jenkins code is already checking out the a-c-r source from Git; it's just doing less with it. I don't think it would be much work to have it install and run from a virtualenv like arvbox does now.

If it helps, I'd be happy to make the Jenkins changes, it's the same darn virtualenv setup code I've written half a dozen times already.

Thanks.

The rationale for using packages was to confirm that the OS packages as built work correctly.

The rationale for running the full test suite instead of a single job was that the cloud install uses more complex infrastructure than the containerized test (because all of this already runs as https://ci.arvados.org/view/CWL/job/arvados-cwl-conformance-tests/), so more comprehensive testing is more likely to turn up obscure problems.

I'm happy to hear counter-arguments that one or both of these have a poor return on investment and we should do something different, but that's the reason it doesn't already do what you're suggesting.

I should mention that what we did before was build and distribute a arvados-cwltest package specifically to do testing, but we ditched it at some point. Which is where I got the idea of folding cwltest into the arvados-cwl-runner package.

Okay, so working backwards:

I really don't want to add cwltest to the existing package. I think installing whole new bits of software on all our users' systems just for our testing convenience is a bad habit to be in. And it'll be one of those weird one-off exceptions in the package-building script that becomes inscrutable over time.

I don't think reintroducing the separate cwltest package will meet your needs either, because at least the way we were packaging that at the time, it was in a whole separate venv.

Testing the packaged version is a totally laudable goal (although it feels organizationally weird we're not doing that in the package tests, since that's kind of what they're there for and they can test packages before publication). What if we modified the Jenkins job to work like this:

• If you don't specify a package version, it builds a new venv and installs everything from source (it already takes a git_hash so there's not even anything to add)
• If you do specify a version, it installs that package version like now, then installs cwltest inside the venv at /usr/lib/python3-arvados-cwl-runner. Obviously this is not something you would want to do on a production system, but this isn't a production system so it's fine. And still straightforward.

IMO this meets all the existing needs, gets the Jenkins job working at least a little more like other test jobs, and the scripting is straightforward.

Brett Smith wrote in #note-14:

Testing the packaged version is a totally laudable goal (although it feels organizationally weird we're not doing that in the package tests, since that's kind of what they're there for and they can test packages before publication). What if we modified the Jenkins job to work like this:

Mainly because doing another 40 minutes of testing would make package building take forever (right now the package tests are just "can the package be installed" and "does the program start without immediately crashing") and the purpose of having "dev" packages is to be able to do further testing.

• If you don't specify a package version, it builds a new venv and installs everything from source (it already takes a git_hash so there's not even anything to add)
• If you do specify a version, it installs that package version like now, then installs cwltest inside the venv at /usr/lib/python3-arvados-cwl-runner. Obviously this is not something you would want to do on a production system, but this isn't a production system so it's fine. And still straightforward.

Installing it into a-c-r's venv is a nice workaround. Here is what it is doing now:

ACR_VERSION=$(./build/get-package-version.sh python3 sdk/cwl)
CLIENT_VERSION=$(./build/get-package-version.sh python3 sdk/python)

# Need to install the client package separately because the a-c-r package on its own 
# doesn't link tools like arv-get into /usr/bin
sudo apt-get install -y python3-arvados-cwl-runner=$ACR_VERSION python3-arvados-python-client=$CLIENT_VERSION

# install cwltest into the same venv used by a-c-r, because cwltest starting with 2.5.20241122133319
# looks for a Keep filesystem plugin that is exported by the a-c-r package as an entry point.
sudo /usr/lib/python3-arvados-cwl-runner/bin/pip3 install 'cwltest >=2.5.20241122133319,<3'
sudo ln -sf /usr/lib/python3-arvados-cwl-runner/bin/cwltest /usr/bin/cwltest

As you can see, it is deriving the package version from the checked out version (determined by git_hash). Note that for this to work, arvados/jobs needs to have been built, and arvados/jobs is built from packages, so it only makes sense to run this using commits that have already been through the packaging process.

run-tests-arvados-cwl: #1064

Peter Amstutz wrote in #note-15:

Mainly because doing another 40 minutes of testing would make package building take forever (right now the package tests are just "can the package be installed" and "does the program start without immediately crashing")

IMO the very limited scope of our current package testing is a problem, not a virtue. It is a contributing factor to bugs like #22349.

run-tests-arvados-cwl: #1064

LGTM.