Repository: opencontainers/runc
Pull request #3303 (2021-11-30 20:25:37)

libcontainer: optimize utils.SearchLabels
* libct/utils: SearchLabels: optimize
    
    Using strings.Split generates temporary strings for GC to collect.
    Rewrite the function to not do that.
    
    Also, add a second return value, so that the caller can distinguish
    between an empty value found and no key found cases.
    
    Fix the test accordingly.

* libct: handleCriuConfigurationFile: use utils.SearchLabels
    
    The utils.Annotations was used here before only because it made it
    possible to distinguish between "key not found" and "empty value" cases.
    
    With the previous commit, utils.SearchLabels can do that, and so it
    makes sense to use it.


Patch:
diff --git a/exec.go b/exec.go
index 18c6bffd..cdc9859c 100644
--- a/exec.go
+++ b/exec.go
@@ -162,7 +162,10 @@ func execProcess(context *cli.Context) (int, error) {
 	if err != nil {
 		return -1, err
 	}
-	bundle := utils.SearchLabels(state.Config.Labels, "bundle")
+	bundle, ok := utils.SearchLabels(state.Config.Labels, "bundle")
+	if !ok {
+		return -1, errors.New("bundle not found in labels")
+	}
 	p, err := getProcess(context, bundle)
 	if err != nil {
 		return -1, err
diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go
index f6877b74..9f3c7eb2 100644
--- a/libcontainer/container_linux.go
+++ b/libcontainer/container_linux.go
@@ -881,8 +881,7 @@ func (c *linuxContainer) handleCriuConfigurationFile(rpcOpts *criurpc.CriuOpts)
 	// Look for annotations. The annotation 'org.criu.config'
 	// specifies if CRIU should use a different, container specific
 	// configuration file.
-	_, annotations := utils.Annotations(c.config.Labels)
-	configFile, exists := annotations["org.criu.config"]
+	configFile, exists := utils.SearchLabels(c.config.Labels, "org.criu.config")
 	if exists {
 		// If the annotation 'org.criu.config' exists and is set
 		// to a non-empty string, tell CRIU to use that as a
diff --git a/libcontainer/utils/utils.go b/libcontainer/utils/utils.go
index 6b9fc343..dbd43534 100644
--- a/libcontainer/utils/utils.go
+++ b/libcontainer/utils/utils.go
@@ -132,19 +132,16 @@ func WithProcfd(root, unsafePath string, fn func(procfd string) error) error {
 	return fn(procfd)
 }
 
-// SearchLabels searches a list of key-value pairs for the provided key and
-// returns the corresponding value. The pairs must be separated with '='.
-func SearchLabels(labels []string, query string) string {
-	for _, l := range labels {
-		parts := strings.SplitN(l, "=", 2)
-		if len(parts) < 2 {
-			continue
-		}
-		if parts[0] == query {
-			return parts[1]
+// SearchLabels searches through a list of key=value pairs for a given key,
+// returning its value, and the binary flag telling whether the key exist.
+func SearchLabels(labels []string, key string) (string, bool) {
+	key += "="
+	for _, s := range labels {
+		if strings.HasPrefix(s, key) {
+			return s[len(key):], true
 		}
 	}
-	return ""
+	return "", false
 }
 
 // Annotations returns the bundle path and user defined annotations from the


Test patch:
diff --git a/libcontainer/utils/utils_test.go b/libcontainer/utils/utils_test.go
index 52fc9363..06c042f5 100644
--- a/libcontainer/utils/utils_test.go
+++ b/libcontainer/utils/utils_test.go
@@ -8,22 +8,28 @@ import (
 )
 
 var labelTest = []struct {
-	labels        []string
-	query         string
-	expectedValue string
+	labels []string
+	query  string
+	expVal string
+	expOk  bool
 }{
-	{[]string{"bundle=/path/to/bundle"}, "bundle", "/path/to/bundle"},
-	{[]string{"test=a", "test=b"}, "bundle", ""},
-	{[]string{"bundle=a", "test=b", "bundle=c"}, "bundle", "a"},
-	{[]string{"", "test=a", "bundle=b"}, "bundle", "b"},
-	{[]string{"test", "bundle=a"}, "bundle", "a"},
-	{[]string{"test=a", "bundle="}, "bundle", ""},
+	{[]string{"bundle=/path/to/bundle"}, "bundle", "/path/to/bundle", true},
+	{[]string{"test=a", "test=b"}, "bundle", "", false},
+	{[]string{"bundle=a", "test=b", "bundle=c"}, "bundle", "a", true},
+	{[]string{"", "test=a", "bundle=b"}, "bundle", "b", true},
+	{[]string{"test", "bundle=a"}, "bundle", "a", true},
+	{[]string{"test=a", "bundle="}, "bundle", "", true},
 }
 
 func TestSearchLabels(t *testing.T) {
 	for _, tt := range labelTest {
-		if v := SearchLabels(tt.labels, tt.query); v != tt.expectedValue {
-			t.Errorf("expected value '%s' for query '%s'; got '%s'", tt.expectedValue, tt.query, v)
+		v, ok := SearchLabels(tt.labels, tt.query)
+		if ok != tt.expOk {
+			t.Errorf("expected ok: %v, got %v", tt.expOk, ok)
+			continue
+		}
+		if v != tt.expVal {
+			t.Errorf("expected value '%s' for query '%s'; got '%s'", tt.expVal, tt.query, v)
 		}
 	}
 }