-
Notifications
You must be signed in to change notification settings - Fork 561
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add cap-add && cap-drop flags #55
Conversation
run.go
Outdated
Name: "cap-add", | ||
Usage: "Add Linux capabilities", | ||
}, | ||
&cli.StringSliceFlag{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please gofmt
run.go
Outdated
} | ||
opts = append(opts, oci.WithCapabilities(caps)) | ||
|
||
caps = []string{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please avoid reusing caps variable here. This should be a new variable like capsDrop
Could you use full name? |
run.go
Outdated
@@ -54,6 +54,7 @@ import ( | |||
"github.com/pkg/errors" | |||
"github.com/sirupsen/logrus" | |||
"github.com/urfave/cli/v2" | |||
"github.com/containerd/cri/pkg/util" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we avoid CRI dependency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok I move it inside nerdctl
Signed-off-by: fahedouch <fahed.dorgaa@gmail.com> fix MR Signed-off-by: fahed dorgaa <fahed.dorgaa@gmail.com> fix MR Signed-off-by: fahed dorgaa <fahed.dorgaa@gmail.com> fmt Signed-off-by: fahed dorgaa <fahed.dorgaa@gmail.com>
d253f26
to
198a3b5
Compare
|
||
// InStringSlice checks whether a string is inside a string slice. | ||
// Comparison is case insensitive. | ||
func InStringSlice(ss []string, str string) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: this should be in existing strutil.go . Also please clarify the origin of the code in the comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AkihiroSuda what do you mean by existing strutil.go
? in nerdctl project or in other project ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -0,0 +1,27 @@ | |||
/* | |||
Copyright The containerd Authors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing nerdctl authors comment I think
Merging. I'll address nits in a follow-up (EDIT: done AkihiroSuda@1f9b447) |
follow-up to #55 Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
Tests for this is being added in https://github.com/AkihiroSuda/nerdctl/pull/56/files |
follow-up to #55 Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
Signed-off-by: fahedouch fahed.dorgaa@gmail.com