(Regression) Tests and bugfixes for multiple socket implementation (#386

)

Fix 2 bugs:
* Cleaning of the socket name before storing in podmanClients, made it unfindable in the Job task unless you know how the cleaning of the name is done. The cleaning of the socket name was reverted for podmanClients map and only applied when using it as an attribute name (which was the goal anyway)
* When all sockets are named and none of them is named default, attributes would not have a driver.podman.* prefix. To fix this, the default podman socket (first in the list) will get mapped to the driver.podman and an extra attribute is exposed to find the original name of the socket given in the driver config.

driver.go

@@ -222,7 +222,6 @@ func (d *Driver) makePodmanClients(sockets []PluginSocketConfig, timeout time.Du
 		if sock.Name == "" {
 			sock.Name = "default"
 		}
-		sock.Name = cleanUpSocketName(sock.Name)
 		if sock.Name == "default" && !foundDefaultPodman {
 			foundDefaultPodman = true
 			podmanClient = d.newPodmanClient(timeout, sock.SocketPath, true)
@@ -270,7 +269,7 @@ func (d *Driver) getPodmanClient(clientName string) (*api.API, error) {
 	if ok {
 		return p, nil
 	}
-	return nil, fmt.Errorf("podman client with name %s was not found, check your podman driver config", clientName)
+	return nil, fmt.Errorf("podman client with name %q was not found, check your podman driver config", clientName)
 }
 
 // newPodmanClient returns Podman client configured with the provided timeout.
@@ -332,8 +331,8 @@ func (d *Driver) buildFingerprint() *drivers.Fingerprint {
 	for name, podmanClient := range d.podmanClients {
 		// Ping podman api
 		apiVersion, err := podmanClient.Ping(d.ctx)
-		attrPrefix := fmt.Sprintf("driver.podman.%s", name)
-		if name == "default" {
+		attrPrefix := fmt.Sprintf("driver.podman.%s", cleanUpSocketName(name))
+		if podmanClient.IsDefaultClient() {
 			attrPrefix = "driver.podman"
 		}
 
@@ -366,6 +365,7 @@ func (d *Driver) buildFingerprint() *drivers.Fingerprint {
 		attrs[fmt.Sprintf("%s.cgroupVersion", attrPrefix)] = pstructs.NewStringAttribute(info.Host.CGroupsVersion)
 		attrs[fmt.Sprintf("%s.defaultPodman", attrPrefix)] = pstructs.NewBoolAttribute(podmanClient.IsDefaultClient())
 		attrs[fmt.Sprintf("%s.health", attrPrefix)] = pstructs.NewStringAttribute("healthy")
+		attrs[fmt.Sprintf("%s.socketName", attrPrefix)] = pstructs.NewStringAttribute(name)
 		attrs[fmt.Sprintf("%s.ociRuntime", attrPrefix)] = pstructs.NewStringAttribute(info.Host.OCIRuntime.Path)
 		attrs[fmt.Sprintf("%s.rootless", attrPrefix)] = pstructs.NewBoolAttribute(info.Host.Security.Rootless)
 		attrs[fmt.Sprintf("%s.seccompEnabled", attrPrefix)] = pstructs.NewBoolAttribute(info.Host.Security.SECCOMPEnabled)
@@ -535,7 +535,7 @@ func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drive
 		var err error
 		podmanClient, err = d.getPodmanClient(podmanTaskSocketName)
 		if err != nil {
-			return nil, nil, fmt.Errorf("podman client with name %s not found, check your podman driver config", podmanTaskSocketName)
+			return nil, nil, fmt.Errorf("podman client with name %q not found, check your podman driver config", podmanTaskSocketName)
 		}
 	}
 	rootless := podmanClient.IsRootless()

driver_test.go

@@ -86,6 +86,13 @@ func podmanDriverHarness(t *testing.T, cfg map[string]interface{}) *dtestutil.Dr
 		}
 	}
 
+	// Configure the Plugin with named socket
+	if v, ok := cfg["Socket"]; ok {
+		if sv, ok := v.([]PluginSocketConfig); ok {
+			pluginConfig.Socket = sv
+		}
+	}
+
 	if err := base.MsgPackEncode(&baseConfig.PluginConfig, &pluginConfig); err != nil {
 		t.Error("Unable to encode plugin config", err)
 	}
@@ -2451,6 +2458,100 @@ func Test_parseImage(t *testing.T) {
 	}
 }
 
+func Test_namedSocketIsDefault(t *testing.T) {
+	ci.Parallel(t)
+
+	defaultSocket := PluginSocketConfig{Name: "default", SocketPath: "unix://run/podman/podman.sock"}
+	sockets := make([]PluginSocketConfig, 1)
+	sockets[0] = defaultSocket
+
+	d := podmanDriverHarness(t, map[string]interface{}{
+		"Socket": sockets,
+	})
+
+	// Get back our podman socket via its name
+	api, err := getPodmanDriver(t, d).getPodmanClient("default")
+	must.NoError(t, err)
+	must.True(t, api.IsDefaultClient())
+}
+
+// Test that omitting a name in the Socket definition gives it the name "default"
+func Test_unnamedSocketIsRenamedToDefault(t *testing.T) {
+	ci.Parallel(t)
+
+	defaultSocket := PluginSocketConfig{Name: "", SocketPath: "unix://run/podman/podman.sock"}
+	sockets := make([]PluginSocketConfig, 1)
+	sockets[0] = defaultSocket
+
+	d := podmanDriverHarness(t, map[string]interface{}{
+		"Socket": sockets,
+	})
+
+	_, err := getPodmanDriver(t, d).getPodmanClient("default")
+	must.NoError(t, err)
+}
+
+// Test that if there's one socket, it becomes the default socket (regardless of name)
+func Test_namedSocketBecomesDefaultSocket(t *testing.T) {
+	ci.Parallel(t)
+
+	socketPath := "unix://run/podman/podman.sock"
+	if os.Geteuid() != 0 {
+		socketPath = fmt.Sprintf("unix://run/user/%d/podman/podman.sock", os.Geteuid())
+	}
+
+	defaultSocket := PluginSocketConfig{Name: "podmanSock", SocketPath: socketPath}
+	sockets := make([]PluginSocketConfig, 1)
+	sockets[0] = defaultSocket
+
+	d := podmanDriverHarness(t, map[string]interface{}{
+		"Socket": sockets,
+	})
+
+	api, err := getPodmanDriver(t, d).getPodmanClient("podmanSock")
+	must.NoError(t, err)
+	must.True(t, api.IsDefaultClient())
+
+	fingerprint := getPodmanDriver(t, d).buildFingerprint()
+	must.MapContainsKey(t, fingerprint.Attributes, "driver.podman.socketName")
+
+	originalName, ok := fingerprint.Attributes["driver.podman.socketName"].GetString()
+	must.True(t, ok)
+	must.Eq(t, "podmanSock", originalName)
+	isDefaultAttr, ok := fingerprint.Attributes["driver.podman.defaultPodman"].GetBool()
+	must.True(t, ok)
+	must.True(t, isDefaultAttr)
+}
+
+// A socket with non alpha numeric ([a-zA-Z0-9_]) chars gets cleaned before being used
+// in attributes.
+func Test_socketNameIsCleanedInAttributes(t *testing.T) {
+	ci.Parallel(t)
+
+	socketPath := "unix://run/podman/podman.sock"
+	if os.Geteuid() != 0 {
+		socketPath = fmt.Sprintf("unix://run/user/%d/podman/podman.sock", os.Geteuid())
+	}
+
+	// We need two sockets because the default socket will get prefix driver.podman. without the socket name.
+	defaultSocket := PluginSocketConfig{Name: "default", SocketPath: socketPath}
+	socketWithSpaces := PluginSocketConfig{Name: "a socket with spaces", SocketPath: socketPath}
+	sockets := make([]PluginSocketConfig, 2)
+	sockets[0] = defaultSocket
+	sockets[1] = socketWithSpaces
+
+	d := podmanDriverHarness(t, map[string]interface{}{
+		"Socket": sockets,
+	})
+
+	fingerprint := getPodmanDriver(t, d).buildFingerprint()
+	must.MapContainsKey(t, fingerprint.Attributes, "driver.podman.a_socket_with_spaces.socketName")
+
+	originalName, ok := fingerprint.Attributes["driver.podman.a_socket_with_spaces.socketName"].GetString()
+	must.True(t, ok)
+	must.Eq(t, "a socket with spaces", originalName)
+}
+
 // read a tasks stdout logfile into a string, fail on error
 func readStdoutLog(t *testing.T, task *drivers.TaskConfig) string {
 	logfile := filepath.Join(filepath.Dir(task.StdoutPath), fmt.Sprintf("%s.stdout.0", task.Name))