From 3d86d31b9f1b1e1632f66cc6cd2c3a94051a1d0b Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 30 Nov 2021 12:15:08 -0800 Subject: [PATCH 1/2] 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. Signed-off-by: Kir Kolyshkin --- exec.go | 5 ++++- libcontainer/utils/utils.go | 19 ++++++++----------- libcontainer/utils/utils_test.go | 28 +++++++++++++++++----------- 3 files changed, 29 insertions(+), 23 deletions(-) diff --git a/exec.go b/exec.go index 18c6bffde..cdc9859ca 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/utils/utils.go b/libcontainer/utils/utils.go index 6b9fc3435..dbd435341 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 diff --git a/libcontainer/utils/utils_test.go b/libcontainer/utils/utils_test.go index 52fc93639..06c042f5f 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) } } } From dffb8db7e1bba94cf48cc59af47014501393d35e Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 30 Nov 2021 12:21:52 -0800 Subject: [PATCH 2/2] 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. Signed-off-by: Kir Kolyshkin --- libcontainer/container_linux.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index f6877b742..9f3c7eb2c 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