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.

Sign up for GitHub

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

Jump to bottom

Add stage_publish_dir field to csi_plugin stanza of a job #13919

Merged
merged 7 commits into from
Aug 2, 2022
Merged

Add stage_publish_dir field to csi_plugin stanza of a job #13919

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ca3c585
Allow specification of CSI staging and publishing directory path
ejweber Jun 16, 2022
c8010c4
Add website documentation for stage_publish_dir
ejweber Jun 16, 2022
1eab7f5
Replace erroneous reference to csi_plugin.mount_config with csi_plugi…
ejweber Jun 16, 2022
3448143
Avoid requiring CSI plugins to be redeployed after introducing StageP…
ejweber Jun 16, 2022
27e95c1
Fix gofmt -s failure
ejweber Jul 25, 2022
b5d29a3
Respond to PR feedback
ejweber Aug 1, 2022
64d48fc
changelog entry
tgross Aug 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 13 additions & 6 deletions api/tasks.go
View file
Open in desktop
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
6 changes: 3 additions & 3 deletions client/allocrunner/taskrunner/plugin_supervisor_hook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,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 +360,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
View file
Open in desktop
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
View file
Open in desktop
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