CSI: allow for concurrent plugin allocations

[tgross]

Fixes #8628 #11784

The dynamic plugin registry assumes that plugins are singletons, which matches the behavior of other Nomad plugins. But because dynamic plugins like CSI are implemented by allocations, we need to handle the possibility of multiple allocations for a given plugin type + ID, as well as behaviors around interleaved allocation starts and stops.

This is a fairly huge PR but none of it ships safely without the rest, so I've cut it up into hopefully-reviewable chunks.

TODO:

• Failing tests for concurrency bugs in plugin registration. Done in 0a46a31
• Update the plugin registry to support a linked list of plugins such that the most recent live alloc ID is what gets dispensed. Done in 1b8eb1e
  • On registry updates, swap out instance managers so that we're always running a single instance manager for the "active" plugin.
  • On plugin restore, do not reorder the linked list.
  • Update the plugin registry to sync to a new client state table with the linked list data structure.
• Update the plugin supervisor to change the location of the csi.sock to somewhere owned by the allocation. Done in 1f095de
• Update the client state store to safely handle updating from the previous data structure to the new one. Done in 0cfe94e
• Changelog entry and upgrade warning docs. Done in 58cccb0

[tgross]

@lgfa29 I think I'm getting a false positive on the semgrep rule here: https://github.com/hashicorp/nomad/runs/5222371451?check_suite_focus=true Arguably these assertions aren't all that useful, so I wouldn't be broken-hearted if I just had to remove them. But any suggestions for a fix?

[lgfa29]

Ahh interesting, I haven't thought of this pattern before. Feel free to leave the code as-is and ignore the check for now. I will try to adjust the rules to match this.

[tgross]

Note to reviewers: this PR is already huge and adding to this field to the jobspec is both a minor change and a lot of code to review. I'm going to do it in a follow-on PR.

[lgfa29]

Probably a silly question, but is there a use case where this would even need to be configurable?

[tgross]

Not a very good use case. 😁

Suppose you had a plugin that required a configuration file that you created via template block, we could conceivably collide with the destination for that template. Ideally the user would just use a different destination of course. But obviously this is low-priority enough that it's not going to be a blocker at all for CSI GA.

[tgross]

Note to reviewers: this is a "blocking GA" issue so I'll have a follow-up PR on that as well.

[tgross]

Note to reviewers: this file got created from the following test code on a 1.2.6 branch:

TestBoltStateDB_WriteTestData

func TestBoltStateDB_WriteTestData(t *testing.T) {
	os.Mkdir("newdata", 0775)
	dbI, err := NewBoltStateDB(testlog.HCLogger(t), "newdata")
	require.NoError(t, err)

	db := dbI.(*BoltStateDB)
	defer db.Close()

	ps := &dynamicplugins.RegistryState{
		Plugins: map[string]map[string]*dynamicplugins.PluginInfo{
			"csi-controller": {
				"plugin-1": {
					Name:    "plugin-1",
					Type:    string(structs.CSIPluginTypeMonolith),
					Version: "v0.1",
					ConnectionInfo: &dynamicplugins.PluginConnectionInfo{
						SocketPath: "/var/nomad/client/csi/plugin-1/csi.sock",
					},
					AllocID: "489cd5cf-3cfc-6792-3465-822371765b32",
					Options: map[string]string{
						"Provider":            "demo",
						"MountPoint":          "/var/nomad/client/csi/plugin-1",
						"ContainerMountPoint": "/csi",
					},
				},
			},
			"csi-node": {
				"plugin-1": {
					Name:    "plugin-1",
					Type:    string(structs.CSIPluginTypeMonolith),
					Version: "v0.1",
					ConnectionInfo: &dynamicplugins.PluginConnectionInfo{
						SocketPath: "/var/nomad/client/csi/plugin-1/csi.sock",
					},
					AllocID: "489cd5cf-3cfc-6792-3465-822371765b32",
					Options: map[string]string{
						"Provider":            "demo",
						"mountpoint":          "/var/nomad/client/csi/plugin-1",
						"ContainerMountPoint": "/csi",
					},
				},
				"plugin-2": {
					Name:    "plugin-2",
					Type:    string(structs.CSIPluginTypeNode),
					Version: "v0.2",
					ConnectionInfo: &dynamicplugins.PluginConnectionInfo{
						SocketPath: "/var/nomad/client/csi/plugin-2/csi.sock",
					},
					AllocID: "68221094-25a6-eeb6-8490-c71a03d46b0f",
					Options: map[string]string{
						"Provider":            "other-demo",
						"MountPoint":          "/var/nomad/client/csi/plugin-2",
						"ContainerMountPoint": "/csi",
					},
				},
			},
		},
	}
	db.PutDynamicPluginRegistryState(ps)

}

[lgfa29]

Minor comments (mostly questions), but LGTM.

Thanks for breaking down the work into commits 🙂

[lgfa29]

Probably a silly question, but is there a use case where this would even need to be configurable?

[lgfa29]

Does this value change depending on the file system isolation capability of the task driver?

[tgross]

A task driver without filesystem isolation never creates the mount, so it's a no-op. (Note this is an exotic case because the spec requires that plugins are shipped as OCI containers)

[github-actions]

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.