Skip to content

Commit

Permalink
Respond to PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
ejweber committed Aug 1, 2022
1 parent 27e95c1 commit b5d29a3
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 37 deletions.
12 changes: 6 additions & 6 deletions api/tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -1036,12 +1036,12 @@ type TaskCSIPluginConfig struct {
// 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
// StagePublishBaseDir 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}
// e.g. staging_target_path = {StagePublishBaseDir}/staging/{volume-id}/{usage-mode}
// e.g. target_path = {StagePublishBaseDir}/per-alloc/{alloc-id}/{volume-id}/{usage-mode}
// Default is /local/csi.
StagePublishDir string `mapstructure:"stage_publish_dir" hcl:"stage_publish_dir,optional"`
StagePublishBaseDir string `mapstructure:"stage_publish_base_dir" hcl:"stage_publish_base_dir,optional"`

// HealthTimeout is the time after which the CSI plugin tasks will be killed
// if the CSI Plugin is not healthy.
Expand All @@ -1053,8 +1053,8 @@ func (t *TaskCSIPluginConfig) Canonicalize() {
t.MountDir = "/csi"
}

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

if t.HealthTimeout == 0 {
Expand Down
15 changes: 7 additions & 8 deletions client/allocrunner/taskrunner/plugin_supervisor_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,13 @@ func newCSIPluginSupervisorHook(config *csiPluginSupervisorHookConfig) *csiPlugi
"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
// StagePublishDir default to StagePublishDir = /csi/local. However,
// within /local/csi. Plugins deployed after the introduction of
// StagePublishBaseDir default to StagePublishBaseDir = /local/csi. However,
// plugins deployed between v1.3.0 and the introduction of
// StagePublishDir have StagePublishDir = "". Default to /csi/local here
// StagePublishBaseDir have StagePublishBaseDir = "". Default to /local/csi here
// to avoid breaking plugins that aren't redeployed.
if task.CSIPluginConfig.StagePublishDir == "" {
task.CSIPluginConfig.StagePublishDir = filepath.Join("/local", "csi")
if task.CSIPluginConfig.StagePublishBaseDir == "" {
task.CSIPluginConfig.StagePublishBaseDir = filepath.Join("/local", "csi")
}

if task.CSIPluginConfig.HealthTimeout == 0 {
Expand Down Expand Up @@ -167,12 +167,11 @@ func (h *csiPluginSupervisorHook) Prestart(ctx context.Context,
}
// where the staging and per-alloc directories will be mounted
volumeStagingMounts := &drivers.MountConfig{
TaskPath: h.task.CSIPluginConfig.StagePublishDir,
TaskPath: h.task.CSIPluginConfig.StagePublishBaseDir,
HostPath: h.mountPoint,
Readonly: false,
PropagationMode: "bidirectional",
}
h.logger.Info("", "volumeStagingMounts", volumeStagingMounts) // TODO: Remove this before merge.
// devices from the host
devMount := &drivers.MountConfig{
TaskPath: "/dev",
Expand Down Expand Up @@ -370,7 +369,7 @@ func (h *csiPluginSupervisorHook) registerPlugin(client csi.CSIPlugin, socketPat
Options: map[string]string{
"Provider": info.Name, // vendor name
"MountPoint": h.mountPoint,
"ContainerMountPoint": h.task.CSIPluginConfig.StagePublishDir,
"ContainerMountPoint": h.task.CSIPluginConfig.StagePublishBaseDir,
},
}
}
Expand Down
2 changes: 1 addition & 1 deletion command/agent/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -1263,7 +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
sc.StagePublishBaseDir = apiConfig.StagePublishBaseDir
sc.HealthTimeout = apiConfig.HealthTimeout
return sc
}
Expand Down
8 changes: 4 additions & 4 deletions nomad/structs/csi.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,12 @@ type TaskCSIPluginConfig struct {
// socket (called CSISocketName) for communication with Nomad. Default is /csi.
MountDir string

// StagePublishDir is the base directory (within its container) in which the plugin
// StagePublishBaseDir 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}
// e.g. staging_target_path = {StagePublishBaseDir}/staging/{volume-id}/{usage-mode}
// e.g. target_path = {StagePublishBaseDir}/per-alloc/{alloc-id}/{volume-id}/{usage-mode}
// Default is /local/csi.
StagePublishDir string
StagePublishBaseDir string

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

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

Expand Down Expand Up @@ -74,11 +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. 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.
the container as the `CSI_ENDPOINT` environment variable.

Some plugins also require the `stage_publish_base_dir` field, which
tells Nomad where to instruct the plugin to mount volumes for staging
and/or publishing.

### Plugin Lifecycle and State

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

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

Expand All @@ -41,13 +41,15 @@ csi_plugin {
`node` at the same time, and these are called `monolith`
plugins. Refer to your CSI plugin's documentation.

- `mount_dir` `(string: <required>)` - The directory path inside the
- `mount_dir` `(string: <optional>)` - The directory path inside the
container where the plugin will expect a Unix domain socket for
bidirectional communication with Nomad.
bidirectional communication with Nomad. This field is typically not
required. Refer to your CSI plugin's documentation for details.

- `stage_publish_dir` `(string: <optional>)` - The base directory
- `stage_publish_base_dir` `(string: <optional>)` - The base directory
path inside the container where the plugin will be instructed to
stage and publish volumes.
stage and publish volumes. This field is typically not required.
Refer to your CSI plugin's documentation for details.

- `health_timeout` `(duration: <optional>)` - The duration that
the plugin supervisor will wait before restarting an unhealthy
Expand Down

0 comments on commit b5d29a3

Please sign in to comment.