-
Notifications
You must be signed in to change notification settings - Fork 85
api,adaptation,generate: allow adjusting linux net devices #157
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
base: main
Are you sure you want to change the base?
Conversation
df6df04 to
02f0b68
Compare
|
❤️ |
02f0b68 to
d886c51
Compare
df21d77 to
20533ae
Compare
20533ae to
6e14a15
Compare
|
/cc @aojea @tao12345666333 Here is the draft PR proposal #180 was asking for. |
|
This is fantastic @klihub , can we undraft it for final review |
6e14a15 to
a03dcca
Compare
We can try to do that to get it reviewed. @mikebrow WDYT? But I think we won't be able to get this merged before the necessary new bits in runtime-spec gets behind a tag. Both CRI-O and containerd main/HEAD is at [email protected] and I think we usually tend to stick to a tagged version in both even in main. |
|
ok, that is fair |
|
Fixes: #180 |
a03dcca to
a999d9b
Compare
|
Containerd containerd/containerd#12295 is already testing runc 1.4.0-rc.2 |
|
Thx for the update @aojea |
8df6cec to
0a96043
Compare
@aojea @mikebrow But I think we'd still need a tagged version of |
|
@klihub yeah, I was using the comment as a breadcrumb , so next time I check I can track teh history, otherwise I forget I was not trying to push for merging before that, sorry for the confusion |
@aojea No prob / no offense taken. I just asked to make sure I understand correctly where we are and whether we can try to move this forward yet. |
|
informational update , ETA for 1.3.0 spec is 2025/11/04 opencontainers/runc#4875 (comment) , runc will follow |
|
@aojea @mikebrow With opencontainers/runtime-spec v1.3.0 now tagged and released, unfortunately opencontainer/runtime-tools is now badly out of sync with runtime-spec. [email protected] brings a flips the linux PID limit setting the from I filed a PR for fixing this but I'm not sure what are the chances of getting that merged soon |
| LinuxIOPriority io_priority = 6; | ||
| SecurityProfile seccomp_profile = 7; | ||
| LinuxSeccomp seccomp_policy = 8; | ||
| map<string, LinuxNetDevice> net_devices = 9; |
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.
What are the keys, and why is a map chosen vs. repeated?
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.
Because network interface names are unique, so there is no need to validate or fail at runtime, the keys are the interface name
|
it seems the dependency problem got unblocked |
0a96043 to
bf5c053
Compare
Allow adding and removing container linux net devices. Signed-off-by: Krisztian Litkey <[email protected]>
bf5c053 to
c2faa3d
Compare
Signed-off-by: Krisztian Litkey <[email protected]>
c2faa3d to
c5f3eae
Compare
| Name: d.Name, | ||
| }) | ||
| if !verbose { | ||
| log.Infof("%s: injected network device %q -> %q...", containerName(pod, ctr), |
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 is unrelated since I see this is an established pattern, but I'm not following why we would conditionally write info logs. I would think we would always info log, and then conditionally decide whether or not to additionally dump verbose output above
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.
True. It is a bit strange not to log that info in verbose mode. We can fix all instances of that wrong pattern in a separate PR.
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.
This PR adds NRI support for injecting 'linux net devices' a.k.a host side network interfaces into containers, building upon all the work @aojea did to make this possible.
Notes:
To test this in practice, one needs
runcorcrunwith support forLinuxNetDevices in the OCI Spec,