Add UCX-Py testing by pentschev · Pull Request #7412 · openucx/ucx

pentschev · 2021-09-15T20:15:35Z

What

Add recipe to run UCX-Py tests.

Why ?

We intend to completely upstream UCX-Py to the mainline UCX repository over the next few months. By beginning to test it here, we can ensure a smooth transition.

pentschev · 2021-09-15T20:16:13Z

@petro-rudenko could you help me validate I'm not missing anything?

@quasiben @yosefe FYI

ghost · 2021-09-15T20:19:22Z

Can one of the admins verify this patch?

shamisp · 2021-09-15T21:27:03Z

Thanks for doing this !

yosefe · 2021-09-16T11:04:21Z

+  - stage: ucx-py
+    dependsOn: [Static_check]
+    jobs:
+      - template: ucx-py/ucx-py-test.yml


we should use "python" naming in the context of UCX python bindings (not "ucx-py")
it would be consistent with "java" and "go"

But java bindings are named jucx: https://github.com/openucx/ucx/tree/master/buildlib/jucx . I can rename that if you prefer, it's just that ucx-py is the package name we use currently.

I prefer to use this opportunity to rename it to "python", as it becomes part of UCX, also to avoid confusion with the already existing ucx-py project. Re java - moving forward may we will rename it as well, but will be good to start with consistent naming for all bindings.

Ok, for now I renamed the directory/files to python in 41155ea, but the package will continue to be named ucx-py, we will need to discuss how to better address renaming the package.

let's also rename lines 180, 183, 188 and other places from "ucx-py" to python , and just keep the package name "ucx-py"

yosefe · 2021-09-16T11:05:22Z

+          source buildlib/az-helpers.sh
+          az_init_modules
+          try_load_cuda_env
+          set -eE


can we set -xeE in line 18?

I don't know, we can try, but this was copied from jucx that does it in that order:

ucx/buildlib/jucx/jucx-test.yml

Lines 45 to 57 in e0bb58f

set -x

source buildlib/az-helpers.sh

az_init_modules

try_load_cuda_env

res=0

az_module_load dev/mvn

res=$(($res+$?))

az_module_load dev/jdk-${JAVA_VERSION}

res=$(($res+$?))

if [ $res -ne 0 ]; then

exit 0;

fi

set -eE

@petro-rudenko WDYT?

yosefe · 2021-09-16T11:07:18Z

+          source buildlib/az-helpers.sh
+          az_init_modules
+          try_load_cuda_env
+          set -eE


can we set -xeE in line 30?

Same as in #7412 (comment)

yosefe

overall LGTM, besides

pipeline does not really run - need to check why
"ucx-py" -> "python" in several places
"set -xeE" once in several places

yosefe · 2021-09-16T20:33:42Z

Pipeline error :https://dev.azure.com/ucfconsort/ucx/_build/results?buildId=25897&view=results

pentschev · 2021-09-16T20:39:42Z

What should be the module names for commits here, AZP or something else?

yosefe · 2021-09-16T21:17:49Z

What should be the module names for commits here, AZP or something else?

AZP/PY: ...

pentschev · 2021-09-16T22:02:19Z

Hmm, I'm not sure why codestyle commit title is still failing, is it because of the merge commit? Do I need to get rid of it as well?

yosefe · 2021-09-16T22:09:24Z

Hmm, I'm not sure why codestyle commit title is still failing, is it because of the merge commit? Do I need to get rid of it as well?

It's probably because of missing semicolon in 76e6b75

yosefe · 2021-09-16T22:10:08Z

BTW, better squash all those to single commit now (since needed to force push anyway in order to fix commit titles)

pentschev · 2021-09-16T22:28:15Z

There we go, nothing that half an hour fighting git rebase won't do...

pentschev · 2021-09-17T14:48:04Z

The Python test seems to point out to be failing because it's missing gpuci_logger, and that's part of the conda docker image. This makes me think that for some reason the Python build is not picking up the correct container. I couldn't find any mention in the logs as to what docker container is being used, is it possible to verify that the right container is picked?

yosefe · 2021-09-17T17:40:34Z

The Python test seems to point out to be failing because it's missing gpuci_logger, and that's part of the conda docker image. This makes me think that for some reason the Python build is not picking up the correct container. I couldn't find any mention in the logs as to what docker container is being used, is it possible to verify that the right container is picked?

Looks like it doesn't run in a container at all: the step : "Initialize containers" is missing

