validate: add network host-specific check

[Mashimiao]

Signed-off-by: Ma Shimiao mashimiao.fnst@cn.fujitsu.com

[wking]

linux.network is a strange setting, because with cgroup setup happening during creation (also before post-create hooks, or whatever we end up calling them, opencontainers/runtime-spec#532) there's no time for hooks or post-create activity to populate a new network namespace before this cgroup is configured. Is this setting only intended for containers joining (or staying in) existing network namespaces? Or can you set priorities for interfaces that don't exist yet, knowing that you'll be creating the interfaces after create finishes up?

[Mashimiao]

It seems hooks can't populate a new network namespace. And runtime SPEC doesn't say how container can setup new network interface in a new network namespace. So, I think we can only set priorities for host interfaces.

[wking]

On Tue, Aug 23, 2016 at 06:42:57PM -0700, Ma Shimiao wrote:

•   for _, prio := range r.Network.Priorities {
  

•       exist = false
  

•       for _, ni := range interfaces {
  

•           if prio.Name == ni.Name {
  

•               exist = true
  

•               break
  

•           }
  

•       }
  

•       if !exist {
  

•           msgs = append(msgs, fmt.Sprintf("Interface %s does not exist on host", prio.Name))
  

It seems hooks can't populate a new network namespace.

Why not? I think this is one of the main use-cases for hooks or other
post-create, pre-start activity. The problem here is that these
cgroup settings are applied during create before those hooks or
post-create actions have fired.

And runtime SPEC doesn't say how container can setup new network
interface in a new network namespace.

Agreed.

So, I think we can only set priorities for host interfaces.

Or if you join an existing interface, since with either of those the
interfaces will exist when the cgroup priorities, etc. are setup.

I'm not clear on how configuring a new cgroup limiting interfaces from
a joined namespace interacts with the language from
opencontainers/runtime-spec#158. It certainly seems to fall under a
literal interpretation of “anything else related to that namespace”.
But the spirit of that limitation (as I see it) was to avoid
contention for shared resources (e.g. multiple containers trying to
set the hostname in a shared UTS namespace), and if you're creating
and configuring a new cgroup you aren't stepping on another
container's toes.

[Mashimiao]

runtime uses cgroups to set container's resource limit. I think when setting cgroups, we does not need to concern about how namespaces have been set.

[wking]

On Wed, Aug 24, 2016 at 02:40:39AM -0700, Ma Shimiao wrote:

runtime uses cgroups to set container's resource limit. I think when
setting cgroups, we does not need to concern about how namespaces
have been set.

A few possible positions here:

a. The current spec wording requires errors “if the config specifies
anything else related to that namespace” [1,2] if a namespace entry
is listed with a ‘path’ (although see 3). I think it's pretty
clear that network interfaces are related to network namespace
(each interface belongs to one namespace). So if we strictly stick
to the current spec wording, I think we have to care about
namespace setup here.

b. I don't see any problem with configuring a new cgroup which happens
to be limiting an old namespace, since that won't step on anybody's
toes (more on this in 3). We'd need to change the spec wording
if folks agree with this.

c. I don't see any problem with configuring an existing cgroup
(assuming you have the appropriate permissions, which the kernel
will check when you try and change the cgroup). Folks can setup
cgroup permissions or audit configs as they see fit to enforce any
anti-tweaking policy they like, but I don't think we need to bake
those restrictions into runtime-spec. We'd need to change the spec
wording if folks agree with this.

Unless/until we land a spec change (which I'm in favor of), I think we
need to stick to (a).

[Mashimiao]

I don't know how get how the network namespace is set if we are not in that namespace.
But I think this PR can check part of interface priorities for host-specific.
we restrict to execute the check when container needs a new network namespace or does not set network namespace. How do you think about it? @wking

[wking]

On Wed, Aug 24, 2016 at 06:36:35PM -0700, Ma Shimiao wrote:

we restrict to execute the check when container needs a new network
namespace or does not set network namespace.

It's not clear to me when cgroups are created/joined/configured
vs. namespaces being joined/created. I'm also not clear on how the
cgroups are associated with network namespaces (do all network cgroups
limit root-network-namespace interfaces? Is there a way to bind
network cgroups to a particular non-root network namespace?). So this
is all very murky for me.

I'd rather:

1. Wait until [linux] Tweaking host namespaces? runtime-spec#537 and
   config-linux: Extend no-tweak requirement to runtime namespaces runtime-spec#538 settle down.
2. Follow them up with language to replace the current
   namespace-specific no-tweaking language with some more generic
   no-tweaking language ((b) in 1).
3. Have somebody research and report back on how network cgroups and
   network namespaces are supposed to work together.

and then come back and figure out how to handle validation. Before
those pieces are in place, I think the link between the spec and the
validating code will be a bit hand-wavy. If we want to land some
hand-wavy stuff because we expect that's where the spec will end up,
that's fine; I just don't have a clear enough understanding
personally.

[Mashimiao]

OK, I will wait until the situation is clear

[wking]

On Thu, Aug 25, 2016 at 07:56:06PM -0700, Ma Shimiao wrote:

OK, I will wait until the situation is clear

With the steps from 1:

• (a) is settled (config-linux: Extend no-tweak requirement to runtime namespaces runtime-spec#538 was merged today).
• (b) has an in-flight PR (config: Shift Linux-namespace no-tweaking rule to a generic rule runtime-spec#540, submitted
  just now).

[Mashimiao]

ping @opencontainers/runtime-tools-maintainers

[liangchenye]

LGTM

[vbatts]

LGTM