Docker workflow #1105

rbanka1 · 2025-02-18T17:34:57Z

Add Docker workflow
changes:

created docker build and push workflow
created ubuntu 24.04 docker image
modified reuseable_fast and basic to work on docker image
modified pr/push to run build image when needed
added few installation to docker files

Checklist

Code compiles without errors locally
All tests pass locally
CI workflows execute properly

CI workflows, not executed per PR (e.g. Nightly), execute properly
New tests added, especially if they will fail without my changes
Added/extended example(s) to cover this functionality
Extended the README/documentation
All newly added source files have a license
All newly added source files are referenced in CMake files
Logger (with debug/info/... messages) is used

changes: - created docker build and push workflow - created ubuntu 24.04 docker image - modified reuseable_fast and basic to work on docker image - modified pr/push to run build image when needed - added few installation to docker files

.github/docker/ubuntu-20.04.Dockerfile

.github/workflows/reusable_fast.yml

.github/docker/ubuntu-20.04.Dockerfile

.github/docker/ubuntu-22.04.Dockerfile

.github/workflows/reusable_fast.yml

PatKamin · 2025-02-26T12:16:24Z

.github/docker/ubuntu-20.04.Dockerfile

@@ -29,7 +29,10 @@ ARG UMF_DEPS="\

 # Dependencies for tests (optional)
 ARG TEST_DEPS="\
-	libnuma-dev"
+	libnuma-dev \
+	libhwloc-dev \


libhwloc and libtbb are already included in UMF_DEPS - in all Dockerfiles

you have right, I will remove it

heh, nope - you should not remove the UMF_DEPS. These ARGs are not only to list all dependencies, this is also to show which ones are required for using the library, and which are required e.g. for testing.

I believe tbb is not required to compile UMF, but it's required for tests. hwloc is required for UMF, so could be removed from test deps.

Okay that's said - there's another issue that's connected.
libhwloc-dev should be installed only on newer Ubuntu, here on Ubuntu-20.04 we should not install it and only install it via our script.
So in this OS, pls move HWLOC_DEPS up, add no UMF_DEPS, but please make a comment that hwloc is required as UMF dependency.
On other OSes, please remove the hwloc script and HWLOC_DEPS, and use UMF_DEPS equal to libhwloc-dev.

PatKamin · 2025-02-26T12:19:01Z

.github/workflows/build_ci_container.yml

+  contents: read
+
+jobs: 
+  build-ci-container:


I think this job shall be limited to upstream only:

if: github.repository == 'oneapi-src/unified-memory-framework'

I'm not sure for that. As we were talking, I left it untouched

could you try and add that, pls? I think it should work just fine; at least we should test it.

Of course as long as you're testing you can omit adding it 😉

.github/workflows/pr_push.yml

PatKamin · 2025-02-26T13:04:12Z

.github/workflows/reusable_basic.yml

+      image: ${{ matrix.image }}
+      options: --user root --privileged
+      volumes:
+        - /home/runner/work/unified-memory-framework/unified-memory-framework:/home/runner/work/unified-memory-framework/unified-memory-framework


Can ${{github.workspace}} be used in case it differs?

yes, i replaced it

PatKamin · 2025-02-26T13:17:34Z

.github/workflows/reusable_fast.yml

+        ${{matrix.extra_build_options}}
+
+    - name: Build
+      run: cmake --build ${{env.BUILD_DIR}} --config Release -j


-j $(nproc)

.github/workflows/reusable_fast.yml

PatKamin · 2025-02-26T13:20:21Z

.github/workflows/reusable_fast.yml

+    strategy:
+      matrix:
+        include:
+          - os: windows-latest


Remove this var as there is only one OS tested

.github/workflows/reusable_fast.yml

PatKamin · 2025-02-26T13:22:49Z

.github/workflows/reusable_fast.yml

@@ -91,6 +134,7 @@ jobs:
        -DCMAKE_PREFIX_PATH="${{env.VCPKG_PATH}}"


CMAKE_INSTALL_PREFIX not used, remove

I also left it because it wouldn't work without it https://github.com/rbanka1/unified-memory-framework/actions/runs/13549985319/job/37871871137

.github/workflows/reusable_fast.yml

lukaszstolarczuk · 2025-02-28T15:58:44Z

.github/docker/ubuntu-20.04.Dockerfile

@@ -29,7 +29,10 @@ ARG UMF_DEPS="\

 # Dependencies for tests (optional)
 ARG TEST_DEPS="\
-	libnuma-dev"
+	libnuma-dev \
+	libhwloc-dev \


