run: Add --uts flag #1683
run: Add --uts flag #1683
Conversation
dcantah
force-pushed
the
uts-flag
branch
6 times, most recently
from
fa180fd
to
bc79305
Compare
|
Thanks for your PR! In my personal opinion, I think it's unnecessary to change the If we need to change all signatures related to |
|
It's not worth the hassle don't worry about it, I'll move the signature change to another PR (or open an issue and link to this). It was impacting the ability to just easily setup all the NS options enough it seemed warranted, and it's not exported or some relied on API here. Will try to do this sometime today! |
Zheaoli
added
status/needs-design-discussion
and removed
status/needs-design-discussion
labels
|
@Zheaoli, removed the setPlatformOptions signature change. I'll make an issue/separate PR. Thanks! |
dcantah
added
enhancement
Doesn't seem removed |
|
A separate PR is preferred, but a separate commit in the same PR is also ok IMHO (for this amount of PR) |
It's the same signature still, it's just over multiple lines as it was a bit long. The signature change was: Beforefunc setPlatformOptions(
ctx context.Context,
opts []oci.SpecOpts,
cmd *cobra.Command,
client *containerd.Client,
id string,
internalLabels internalLabels,
) ([]oci.SpecOpts, internalLabels, error) {Afterfunc setPlatformOptions(
ctx context.Context,
opts []oci.SpecOpts,
cmd *cobra.Command,
client *containerd.Client,
id string,
internalLabels *internalLabels,
) ([]oci.SpecOpts, error) {but I reverted the |
Yea, this didn't seem too unwieldy to all be in one PR, but I'll just make a separate at this point no worries. Edit: #1771. I changed the signature a bit more since the above and got rid of passing in opts. |
| } | ||
| internalLabels.pidContainer = pidInternalLabel | ||
| opts = append(opts, pidOpts...) | ||
|
|
There was a problem hiding this comment.
The pidns changes seem irrelevant to utsns, could you consider splitting the commit? Not necessary to open a separate PR.
There was a problem hiding this comment.
Sure, will try and get to today
Adds support for specifying the host UTS namespace (the only thing docker supported), and a small test for this. Signed-off-by: Danny Canter <danny@dcantah.dev>
This change just consolidates all of the namespace validation/setting logic into a single function and will return all of the resulting options. Signed-off-by: Danny Canter <danny@dcantah.dev>
Adds support for specifying the host UTS namespace (the only thing
docker supported). This also consolidates the namespace options
checks for linux on run into a single function.