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

Mount volume spec #22

Merged
merged 3 commits into from
Mar 3, 2024
Merged

Mount volume spec #22

merged 3 commits into from
Mar 3, 2024

Conversation

jagedn
Copy link
Collaborator

@jagedn jagedn commented Mar 1, 2024

close #13

This PR implements a new volume config where the user can specify the type and the nameof the volume

Valid types are host, csi and docker and the service create a JobSpec mounting it

dockerVolume is deprecated so the user must to use new configuration if want to mount a docker volume

We've agreed only a volume is required, at least by the moment

@jagedn jagedn requested review from abhi18av and matthdsm and removed request for abhi18av March 1, 2024 15:59
@jagedn jagedn self-assigned this Mar 3, 2024
@jagedn jagedn requested a review from abhi18av March 3, 2024 13:14
Copy link
Member

@abhi18av abhi18av left a comment

Choose a reason for hiding this comment

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

Thanks dear Jorge for finalizing this.

As discussed in our dev sync, I think we can now start the next phase of implementing other Nomad Job specs and map the Nextflow directives over them.

Excited to test a couple more pipelines with these changes! 🤩

@abhi18av abhi18av merged commit 6376aee into master Mar 3, 2024
2 checks passed
@abhi18av abhi18av deleted the mount-volume branch March 3, 2024 21:16
@abhi18av
Copy link
Member

abhi18av commented Mar 3, 2024

Adding some context here regarding the current syntax for specifying the volume

nomad {
    client {
        address = "http://NOMAD_IP:4646"
        token = ""
    }

    jobs {
       datacenters = "sun-nomadlab"
       volume = { //final version is a closure, not a map
           type "csi" 
           name "csi-volume-name" 
       }
    }
}

@jagedn
Copy link
Collaborator Author

jagedn commented Mar 3, 2024

Once we have the "asciidoctor/gh-pages/user guide" ready it will be a "requirement" to document this kind of stuff on it

@abhi18av
Copy link
Member

abhi18av commented Mar 3, 2024

@jagedn , that makes sense.

Just adding a thought for future reference, what if we want to allow the mounting multiple volumes within the same job?

In that case, wouldn't it be easier to have Map oriented syntax, which can compose multiple volume-maps into an array specification. As shown below

    jobs {
       datacenters = "sun-nomadlab"
       volume = [
                  [
                     type: "csi"  // CSI volume type - a volume shared across multiple client nodes
                     name: "csi-volume-name" 
                  ],
                  [
                     type: "host"  // HOST volume type - maybe nomad client node specific block storage
                     name: "host-volume-name" 
                  ],
                ]
    }

This way we would keep the possibility of (multiple) HOST + CSI volumes into the same job definition and therefore allow a mix-n-match of node-specific and shared file systems for each task.

What do you think? 🤔

@abhi18av abhi18av linked an issue Mar 9, 2024 that may be closed by this pull request
16 tasks
@abhi18av abhi18av mentioned this pull request Mar 9, 2024
16 tasks
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.

Plugin design - dev meetings. Allow storage config using nomad volume blocks
2 participants