heh, nope - you should not remove the UMF_DEPS. These ARGs are not only to list all dependencies, this is also to show which ones are required for using the library, and which are required e.g. for testing.

I believe tbb is not required to compile UMF, but it's required for tests. hwloc is required for UMF, so could be removed from test deps.

Okay that's said - there's another issue that's connected.
libhwloc-dev should be installed only on newer Ubuntu, here on Ubuntu-20.04 we should not install it and only install it via our script.
So in this OS, pls move HWLOC_DEPS up, add no UMF_DEPS, but please make a comment that hwloc is required as UMF dependency.
On other OSes, please remove the hwloc script and HWLOC_DEPS, and use UMF_DEPS equal to libhwloc-dev.

.github/docker/ubuntu-20.04.Dockerfile

.github/docker/ubuntu-22.04.Dockerfile

.github/docker/ubuntu-24.04.Dockerfile

.github/workflows/reusable_fast.yml

.github/workflows/reusable_basic.yml

.github/workflows/pr_push.yml

.github/workflows/reusable_basic.yml

lukaszstolarczuk · 2025-02-28T17:18:15Z

.github/workflows/pr_push.yml

+          ALL_CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
+        run: |
+          echo "Changed files: ${{ steps.changed-files.outputs.all_changed_files }}"
+  BuildDockers:


pls verify if detectChanges works properly when we merge a PR - pls test scenarios when you merged PR with and without any docker changes

done, test with changes https://github.com/rbanka1/unified-memory-framework/actions/runs/13627636943
test without https://github.com/rbanka1/unified-memory-framework/actions/runs/13627807242/job/38088708419

hmm tbh I'm not sure what has been tested here...? these were not a "test PRs" being merged into main, right? I was thinking - you push your new workflow (with building docker) into your main, then open 2 PRs on your fork and see if they are working properly (on PR). Later, test if after merging they are still working properly.

// for testing purpose I feel like you have to use different actor and set token in secrets

lukaszstolarczuk · 2025-03-03T13:12:26Z

.github/workflows/pr_push.yml

+          ALL_CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
+        run: |
+          echo "Changed files: ${{ steps.changed-files.outputs.all_changed_files }}"
+  BuildDockers:


hmm tbh I'm not sure what has been tested here...? these were not a "test PRs" being merged into main, right? I was thinking - you push your new workflow (with building docker) into your main, then open 2 PRs on your fork and see if they are working properly (on PR). Later, test if after merging they are still working properly.

// for testing purpose I feel like you have to use different actor and set token in secrets

lukaszstolarczuk · 2025-03-03T13:13:58Z

.github/workflows/reusable_basic.yml

-    - name: Install g++-7
-      if: matrix.compiler.cxx == 'g++-7'
-      run: sudo apt-get install -y ${{matrix.compiler.cxx}}
-
    - name: Install libhwloc
      run: .github/scripts/install_hwloc.sh


this step shouldn't be here - it's already covered in docker

lukaszstolarczuk · 2025-03-03T13:14:27Z

.github/workflows/reusable_basic.yml

@@ -181,11 +168,11 @@ jobs:
        ${{ matrix.compiler.cxx == 'icpx' && '. /opt/intel/oneapi/setvars.sh' || true }}
        LD_LIBRARY_PATH="${{env.BUILD_DIR}}/lib/:${LD_LIBRARY_PATH}" ctest --output-on-failure

-    - name: Check coverage
+    - name: Check os coverage


why this change in name?

lukaszstolarczuk · 2025-03-03T13:22:20Z

.github/workflows/reusable_dockers_build.yml

+        with: 
+          registry: ghcr.io
+          username: bb-ur
+          password: ${{ secrets.BB_GHCR_TOKEN }}


sooo, I just realized - this job won't work on PR, as there's no access to secrets. So, if I'm changing anything in the docker there's no way it's going to pass...

please propose some solution, as this will be a blocker (GHA cache? connecting docker building with Basic/Fast jobs execution...? something else...?)

lukaszstolarczuk · 2025-03-03T13:25:03Z

.github/workflows/reusable_basic.yml

      if:  ${{ matrix.build_type == 'Debug' && matrix.compiler.c == 'gcc' }}
      working-directory: ${{env.BUILD_DIR}}
      run: |
-        export COVERAGE_FILE_NAME=${{env.COVERAGE_NAME}}-${{matrix.os}}-shared-${{matrix.shared_library}}-no_hwloc-${{matrix.disable_hwloc}}
+        export COVERAGE_FILE_NAME=${{env.COVERAGE_NAME}}-${{matrix.ubuntu_ver}}-shared-${{matrix.shared_library}}-no_hwloc-${{matrix.disable_hwloc}}


