From 3da289f4833ae0c052ec9294b6073b1ae550e09a Mon Sep 17 00:00:00 2001 From: Adriaan Dens Date: Tue, 15 Oct 2024 20:59:02 +0200 Subject: [PATCH] (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 | 10 ++--- driver_test.go | 101 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 5 deletions(-) diff --git a/driver.go b/driver.go index a6598a9..b698d3e 100644 --- a/driver.go +++ b/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() diff --git a/driver_test.go b/driver_test.go index 5f41673..f33a747 100644 --- a/driver_test.go +++ b/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))