Looks like you passed the container as a parameter, but not really using it

pentschev · 2021-09-17T18:23:30Z

Looks like it doesn't run in a container at all: the step : "Initialize containers" is missing

Do we have an example of how to do that somewhere? I thought https://github.com/openucx/ucx/pull/7412/files#diff-a467360eb428ca6bcb03d8b3b0f3f0d7f10aa1a9a1c11f23d55bffdb9a5be05fR187 should be doing that, no?

pentschev · 2021-09-17T18:25:25Z

Looks like it doesn't run in a container at all: the step : "Initialize containers" is missing

Do we have an example of how to do that somewhere? I thought https://github.com/openucx/ucx/pull/7412/files#diff-a467360eb428ca6bcb03d8b3b0f3f0d7f10aa1a9a1c11f23d55bffdb9a5be05fR187 should be doing that, no?

Or perhaps I have to do it in the job yaml file too, as in

ucx/buildlib/pr/codestyle.yml

Line 41 in 0d1e317

container: fedora

?

yosefe · 2021-09-17T18:52:21Z

Or perhaps I have to do it in the job yaml file too, as in

Yes

pentschev · 2021-09-17T20:47:16Z

It seems now we're running into different errors, seemingly it can't reach the docker image server:

Starting: Initialize containers
/usr/bin/docker version --format '{{.Server.APIVersion}}'
'1.41'
Docker daemon API version: '1.41'
/usr/bin/docker version --format '{{.Client.APIVersion}}'
'1.41'
Docker client API version: '1.41'
/usr/bin/docker ps --all --quiet --no-trunc --filter "label=fad461"
/usr/bin/docker network prune --force --filter "label=fad461"
/usr/bin/docker pull rdmz-harbor.rdmz.labs.mlnx/ucx/conda
Error response from daemon: Get "https://rdmz-harbor.rdmz.labs.mlnx/v2/": dial tcp 10.224.1.1:443: connect: connection refused
Using default tag: latest
##[warning]Docker pull failed with exit code 1, back off 7.605 seconds before retry.
/usr/bin/docker pull rdmz-harbor.rdmz.labs.mlnx/ucx/conda
Using default tag: latest
Error response from daemon: Get "https://rdmz-harbor.rdmz.labs.mlnx/v2/": dial tcp 10.224.1.1:443: connect: connection refused
##[warning]Docker pull failed with exit code 1, back off 4.264 seconds before retry.
/usr/bin/docker pull rdmz-harbor.rdmz.labs.mlnx/ucx/conda
Using default tag: latest
Error response from daemon: Get "https://rdmz-harbor.rdmz.labs.mlnx/v2/": dial tcp 10.224.1.1:443: connect: connection refused
##[error]Docker pull failed with exit code 1
Finishing: Initialize containers

Perhaps some network instability?

yosefe · 2021-09-17T21:31:39Z

Perhaps some network instability?

@avildema WDYT?

avildema · 2021-09-20T07:59:45Z

Perhaps some network instability?

@avildema WDYT?

No , It's error in pipeline, Need to set correct tag for docker

avildema · 2021-09-20T08:01:49Z

      image: rdmz-harbor.rdmz.labs.mlnx/ucx/fedora34:2
      options: -v /hpc/local:/hpc/local -v /auto/sw_tools:/auto/sw_tools
+    - container: conda
+      image: rdmz-harbor.rdmz.labs.mlnx/ucx/conda


please set correct tag for docker

What's the correct tag here? I don't set it anywhere and believe this is done when building the Docker container, I don't know where that's handled but it doesn't seem to be from the UCX code repo.

you should build and push docker to hardor , when you push docker you set tag for your image

I don't think I have access to that network. Could someone from your team please do it?

@avildema can you pls assist with pushing the docker image to harbor?

Actually, it seems that network is indeed reachable from part of the network I'm connected to. I don't have access to the machine nevertheless, so if you prefer we can sync on how I can get access to it so I can build the image.

avildema · 2021-09-20T08:04:46Z

+      - template: python/python-test.yml
+        parameters:
+          name: new
+          demands: ucx_new -equals yes


make sure that docker installed for new host

I also don't know who should handle this.

yosefe · 2021-09-20T17:27:53Z

+          demands: ucx_new -equals yes
+          container: conda
+      - template: python/python-test.yml
+        parameters:
+          name: gpu
+          demands: ucx_gpu -equals yes
+          container: conda


