interTwin-eu / interlink-slurm-plugin

MIT License
1 stars 3 forks source link

Adding a image prefix like docker:// for singularity #30

Open antoinetran opened 1 month ago

antoinetran commented 1 month ago

Short Description of the issue

When applying an image like alpine, the singularity fails to run because it does not know it is a docker image.

Environment

Steps to reproduce

Logs, stacktrace, or other symptoms

kubectl -n argo-workflows logs app-lsdsharedfs-template-gnj4q-group-block-1706385343 -c init
FATAL:   While checking container encryption: could not open image /home/username/.interlink/quay.io/argoproj/argoexec:v3.5.4: failed to retrieve path for /home/username/.interlink/quay.io/argoproj/argoexec:v3.5.4: lstat /home/username/.interlink/quay.io: no such file or directory

Summary of proposed changes

Allow to configure Slurm plugin to add, for all image: if it does not start with / (meaning an absolute path to a local image), then add a prefix like docker:// or oras:// or nothing if the prefix configuration is absent.

antoinetran commented 1 month ago

I think this is already implemented through https://github.com/interTwin-eu/interlink-slurm-plugin/blob/main/examples/config/SlurmConfig.yaml#L7C1-L7C18 . Testing it...

antoinetran commented 1 month ago

The test failed. The prefix did nothing.

In fact, the SingularityPrefix is not a prefix added to image. After looking in the code, I can see that:

My proposition is to add a SingularityImagePrefix in the slurm configuration file and use it in the container creation.

dciangot commented 1 month ago

Yeah, I agree. This was overlooked and never used I suspect.

antoinetran commented 3 weeks ago

Workaround: this is already implemented and tested in main branch. Add an annotation:

apiVersion: batch/v1
kind: Job
metadata:
  name: helloworld
spec:
  template:
    metadata:
      labels:
        app: helloworld
      annotations:
        slurm-job.vk.io/image-root: "docker://"

Then InterLink will add "docker://" as expected for singularity. Currently we have to do that for each pod.