Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add OpenCL-SDK related images #32

Merged
merged 2 commits into from
May 30, 2024
Merged

Conversation

MathiasMagnus
Copy link
Contributor

These images are used throughout the OpenCL ecosystem, CI in OpenCL-Headers, OpenCL-ICD-Loader, OpenCL-CLHPP, OpenCL-Layers and OpenCL-SDK are all using these images. Currently they're stored under the Stream HPC docker account, but we would like to hand them over to Khronos.

@CLAassistant
Copy link

CLAassistant commented Oct 27, 2023

CLA assistant check
All committers have signed the CLA.

@mfep
Copy link
Contributor

mfep commented May 29, 2024

@rpavlik @oddhack Can you please review this PR?

The working group has made the decision that the CI/CD improvement PRs should be merged with using the khronosgroup/opencl* Docker images in the Linux jobs. This affects the following PRs:

I expect the CI on this PR to pass, see this job on our fork: Added OpenCL entries to build-all.sh

@oddhack
Copy link
Collaborator

oddhack commented May 30, 2024

It looks fine to me. If it passes CI then I will just merge it, this is purely additive and does as much reuse as possible so all good.

@oddhack
Copy link
Collaborator

oddhack commented May 30, 2024

One comment, IME github and gitlab CI both suffer from some weird caching issues when it comes to updating the image on Dockerhub such that just using the image name is not guaranteed to get you the current version of the image. We deal with this in Vulkan by using the image SHA when specifying the image in CI scripts. Tagging the images with dates as some of the XR images have done is also viable. It is unfortunate and annoying since the image specification has to be updated when pushing new images with new required components, OTOH at least you know you are using a known-good image.

@oddhack oddhack self-assigned this May 30, 2024
@oddhack
Copy link
Collaborator

oddhack commented May 30, 2024

@rpavlik I wonder if we should parallelize the build-all script? It takes a long time to run CI and should be trivial.

@oddhack
Copy link
Collaborator

oddhack commented May 30, 2024

CI passed, so merging.

@oddhack oddhack merged commit bfb2679 into KhronosGroup:main May 30, 2024
2 checks passed
@mfep mfep deleted the opencl branch May 30, 2024 14:23
@oddhack
Copy link
Collaborator

oddhack commented May 31, 2024

@MathiasMagnus now that the PR has been accepted, the other part is getting the images onto khronosgroup on dockerhub. Due to cost issues only a couple of us have write access to do that. If you are happy with how the Dockerfiles stand in main branch now, let me know and I'll go ahead and build / push the images for you, which will need to be repeated whenever changes to the Dockerfiles happen. It is unfortunately clunky.

@MathiasMagnus
Copy link
Contributor Author

@oddhack Yes, the images as they are submitted are currently fine. Please build and push the images so downstream repos can use it instead of the images under the Stream HPC DockerHub user. Ubuntu 24.04 will need to be tackled in the near-to-medium future, but 20.04 should still be tested and supported for the foreseeable future.

@oddhack
Copy link
Collaborator

oddhack commented Jun 14, 2024

Sorry for skipping this - the images have been pushed to khronosgroup/docker-images on dockerhub now. They were so large and consumed so much Docker build space that it nearly filled up my SSD - we will need to find another solution when these need to be updated in the future, probably a Khronos VM from whence the images can be built and pushed. Also BTW there are a large number of severe/critical vulnerabilities identified in the Ubuntu 20 images according to dockerhub, perhaps because the OS is so old.

@Beanavil
Copy link

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants