Skip to content

Conversation

aelovikov-intel
Copy link
Contributor

To be used for compatibility testing, i.e. running tests built with the latest released toolchain using trunk SYCL RT.

To be used for compatibility testing, i.e. running tests built with the
latest released toolchain using trunk SYCL RT.
sparse-checkout: |
devops/
- uses: actions/checkout@v4
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we name some of these steps so it looks better in the log?

ghcr.io/${{ github.repository }}/sycl_prebuilt_tests:${{ inputs.ref || github.ref }}
run-e2e:
runs-on: [Linux, pvc]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name would help here too since its PVC only

Copy link
Contributor Author

@aelovikov-intel aelovikov-intel Aug 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I think I need to provide some explanation here, not sure where it should go.

I'm not that interested in this task for the sake of it. What's happening here instead, is that I'll be using those pre-built E2E tests in trunk pre-commit, so I'm trying to show that they could be run successfully with the "original" toolchain the tests were built with.

With that context, any more detailed suggestions? Is PVC even the right runner to target here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry maybe i'm too tired but i have absolutely no idea what you mean :)

@aelovikov-intel
Copy link
Contributor Author

Can't test the new workflow to full extent because manual trigger needs a workflow at origin/sycl. The source branch's name is slightly incorrect too - should have been sycl-rel-trunk-test-ci-..., but that image is throwaway regardless.

I don't think I will be cherry-picking this to sycl-rel-6.2.0 branch as it depends on some previous CI PRs, so I'll use manual dispatches to keep the image up-to-date with the sycl-rel-6.2.0 updates.

For sycl-rel-6.3.0 I expect this PR to be part of the branch, so the push trigger will keep the image up-to-date automatically.

@KornevNikita
Copy link
Contributor

Need to call this script to bypass Trivy:

COPY scripts/create-sycl-user.sh /user-setup.sh
RUN /user-setup.sh

@aelovikov-intel
Copy link
Contributor Author

aelovikov-intel commented Aug 5, 2025

Need to call this script to bypass Trivy:

COPY scripts/create-sycl-user.sh /user-setup.sh
RUN /user-setup.sh

Didn't seem to help... Also, why would it be necessary? That was already done as part of the base image...

@KornevNikita
Copy link
Contributor

KornevNikita commented Aug 5, 2025

Can you give one more try?


Trivy report:

        {
          "Type": "Dockerfile Security Check",
          "ID": "DS002",
          "AVDID": "AVD-DS-0002",
          "Title": "Image user should not be 'root'",
          "Description": "Running containers with 'root' user can lead to a container escape situation. It is a best practice to run containers as non-root users, which can be done by adding a 'USER' statement to the Dockerfile.",
          "Message": "Specify at least 1 USER command in Dockerfile with non-root user as argument",
          "Namespace": "builtin.dockerfile.DS002",
          "Query": "data.builtin.dockerfile.DS002.deny",
          "Resolution": "Add 'USER \u003cnon root user name\u003e' line to the Dockerfile",
          "Severity": "HIGH",
          "PrimaryURL": "https://avd.aquasec.com/misconfig/ds002",
          "References": [
            "https://docs.docker.com/develop/develop-images/dockerfile_best-practices/",
            "https://avd.aquasec.com/misconfig/ds002"
          ],
          "Status": "FAIL",
          "CauseMetadata": {
            "Provider": "Dockerfile",
            "Service": "general"
          }
        }

@aelovikov-intel
Copy link
Contributor Author

Can you give one more try?

image

🎉 🎉 🎉

@aelovikov-intel
Copy link
Contributor Author

https://github.com/intel/llvm/actions/runs/16756327505 is the new workflow task, passed.

I think I've addressed all comments. @sarnex , @KornevNikita , can I ask for another (final? 😉) look?

Copy link
Contributor

@sarnex sarnex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just nits

ghcr.io/${{ github.repository }}/sycl_prebuilt_tests:${{ inputs.ref || github.ref_name }}
run-e2e:
# Ensure those test can actually pass with the toolchain they were built
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Ensure those test can actually pass with the toolchain they were built
# Ensure those tests can actually pass with the toolchain they were built

sycl-ls
- name: Run E2E tests
uses: ./devops/actions/run-tests/e2e
timeout-minutes: 60
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we're using prebuilt tests so we can probably be more conservative, probably 30 is enough

using: "composite"
steps:
- name: Checkout E2E tests
if: ${{ inputs.testing_mode != 'run-only' || inputs.binaries_artifact }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we unify how we are checking inputs.binaries_artifact? here we just rely on non-empty being true but below we explicitly check it like inputs.binaries_artifact != '', i have no opinion which one we do but would be a bit cleaner to always do the same thing

@aelovikov-intel aelovikov-intel merged commit 457ef60 into sycl Aug 5, 2025
49 of 53 checks passed
@aelovikov-intel aelovikov-intel deleted the sycl-rel-6_2-test-ci-prebuilt-e2e branch August 5, 2025 19:39
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.

3 participants