-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Changes from 5 commits
ca3c585
c8010c4
1eab7f5
3448143
27e95c1
b5d29a3
64d48fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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/ | ||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
// 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 | ||
} | ||
|
@@ -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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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", | ||
|
@@ -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, | ||
}, | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
} | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
andstage_publish_base_dir
?