cni icon indicating copy to clipboard operation
cni copied to clipboard

spec: add ability for ADD to return arbitrary k/v data to be passed back to DEL

Open eyakubovich opened this issue 8 years ago • 4 comments

Current design requires the same set of args/data to be passed to both the ADD and DEL commands. This is slightly tricky b/c the net conf (JSON) is often stored on disk. In that case, the container runtime needs to save a copy of it in case it gets changed by the user prior to DEL command execution.

Since the runtime must be saving data to disk, we can clean the API a bit by having the ADD command return a dictionary with arbitrary key/values for the runtime to persist. It would then pipe it back to DEL command. It is more general b/c it allows the plugin to not have to serialize state to disk itself (at least for a subset of cases).

Example ADD:

$plugin_type < /etc/cni/mynet.conf
{
    "ip": "1.2.3.4/24",
    "data": {
        "masqChain": "xyz123",
        "bridgeName": "abc"
    }
}

Followed by DEL:

$plugin_type <<EOF
{
    "masqChain": "xyz123",
    "bridgeName": "abc"
}
EOF

eyakubovich avatar Oct 30 '15 19:10 eyakubovich

It is a good idea to pass this state out to the caller, but I would like to keep it optional/configurable.

This might be related to #116, because if we allow every plugin to have a state we should at minimum offer to take care of the state as we already do with the host-local allocator. It looks like this is developing to be a generic pattern so that every plugin can use a store backend to serialize state to. If the intention is to pass the state back to the caller, a store plugin possibly called stdout would just write it to stdout, a plugin possibly called embed would embed it in the plugin's result. The file backend we have so far would be an option too of course.

If this sounds reasonable not only to me I'll prepare a PR.

RFC @squaremo @thockin @jonboulle @zachgersh @jonboulle

steveej avatar Feb 12 '16 10:02 steveej

@zachgersh @squaremo @jonboulle @philips awaiting your feedback

steveej avatar Apr 15 '16 19:04 steveej

FWIW, I like the idea of this as an optional CNI feature, but I'd suggest that when the k/v's are passed back to the plugin on DEL, that they also be nested underneath a data field. Otherwise I'd be concerned contaminating the top-level namespace.

rosenhouse avatar Apr 18 '16 07:04 rosenhouse

This might be related to #116, because if we allow every plugin to have a state we should at minimum offer to take care of the state as we already do with the host-local allocator

I don't see this as the same situation -- so I don't think that just because host-local happens to put its state on disk, it should be a general mechanism.

I'm not sure I see the need for making this an item of configuration -- is it really up to the operator to decide what kind of backend store is used? Or is it up to the client application?

Usually I'd expect the client to have its own idea of where state goes, but we can certainly make it easier by providing lowest-common-denominator procedures in libCNI.

squaremo avatar Apr 18 '16 14:04 squaremo