@pentschev for this PR, please run only with this:
demands: ucx_docker -equals yes
otherwise, you would either run on a machine that does not run a docker daemon (ucx_new) or on a machine that already runs ucx bare-metal (ucx_gpu). Currently , until we fix the issue, running a docker test on the same machine as the bare metal test will cause some bare metal tests to fail because of using the temporary docker bridge interface.

I'll update it. Does ucx_docker have GPU(s)?

unfortunately no, but should still be able to run ucx-py right?
let's add gpu dimension in next or after fixing the above issue

Barely, I think, I don't recall us every running on non-GPU machines.. Our main focus is indeed GPUs, so for a first pass it's ok, but we really need GPU support for any reasonable UCX-Py coverage.

yosefe · 2022-02-03T19:30:18Z

@pentschev FYI now that we merged #7212, we are running CI GPU tests in docker containers; and issue #7412 (comment) is fixed by #7873
So we should be able to run UCX-py on "ucx_gpu" label

pentschev · 2022-02-04T22:22:00Z

Thanks @yosefe for the ping. I'll be out next week but will follow-up here on the week after.

gleon99 · 2026-07-02T13:37:27Z

Hi @pentschev - we're cleaning up the UCX PR backlog and plan to close this PR in 7 days. Please let us know if you'd like to keep it open.

pentschev · 2026-07-02T13:58:41Z

Thanks for the ping @gleon99 . This is superseded by #11473 so no reason to keep it open. Closing.

yosefe reviewed Sep 16, 2021

View reviewed changes

Comment thread buildlib/pr/python/python-test.yml Outdated

Comment thread buildlib/pr/python/python-test.yml Outdated

pentschev force-pushed the ucx-py-ci branch from 4833cd8 to b2e708d Compare September 16, 2021 21:56

pentschev force-pushed the ucx-py-ci branch from 4f1dec9 to d324b84 Compare September 16, 2021 22:05

pentschev force-pushed the ucx-py-ci branch 4 times, most recently from 27f8b33 to cfa3e50 Compare September 16, 2021 22:24

AZP/PY: Enable UCX-Py test in PR recipe

cc62bb9

pentschev force-pushed the ucx-py-ci branch from cfa3e50 to cc62bb9 Compare September 16, 2021 22:27

AZP/PY: Fix conda docker image name

f87ce10

AZP/PY: Add missing container specification for Python tests

437badb

avildema reviewed Sep 20, 2021

View reviewed changes

yosefe reviewed Sep 20, 2021

View reviewed changes

AZP/PY: Constrain Python tests to ucx_docker

4f46978

petro-rudenko mentioned this pull request Sep 22, 2021

AZP: use -xeE for JUCX and Go tests. #7438

Merged

pentschev closed this Jul 2, 2026

	set -x
	source buildlib/az-helpers.sh
	az_init_modules
	try_load_cuda_env
	res=0
	az_module_load dev/mvn
	res=$(($res+$?))
	az_module_load dev/jdk-${JAVA_VERSION}
	res=$(($res+$?))
	if [ $res -ne 0 ]; then
	exit 0;
	fi
	set -eE

Uh oh!

Conversation

pentschev commented Sep 15, 2021 • edited by yosefe Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why ?

Uh oh!

pentschev commented Sep 15, 2021

Uh oh!

ghost commented Sep 15, 2021

Uh oh!

shamisp commented Sep 15, 2021

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

yosefe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

yosefe commented Sep 16, 2021

Uh oh!

pentschev commented Sep 16, 2021

Uh oh!

yosefe commented Sep 16, 2021

Uh oh!

pentschev commented Sep 16, 2021

Uh oh!

yosefe commented Sep 16, 2021

Uh oh!

yosefe commented Sep 16, 2021

Uh oh!

pentschev commented Sep 16, 2021

Uh oh!

pentschev commented Sep 17, 2021

Uh oh!

yosefe commented Sep 17, 2021

Uh oh!

pentschev commented Sep 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pentschev commented Sep 17, 2021

Uh oh!

yosefe commented Sep 17, 2021

Uh oh!

pentschev commented Sep 17, 2021

Uh oh!

yosefe commented Sep 17, 2021

Uh oh!

avildema commented Sep 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pentschev commented Sep 15, 2021 •

edited by yosefe

Loading

pentschev commented Sep 17, 2021 •

edited

Loading

avildema commented Sep 20, 2021 •

edited

Loading