Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions cmd/nerdctl/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (

"github.com/containerd/console"
"github.com/containerd/containerd"
"github.com/containerd/containerd/cio"
"github.com/containerd/containerd/cmd/ctr/commands"
"github.com/containerd/containerd/cmd/ctr/commands/tasks"
"github.com/containerd/containerd/containers"
Expand Down Expand Up @@ -769,17 +768,25 @@ func generateLogURI(dataStore, logDriver string, logOptMap map[string]string) (*
} else {
return nil, fmt.Errorf("%s is not yet supported", logDriver)
}
args := map[string]string{
logging.MagicArgv1: dataStore,
}
for k, v := range logOptMap {
args[k] = v
filePath := filepath.Clean(selfExe)
if !strings.HasPrefix(filePath, "/") {
return nil, errors.New("absolute filePath needed")
}
if runtime.GOOS == "windows" {
return nil, nil
}
uri := &url.URL{
Scheme: "binary",
Path: filePath,
}
query := strutil.NewOrderedQuery()
query.Set(logging.MagicArgv1, dataStore)

return cio.LogURIGenerator("binary", selfExe, args)
for k, v := range logOptMap {
query.Set(k, v)
}
uri.RawQuery = query.Encode()
return uri, nil
}

func withNerdctlOCIHook(cmd *cobra.Command, id, stateDir string) (oci.SpecOpts, error) {
Expand Down
56 changes: 56 additions & 0 deletions pkg/strutil/strutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ package strutil
import (
"encoding/csv"
"fmt"
"net/url"
"reflect"
"strconv"
"strings"
Expand Down Expand Up @@ -130,3 +131,58 @@ func ParseBoolOrAuto(s string) (*bool, error) {
b, err := strconv.ParseBool(s)
return &b, err
}

type OrderedQuery struct {
index []string
data map[string][]string
}

func NewOrderedQuery() *OrderedQuery {
return &OrderedQuery{[]string{}, map[string][]string{}}
}

func (orderedQuery *OrderedQuery) Get(key string) string {
if orderedQuery == nil {
return ""
}
vs := orderedQuery.data[key]
if len(vs) == 0 {
return ""
}
return vs[0]
}

// Set sets the key to value. It replaces any existing
// values.
func (orderedQuery *OrderedQuery) Set(key, value string) {
orderedQuery.data[key] = []string{value}
orderedQuery.index = append(orderedQuery.index, key)
}

// Add adds the value to key. It appends to any existing
// values associated with key.
func (orderedQuery *OrderedQuery) Add(key, value string) {
orderedQuery.data[key] = append(orderedQuery.data[key], value)
}

// Encode encodes the values into ``URL encoded'' form
// ("bar=baz&foo=quux") sorted by key.
func (orderedQuery *OrderedQuery) Encode() string {
if orderedQuery == nil {
return ""
}
var buf strings.Builder
for _, k := range orderedQuery.index {
vs := orderedQuery.data[k]
keyEscaped := url.QueryEscape(k)
for _, v := range vs {
if buf.Len() > 0 {
buf.WriteByte('&')
}
buf.WriteString(keyEscaped)
buf.WriteByte('=')
buf.WriteString(url.QueryEscape(v))
}
}
return buf.String()
}
8 changes: 8 additions & 0 deletions pkg/strutil/strutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,3 +281,11 @@ func TestParseCSVMap(t *testing.T) {
})
}
}

func TestOrderedQuery(t *testing.T) {
query := NewOrderedQuery()
query.Set("abc", "1")
query.Set("aa", "2")
result := query.Encode()
assert.Equal(t, result, "abc=1&aa=2", "")
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, this is not correct, as the ordering is ignored on parsing back the string to *net.URL

https://github.com/containerd/containerd/blob/e85b5a0b81a610f564c3240c69d8f7324b5ecadb/pkg/process/io_util.go#L31

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I ask if it is correct if we sort the args when in the nerdctl xmain function?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, we have a fallback here https://github.com/containerd/nerdctl/blob/master/cmd/nerdctl/main.go#L75-L80 to help us ignore the args order. Maybe we don't need to concern about the args order?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I ask if it is correct if we sort the args when in the nerdctl xmain function?

No, that doesn't work with nerdctl run busybox echo "_NERDCTL_INTERNAL_LOGGING"

Maybe we don't need to concern about the args order?

Need