ok, that's not good - the name of the artifact will be cov-22.04-shared... - we should definitely keep the old name with "ubuntu" in it - as we may have more OSes than just ubuntu - perhaps you should use the var as before os instead of ubuntu_ver -- so basically turn back what it was in matrix - os: ['ubuntu-20.04', 'ubuntu-22.04']

For unity, the same approach in FastBuilds - os: ['ubuntu-20.04', 'ubuntu-22.04'] instead of ubuntu_ver: <number>

lukaszstolarczuk · 2025-03-03T13:26:53Z

.github/docker/ubuntu-20.04.Dockerfile

-ARG UMF_DEPS="\
-	libhwloc-dev \
-	libtbb-dev"
+# libhwloc-dev is required


this comment should be a little more verbose - saying that it's installed here via script, as the hwloc version is too old on this OS

and please move HWLOC_DEPS and # Copy hwloc here, above.

.github/docker/ubuntu-24.04.Dockerfile

lukaszstolarczuk · 2025-03-03T13:32:52Z

.github/workflows/reusable_basic.yml

@@ -111,45 +118,25 @@ jobs:
            install_tbb: 'ON'
            disable_hwloc: 'OFF'
            link_hwloc_statically: 'ON'
-    runs-on: ${{matrix.os}}
-
    steps:
    - name: Checkout
      uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
      with:
        fetch-depth: 0



still missing tbb removal for matrix.install_tbb == 'OFF'

Add Docker workflow

527baa9

changes: - created docker build and push workflow - created ubuntu 24.04 docker image - modified reuseable_fast and basic to work on docker image - modified pr/push to run build image when needed - added few installation to docker files

rbanka1 force-pushed the DockerWorkflow branch from d1700d4 to 527baa9 Compare February 18, 2025 17:39

lukaszstolarczuk mentioned this pull request Feb 19, 2025

[WIP] Docker in the nightly testing #1033

Closed

10 tasks

lukaszstolarczuk reviewed Feb 19, 2025

View reviewed changes

corrections

25d9501

lukaszstolarczuk reviewed Feb 25, 2025

View reviewed changes

PatKamin reviewed Feb 26, 2025

View reviewed changes

rbanka1 added 2 commits February 28, 2025 08:26

corrections v2

3c7360d

add GH user

c854942

PatKamin reviewed Feb 28, 2025

View reviewed changes

.github/workflows/reusable_fast.yml Show resolved Hide resolved

.github/workflows/reusable_fast.yml Show resolved Hide resolved

corrections V3

c26684e

lukaszstolarczuk reviewed Feb 28, 2025

View reviewed changes

.github/workflows/reusable_basic.yml Outdated Show resolved Hide resolved

lukaszstolarczuk reviewed Feb 28, 2025

View reviewed changes

correctionsV4

eece86b

lukaszstolarczuk reviewed Mar 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Docker workflow #1105

Docker workflow #1105

rbanka1 commented Feb 18, 2025

PatKamin Feb 26, 2025

rbanka1 Feb 28, 2025

lukaszstolarczuk Feb 28, 2025

PatKamin Feb 26, 2025

rbanka1 Feb 28, 2025

lukaszstolarczuk Feb 28, 2025

PatKamin Feb 26, 2025

rbanka1 Feb 28, 2025

PatKamin Feb 26, 2025

rbanka1 Feb 28, 2025

PatKamin Feb 26, 2025

rbanka1 Feb 28, 2025

PatKamin Feb 26, 2025

rbanka1 Feb 28, 2025

lukaszstolarczuk Feb 28, 2025

lukaszstolarczuk Feb 28, 2025

rbanka1 Mar 3, 2025

lukaszstolarczuk Mar 3, 2025

lukaszstolarczuk Mar 3, 2025

lukaszstolarczuk Mar 3, 2025

lukaszstolarczuk Mar 3, 2025

lukaszstolarczuk Mar 3, 2025

lukaszstolarczuk Mar 3, 2025

lukaszstolarczuk Mar 3, 2025

lukaszstolarczuk Mar 3, 2025

		@@ -91,6 +134,7 @@ jobs:
		-DCMAKE_PREFIX_PATH="${{env.VCPKG_PATH}}"

Docker workflow #1105

Are you sure you want to change the base?

Docker workflow #1105

Conversation

rbanka1 commented Feb 18, 2025

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment