Wire proposal

[adracus]

This proposes the wire API for configuring switches

[hardikdr]

Thanks for the proposal @adracus @afritzler

[peanball]

Some typos, some clarification suggestions (to be discussed), some questions.

[peanball]

Suggested change

	* The `Cell` expresses desired functionality and leaves the switch specific
	details to the implementation., not as implementation instructions.
	A DHCP relay is a desired feature, while the per-port VLAN that is in the
	current setup is an implemetation detail.

Not sure if this point fits here or before. But I think it's worth spelling this out anyway to make sure we focus on functionality and not on switch flags.

[adracus]

Incorporated passage.

[peanball]

So is adminState on the interface also immutable? This is mostly needed to prepare for physical changes in the data center and the techs there don't like, or rather won't, unplug a blinking port.

It's fair enough though to have to create a new Interface that swaps out only the admin state.

Is there any issue with concurrency and name conflicts possible with this?

[adracus]

I now removed adminState from the spec entirely. wire only cares about configuring the cells in an environment. Mutating adminState should be done in a lower-level controller (e.g. sonic operator).

[peanball]

what is the difference between providerID and handle (as used in Interface and Cell?)

[adracus]

It follows the same pattern we have in ironcore - providerID denotes a root resource (like MachinePool's providerID https://github.com/ironcore-dev/ironcore/blob/0369ab0095e1ff571c42fc7b59b9302aa1d5ca61/api/compute/v1alpha1/machinepool_types.go#L17) and handle denotes a sub-resource (roughly like https://github.com/ironcore-dev/ironcore/blob/0369ab0095e1ff571c42fc7b59b9302aa1d5ca61/internal/apis/compute/machine_types.go#L156)

[peanball]

coming back to my question before on Interface: This then seems that Interface.spec.adminState is in fact modifiable on the fly and not immutable?

Re-read the first passage, leaving it for other readers anyway. Only the Cell is immutable, and is also what triggers reconfiguration of the switch.

The controller watches Nodes and Interfaces, which are mutable. But it only changes the switch's base configuration when a Cell is referenced. For Interfaces it should then apply changes ad hoc, assuming the corresponding Node is already configured / claimed via valid Cell ref.

@adracus, please confirm that my understanding is correct.

[adracus]

Nodes and Interfaces don't have any mutable fields in this proposal (yet) as well. They serve as an announcement by the implementation to say what participates in wire. Then, once a Cell references a Node / Interface, the implementation actually does its reconciliation.

[peanball]

Some further reflection on the naming. I think it's so generic that it becomes intangible.

[peanball]

A Node is an overloaded term: https://kubernetes.io/docs/concepts/architecture/nodes

We've criticized the term Device in other proposals, but this is still also not a generic network device but a switch.

My suggestion would be Switch, or NetworkSwitch.

[adracus]

Node does not stand on its own: In k8s, there's also the API group, which makes it a wire.ironcore.dev/Node.

Additionally, Node has some advantages over Switch or more specific terms, as our network now does not denote anymore that you need to have real switches, but also could just provision everything using e.g. plain linux boxes. Node hides how you implement it.

[peanball]

So the Cell is an applicable switch configuration? Could we give this a clearer name that reflects this is about a particular configuration?

My suggestion would be SwitchConfiguration, or if you wanted a SwitchClaim. It's the linked specific immutable configuration of the hardware represented by Switch (or now still Node).

[adracus]

Cell is a configuration that states how a Node participates in wire. In our concrete cases, this is for our switches, but again, this is not to leak the implementation.

[peanball]

I think this part is the crux of this whole proposal. The single object, and with that a "complete" state that can be applied and also verified (on the switch and in a controller) is the important bit.

I am still not 100% certain what the immutability helps, beyond being more annoying for the implementer and operator and with that complexity trying to deter them to reconfigure.

Making sure that the config is applied clean in one go is up to the implementer of the controller / agent / whatever else machinery that touches the switch. There is always the "current real state of the switch" to fall back on for making a diff if you wanted.

So having the single object with all the desired state in one thing is the main benefit of the approach outlined in this proposal. The clean update, and maintenance for taking as witch out of rotation could equally be done with taints / cordon (drain and do not use this switch) and resource versioning (this is the newly applicable version of the configuration. Apply it in full).

It is still logically the same switch in the same place with the same name, but it is e.g. running an OS update. When it comes back it's still the same switch.

This is also represented by keeping the Node (or as I suggest Switch) intact, and just changing the currently applied configuration (i.e. Cell, or SwitchClaim as I suggest), which needs to be applied to the switch in some way. But that way is the same for whether there is a new version of the same resource, or there is a new resource entirely.

What an update for me guarantees is that there is no possibility of something else interfering between "delete Cell" and "create new Cell".

[adracus]

Some cents here:

• What the immutability helps: In all previous proposals, there was no stable state: We had something like Switch or additional objects (like BGPPeering or similar) that, whenever they were created / updated, caused a reconfiguration of the switch. From the API level, you could not avoid that, as at any point in time it's of course ok to create e.g. new BGPPeering objects. With this design, we definitely know what the desired configuration is - just like k8s's Pods are immutable, our Cell is immutable.
• Regarding 'annoying the implementer': Actually, with this, the implementation becomes more straight-forward: No partial updates, complete state is presented from the get-go and can be applied. For the operator it's also simplistic as it is in k8s (no operator in k8s complains about it being hard to 'replace' pods): Just delete + recreate, with kubectl tooling its as simple as kubectl delete cell x and then kubectl apply cell y.