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 stage_publish_dir field to csi_plugin stanza of a job #13919

Merged
merged 7 commits into from
Aug 2, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions api/tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -1032,14 +1032,17 @@ type TaskCSIPluginConfig struct {
// CSIPluginType instructs Nomad on how to handle processing a plugin
Type CSIPluginType `mapstructure:"type" hcl:"type,optional"`

// MountDir is the destination that nomad should mount in its CSI
// directory for the plugin. It will then expect a file called CSISocketName
// to be created by the plugin, and will provide references into
// "MountDir/CSIIntermediaryDirname/VolumeName/AllocID for mounts.
//
// Default is /csi.
// MountDir is the directory (within its container) in which the plugin creates a
// socket (called CSISocketName) for communication with Nomad. Default is /csi.
MountDir string `mapstructure:"mount_dir" hcl:"mount_dir,optional"`

// StagePublishDir is the base directory (within its container) in which the plugin
// mounts volumes being staged and bind mounts volumes being published.
// e.g. staging_target_path = {StagePublishDir}/staging/{volume-id}/{usage-mode}
// e.g. target_path = {StagePublishDir}/per-alloc/{alloc-id}/{volume-id}/{usage-mode}
// Default is /local/csi.
StagePublishDir string `mapstructure:"stage_publish_dir" hcl:"stage_publish_dir,optional"`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This naming may or may not be controversial (as a new part of Nomad's public API). I tried to make it as descriptive as possible, but I'm definitely open to other ideas.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah seems like a tricky name to get right. Maybe StagePublishBaseDir and stage_publish_base_dir?


// HealthTimeout is the time after which the CSI plugin tasks will be killed
// if the CSI Plugin is not healthy.
HealthTimeout time.Duration `mapstructure:"health_timeout" hcl:"health_timeout,optional"`
Expand All @@ -1050,6 +1053,10 @@ func (t *TaskCSIPluginConfig) Canonicalize() {
t.MountDir = "/csi"
}

if t.StagePublishDir == "" {
t.StagePublishDir = filepath.Join("/local", "csi")
}

if t.HealthTimeout == 0 {
t.HealthTimeout = 30 * time.Second
}
Expand Down
18 changes: 14 additions & 4 deletions client/allocrunner/taskrunner/plugin_supervisor_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ var _ interfaces.TaskStopHook = &csiPluginSupervisorHook{}
// Per-allocation directories of unix domain sockets used to communicate
// with the CSI plugin. Nomad creates the directory and the plugin creates
// the socket file. This directory is bind-mounted to the
// csi_plugin.mount_config dir in the plugin task.
// csi_plugin.mount_dir in the plugin task.
//
// {plugin-type}/{plugin-id}/
// staging/
Expand All @@ -103,6 +103,16 @@ func newCSIPluginSupervisorHook(config *csiPluginSupervisorHookConfig) *csiPlugi
socketMountPoint := filepath.Join(config.clientStateDirPath, "csi",
"plugins", config.runner.Alloc().ID)

// In v1.3.0, Nomad started instructing CSI plugins to stage and publish
// within /csi/local. Plugins deployed after the introduction of
Copy link
Member

Choose a reason for hiding this comment

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

The code is right but this comment has swapped the path ordering. It's /local/csi.

// StagePublishDir default to StagePublishDir = /csi/local. However,
// plugins deployed between v1.3.0 and the introduction of
// StagePublishDir have StagePublishDir = "". Default to /csi/local here
// to avoid breaking plugins that aren't redeployed.
if task.CSIPluginConfig.StagePublishDir == "" {
task.CSIPluginConfig.StagePublishDir = filepath.Join("/local", "csi")
}
tgross marked this conversation as resolved.
Show resolved Hide resolved

if task.CSIPluginConfig.HealthTimeout == 0 {
task.CSIPluginConfig.HealthTimeout = 30 * time.Second
}
Expand Down Expand Up @@ -157,12 +167,12 @@ func (h *csiPluginSupervisorHook) Prestart(ctx context.Context,
}
// where the staging and per-alloc directories will be mounted
volumeStagingMounts := &drivers.MountConfig{
// TODO(tgross): add this TaskPath to the CSIPluginConfig as well
TaskPath: "/local/csi",
TaskPath: h.task.CSIPluginConfig.StagePublishDir,
HostPath: h.mountPoint,
Readonly: false,
PropagationMode: "bidirectional",
}
h.logger.Info("", "volumeStagingMounts", volumeStagingMounts) // TODO: Remove this before merge.
Copy link
Member

Choose a reason for hiding this comment

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

Don't forget to remove this! 😀

// devices from the host
devMount := &drivers.MountConfig{
TaskPath: "/dev",
Expand Down Expand Up @@ -360,7 +370,7 @@ func (h *csiPluginSupervisorHook) registerPlugin(client csi.CSIPlugin, socketPat
Options: map[string]string{
"Provider": info.Name, // vendor name
"MountPoint": h.mountPoint,
"ContainerMountPoint": "/local/csi",
"ContainerMountPoint": h.task.CSIPluginConfig.StagePublishDir,
},
}
}
Expand Down
1 change: 1 addition & 0 deletions command/agent/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -1263,6 +1263,7 @@ func ApiCSIPluginConfigToStructsCSIPluginConfig(apiConfig *api.TaskCSIPluginConf
sc.ID = apiConfig.ID
sc.Type = structs.CSIPluginType(apiConfig.Type)
sc.MountDir = apiConfig.MountDir
sc.StagePublishDir = apiConfig.StagePublishDir
Copy link
Member

Choose a reason for hiding this comment

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

Good catch 👍 I've still got #10471 open to remove this whole gross blob of code but we'll need it for now.

sc.HealthTimeout = apiConfig.HealthTimeout
return sc
}
Expand Down
13 changes: 9 additions & 4 deletions nomad/structs/csi.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,17 @@ type TaskCSIPluginConfig struct {
// Type instructs Nomad on how to handle processing a plugin
Type CSIPluginType

// MountDir is the destination that nomad should mount in its CSI
// directory for the plugin. It will then expect a file called CSISocketName
// to be created by the plugin, and will provide references into
// "MountDir/CSIIntermediaryDirname/{VolumeName}/{AllocID} for mounts.
// MountDir is the directory (within its container) in which the plugin creates a
// socket (called CSISocketName) for communication with Nomad. Default is /csi.
MountDir string

// StagePublishDir is the base directory (within its container) in which the plugin
// mounts volumes being staged and bind mount volumes being published.
// e.g. staging_target_path = {StagePublishDir}/staging/{volume-id}/{usage-mode}
// e.g. target_path = {StagePublishDir}/per-alloc/{alloc-id}/{volume-id}/{usage-mode}
// Default is /local/csi.
StagePublishDir string

// HealthTimeout is the time after which the CSI plugin tasks will be killed
// if the CSI Plugin is not healthy.
HealthTimeout time.Duration `mapstructure:"health_timeout" hcl:"health_timeout,optional"`
Expand Down
13 changes: 9 additions & 4 deletions website/content/docs/concepts/plugins/csi.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,10 @@ A CSI plugin task requires the [`csi_plugin`][csi_plugin] block:

```hcl
csi_plugin {
id = "csi-hostpath"
type = "monolith"
mount_dir = "/csi"
id = "csi-hostpath"
type = "monolith"
mount_dir = "/csi"
stage_publish_dir = "/local/csi"
}
```

Expand Down Expand Up @@ -73,7 +74,11 @@ Nomad exposes a Unix domain socket named `csi.sock` inside each CSI
plugin task, and communicates over the gRPC protocol expected by the
CSI specification. The `mount_dir` field tells Nomad where the plugin
expects to find the socket file. The path to this socket is exposed in
the container as the `CSI_ENDPOINT` environment variable.
the container as the `CSI_ENDPOINT` environment variable. In
addition, the `stage_publish_dir` field tells Nomad where the plugin
wants to be instructed to mount volumes for staging and/or publishing.
This field is generally not required and, like `mount_dir`, only
affects the plugin container's internal view of the file system.
tgross marked this conversation as resolved.
Show resolved Hide resolved

### Plugin Lifecycle and State

Expand Down
13 changes: 9 additions & 4 deletions website/content/docs/job-specification/csi_plugin.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@ to claim [volumes][csi_volumes].

```hcl
csi_plugin {
id = "csi-hostpath"
type = "monolith"
mount_dir = "/csi"
health_timeout = "30s"
id = "csi-hostpath"
type = "monolith"
mount_dir = "/csi"
stage_publish_dir = "/local/csi"
health_timeout = "30s"
}
```

Expand All @@ -44,6 +45,10 @@ csi_plugin {
container where the plugin will expect a Unix domain socket for
bidirectional communication with Nomad.

- `stage_publish_dir` `(string: <optional>)` - The base directory
path inside the container where the plugin will be instructed to
stage and publish volumes.
tgross marked this conversation as resolved.
Show resolved Hide resolved

- `health_timeout` `(duration: <optional>)` - The duration that
the plugin supervisor will wait before restarting an unhealthy
CSI plugin. Must be a duration value such as `30s` or `2m`.
Expand Down