Add internal ipam, use in ip.cni, support ip.netns on backend net, atomic nft ruleset swap - #1106
Merged
Merged
Conversation
Two bugs in the nft handle that could not fire, because the only tables om wrote to were "nat" and "filter", which every host already has, and the only chains it made were already there from an older om. AddTable answered nil for a table it had just created. It looks the table up, creates it when absent, then looks it up again to return it, and that second lookup reads the cache the first one filled from a ruleset without the table. A caller handed that nil to AddChain, which dereferenced the table of the chain it was building, and the network setup died on a segmentation fault. The cache is dropped after the creation now, and a table still absent is an error rather than a nil. And fmtChain printed the address of a priority instead of a priority. Chain.Priority is a *ChainPriority in this version of the library, so "%d" formatted the pointer: a chain asking for the srcnat 100 was created with "priority 4219880", which put it behind every other chain on its hook. A masquerade there never runs. Both were found by moving om's rules to a table of its own, which is the next commit: nothing before it created a table or a chain that did not already exist.
"om net setup" made podman unable to start a container. Every start ended: Error: netavark: code: 1, msg: iptables: Chain already exists. om added its chains to "nat" and "filter", two of the five tables the iptables compatibility layer owns, and one of them, osvc-postrouting, is a base chain on the postrouting hook at the srcnat priority the standard POSTROUTING chain of that table already sits on. That makes the whole table unrepresentable in iptables terms: # iptables -t nat -S iptables v1.8.10 (nf_tables): table `nat' is incompatible, use 'nft' tool. Every firewall driver reaching the ruleset through iptables is blind to that table from then on. netavark is one of them, and it has no other driver before 1.9: it stopped seeing the chains it had created, created them again, and the kernel refused the duplicates. A node om had set a network up on could run no container of a podman-built network. The rules go in the "osvc" table now, one per family, on the same hooks and priorities as before. nftables evaluates the base chains of every table registered on a hook, so the rules keep working, and the tables om does not own are left representable. A setup deletes the chains om used to leave in nat and filter, or a node upgrading would keep hiding those tables forever. And it deletes om's own table rather than flushing its chains: a chain keeps the hook and priority it was created with, so flushing one an older om made would leave it wherever that om put it, which the priority bug fixed in the previous commit made a real address. Checked on a live three node cluster: nat and filter read back through iptables, "podman run" works again, and a container of an object on a routed_bridge network reaches the internet through osvc-masq.
The podman drivers passed "--cni-config-dir" on every command, from a node keyword with a default, so it was always there. Podman never read it. This model never asks podman to build a network: the netns keyword resolves to the host namespace, to a private one, or to the one of another container, and the addresses are configured from outside by the ip drivers. Podman looks up a network configuration in none of those cases. The flag was inert, and about to be worse than inert: podman 5 dropped the cni backend and the flag with it, where an argument nothing reads becomes an unknown flag and every podman command fails. That is RHEL 10 and any recent Fedora. The node keyword stays, and so does the cni.config context: ip.cni reads that directory to find the network configuration it hands the plugin, which is the one thing in om that does read it.
A container of an object using the pause container model had no resolver at all. It could not resolve a name of its own cluster, with nothing anywhere saying why. podman refuses the dns options in the two network modes that model uses: conflicting options: dns and the network mode: none conflicting options: dns and the network mode: container so the driver drops them, and has to. But podman writes no /etc/resolv.conf of its own for those modes either, in the pause container or in the containers joining it, so dropping the options left the container with whatever resolver its image ships, which for most images is none. The driver writes the file the options would have produced and mounts it read only. Written on every start, and read by the container for as long as it runs: a container adapts to a cluster layout change by being restarted, rather than by having a file changed under it. The content lives in the rescontainer package, shared with the docker driver, and is used from podman only for now. The search list is the domain of the object and each of its parents, so a name resolves in the namespace of the object first, then in its kind, then in the cluster. That walk was written twice: the --dns-search arguments are built from the shared one now. The nameservers stop at three, which is where glibc and musl both stop reading, and a cluster naming more says so in a warning rather than writing a line nothing reads. The search list is not capped: the implementations disagree on its limit, glibc counting domains and musl counting bytes, so the file says what was asked. Checked on a live cluster against four resolvers, in the pause container and in the two containers sharing its namespace: glibc from nginx, busybox, musl from alpine, and the go resolver from an image holding nothing but one static binary. Also against an image whose /etc/resolv.conf is a dangling symlink, which podman resolves. A short name reaches the peer object, an fqdn reaches it, and a public name resolves.
The dns_search keyword promised a default list the code has not built since v3: the fqdn of the object first, then its parents. It builds the domain of the object and each parent of that domain, and the fqdn is not in it. The difference is a feature of v2 that the layout the ip drivers are written for cannot deliver. The fqdn as the first search domain let a container reach a container of the same object by its hostname alone, because a hostname is published as "<hostname>.<objectfqdn>". That needs one address per container. Containers of an instance share a netns instead, and one ip resource carries one hostname, the one of the container its netns keyword points at, so there is one name for the whole instance and the containers in it reach each other over 127.0.0.1, which needs no name. The keyword text says what the list is and what it takes to search the fqdn too, since a v2 configuration relying on the shortname now gets NXDOMAIN with nothing to explain it. The changelog says the same, where a reader migrating looks. dns_name_suffix leaves the removed keywords of the ip driver: it is still there, still appended to the hostname the record is published under, and it is how an object with more than one address names them apart. The changelog also records the nft rules moving to the osvc table, and that a setup deletes the chains an earlier om left in nat and filter.
A setup added its rules one nft call at a time: 37 processes over a quarter of a second, and for that whole time the masquerade and the forward accepts were absent. Established flows kept their translation from conntrack, but a connection a container opened in that window left unmasqueraded and got no answer back. A daemon restart and a cluster configuration change both run a setup, so the window was not rare. A rule nft refused was worse: the ruleset stayed as the failure left it, part of it written and the rest not, with nothing to say which. The rules are rendered as one nft document now and handed to a single "nft -f -". The table is created, deleted and defined again in the same transaction, which the kernel applies in one step: the rules are never half there, and a refused rule leaves the ruleset untouched. The chains an older om left in the nat and filter tables are deleted in that same document, and only the ones the node has, since nft aborts a transaction on a delete of an absent chain. Rendering the wanted state replaces accumulating it, so the flush a second run needed is gone with the code that needed it: AddTable, AddChain, the two chain formatters, the five rule adders, FlushChains and the dead debugRules. The priority bug fixed two commits ago lived in fmtChain, which this deletes. A test renders the document for a mixed v4 and v6 network set and reads it back, and another has the nft binary parse it with --check, so a syntax error fails a test rather than a node. Checked on a live three node cluster: one nft call instead of 37, the tables identical to what the rule adders built, a second setup idempotent, and a container still resolving a peer and reaching the internet.
A node with no address in the family of a routed_bridge network logged: network: backend5: network setup: get local ip: address dev2n1: no suitable address found and left the reader to work out the rest from the code. The setup stops between the bridge and the peers, so the network is left half made: the bridge is up with its gateway address, no tunnel and no route reaches a peer, and setupNetwork returns before writing the cni configuration, so the file an object needs to start is not there. None of that is in the message, and the network looks present. It says so now, with the way out. The addr keyword of a routed_bridge is scopable, so an address for this node is the fix. Its network keyword is not, deliberately: the subnet is one cluster wide plan sliced per node, and there is no value that disables it here alone. There is therefore no configuration in which a node without an address is a resting state, which is why this stays an error rather than becoming a warning nobody reads. The wrapped error names the family it looked an address up in, which was the one fact needed to tell a misconfigured node from a node without ipv6.
The first half of taking the addressing of the om networks away from the host-local cni plugin: the allocator, and the one thing it needs from a network driver. It allocates on one node, from the addresses that node draws from, and that is what makes it safe with no lock held across the cluster. The two drivers reach that property differently. A routed_bridge slices its network, one subnet per node, and routes between them, so two nodes never draw the same address. A bridge hands every node the whole subnet, but its addresses are node local and not routable, so the same address on two nodes never meets. AllocatableRange is the one method that distinguishes them, and a driver that does not implement it, the lo network, is one om allocates nothing in. NodeSubnet could not serve as that method: it is the slicing, it divides by ips_per_node, and it writes subnet@<node> back to the cluster configuration. The default network is implicit and has no configuration to write to. An address is drawn from a hash of the key rather than from the first free address, so a resource keeps its address across restarts with no record of what it drew last. host-local reallocates on every add, which is why an object renumbers itself and its name moves. The walk from that first candidate is what resolves two keys landing on the same one, which a small range makes ordinary rather than unlikely. The key names the resource, not the object: an instance holds as many ip resources as it needs, several of them in one network, and each has an address of its own. Key() is in the package so a caller cannot key on the object alone and have ip#0 and ip#1 draw the same address. An allocation avoids what this node reserved, what an allocator sharing the range reserved, and what the cluster reports in use. The second is host-local while ip.cni is still served by it: an address it handed out has no reservation here. The third is the daemon's view of the instances of every node, which sees addresses whose reservation file this node cannot read. Nothing calls this yet. The ip drivers are next.
An object attaching to a cluster network had to say where, how wide and through what: [ip#0] type = netns netns = container#0 name = 10.22.149.134 netmask = 16 dev = obr_default gateway = 10.22.0.1 which is the network repeated in the object, four values to keep in step with it, and an address a reader had to pick by hand and remember was taken. ip.cni needed one line for the same thing, the name of the network, because the plugin behind it did the rest. It needs one line now too: [ip#0] type = netns netns = container#0 network = default The dev, the netmask and the gateway are the network's to know, and are read from it when the configuration is silent. An explicit value still wins, so nothing changes for a configuration that sets them. The address is drawn by the allocator, from a hash of the object and the rid, so a resource keeps its address across restarts and two resources of one instance in one network do not draw the same one. The network keyword held the address of the network in dotted notation, which set the destination of the route del_net_route removes. That destination is the connected route the kernel adds along with the address, so the keyword could name no other: the length has always come from the netmask, and a base that is not the address masked names a route that is not there. It is derived now. A value still in that form is reported as obsolete and ignored, and a value that is neither an address nor a network is refused, naming the networks that exist, which is what a rename or a typo deserves. dev stops being required, since a network names it. The label names the network the address came from, as the cni label does. An empty name is no longer a warning: it is how a resource says the address is the network's to choose, and the warning is for a resource that names neither. Checked on a live cluster: one address allocated and the veth enslaved to obr_default with the mask and the gateway of the network, the same address drawn again after a stop that released it, two resources of one object in one network holding 10.22.149.134 and 10.22.151.57, and both the obsolete and the unknown value reported for what they are.
Two entries under the ip driver. The network keyword of ip.netns names the om network the address is drawn from now, as it does on ip.cni, and the dev, the netmask and the gateway are read from that network when the configuration is silent. It used to hold the address of the network, for the destination of the route del_net_route removes, which is derived now: a value in that form is reported as obsolete and ignored. A reader migrating a configuration has that spelled out, and so is the reason the old value could name no other route than the one that is derived. And the addresses of the bridge and routed_bridge networks are om's to allocate rather than the host-local plugin's. A resource draws the same address every start, so an object is not renumbered by a restart and the name it is published under keeps resolving to the same place, which is a behaviour change from the plugin worth expecting.
An address a resource holds must be a reservation of om's before om allocates in that network, or the switch from another allocator goes wrong both ways. The plugin that was handing them out stops running, so it stops releasing what it gave: its record then blocks every address in it forever, and ignoring that record hands out addresses objects are still using. A network setup records them. It is where the once per network work of a node already happens, it runs on daemon start and on every cluster configuration change, and it leaves the resource start path local: no daemon is asked anything to start an object. The addresses are read from the status every object caches on this node, which names the resource holding each one. The cache of the host-local plugin cannot serve: it records the pid of a network namespace as the holder, which says nothing about which resource that is. An address adopted without a key is an address no stop can free, since a free matches the key of the resource, so it would leak for as long as the store lasts. The key is what makes an address of the plugin releasable, and the pinning it also buys is a side effect rather than the reason. An adoption leaves an address already reserved alone, whoever holds it, so it changes nothing run twice, and run late it does not take an address from the resource that drew it. It reports what it recorded, so a reader of the setup log sees the switch happen. Checked on a live cluster: four addresses adopted in one network and one in another, under the keys of the objects holding them, and a reservation created by adoption alone released by the stop of its resource. Also drops invalidate and networkFamily from the firewall, left with no caller by the nft transaction.
The last allocator of an om network that was not om. ip.cni asked the host-local plugin for an address, ip.netns asks om, and one network serving both had two allocators drawing from one range with no idea of each other. The plugin keeps the wiring, which is what a cni plugin is for, and loses the addressing. The ipam section of the configuration om already pipes to it on stdin is rewritten: static, holding the address om allocated, the mask of the range and the gateway both network drivers put their bridge at, which is the address host-local defaulted to. The routes of the network are carried over untouched, or a container loses its default route. Nothing else of the configuration is touched. Nothing changes in an object configuration, and nothing changes for a network om does not own: a cni configuration an administrator wrote for a plugin om knows nothing about keeps the ipam it was given, and om allocates nothing in it. A network keyword naming neither a configuration on disk nor a network of the cluster is reported when the resource is loaded, rather than failing later with a missing file. The allocator construction moves to core/network, so both drivers build it the same way rather than each keeping a copy. Checked on a live cluster: an object allocated 10.100.0.240, the namespace came up with that address, the gateway and the route to the network, it reached a peer object by name and the internet, host-local recorded nothing, and a restart drew the same address again, which it never did before.
The host-local plugin does not allocate for om any more, so nothing it left behind is created again, and everything om had to tidy up after it is dead. The three purge functions go, and with them the reason they were hard: a host-local reservation names its holder by the pid of a network namespace, which dies and is reused, so a leak could only be guessed at from a file being empty and more than five seconds old. An om reservation names the object and the rid, which do not die, and the resource that took an address is the one that releases it. The retry goes too. It fired on the two messages host-local answers a range it cannot allocate in with, cleaned every allocation of the network and ran the plugin again, which is a heavy answer to a bookkeeping problem and no answer at all now that the bookkeeping is om's. A plugin that fails is reported. The record of the plugin is still read, and still never written: a node that has not run a network setup since the upgrade has addresses in it that no reservation of om's covers yet. It stops being read when it stops holding anything, which is a change of its own.
The record of the host-local plugin is read so an address it handed out is not handed out again, and nothing removes from it any more: the plugin does not run, and the code that tidied up after it is gone. Every address in it would be unusable for as long as the file lasts. A network setup drops the ones om reserves. Those say nothing om does not know, and om is the one that releases them now. Only those. An address in the record that om accounts for in no way is left, and stays excluded: the reason it is there may be a resource whose status this node has not read, and an address wrongly freed is an address handed to two resources. It is reported, with the directory to remove it from once nothing uses it, which is a judgement for a reader rather than for a setup. The files of the record that are not addresses are the bookkeeping of whoever wrote it, and are neither read nor removed. Checked on a live cluster: two addresses dropped from one network and one from another, the last_reserved_ip and lock files untouched, and one address left with the warning naming it, which om net ip ls confirms no object holds.
An address is reserved before the namespace is wired, so a start that fails between the two leaves the resource holding one while it is down. Stopping released it, but nothing says a failed start is followed by a stop: an object deleted as it stands leaves a reservation naming an object that no longer exists, which nothing will ever come to release. Two answers, because one of them cannot be complete. The allocation is registered for rollback, so a start that fails past it releases the address as it unwinds the links. That covers the way an address is lost in practice. And a network setup releases the reservations whose key names an object this node does not have. That covers the ways a rollback cannot: a daemon killed between the reservation and the start, a crash, a bug. It only releases what it knows to be gone, since an object that exists and holds an address may be one that has just reserved it and not yet finished starting, and an address released twice is an address given to two resources. Found by walking an object through a start that fails after the allocation, then deleting it: the reservation stayed, and the address was lost for good. It is released now, and the one already lost was released by the next setup.
Naming the lo network in an ip.netns resource failed with ip#0: start: mtu: route ip+net: invalid network interface name which is the device lookup failing on the empty name it was left with. om draws no address from that network and it has no bridge, so there is nothing there for this resource to be built out of, and saying so when the resource is loaded beats a message about an interface nobody named.
A github runner refused the ruleset a test renders: Error: Interface does not exist iif "obr_backend3" counter accept iif is an interface index, resolved when the rule is loaded, so a rule naming a device that does not exist cannot be written. The test named the devices of a cluster and passed only on a machine that had them. The test was right to fail. The rules are one transaction now, so a rule nft refuses is the whole ruleset refused: one network whose bridge is not up would leave the node with no firewall at all, no masquerade and no forward accept, for every network. An index also goes stale when a bridge is deleted and recreated, where the rule then names whatever took its number. iifname matches the name, at runtime, and is written whether the device exists or not. A device named past the sixteen characters the kernel accepts is refused all the same, so it is left out with a warning: it cannot exist, no rule of it would ever match, and it would otherwise take the ruleset of every other network down with it. It is treated as a network with no device, which is what a public network already is.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.