Mount multus socket and invoke delegate #24
Conversation
maiqueb
force-pushed
the
mount-multus-socket-and-invoke-delegate
branch
2 times, most recently
from
5d0b9bb to
0e518ba
Compare
maiqueb
force-pushed
the
mount-multus-socket-and-invoke-delegate
branch
6 times, most recently
from
a323b42 to
62860a5
Compare
maiqueb
force-pushed
the
mount-multus-socket-and-invoke-delegate
branch
from
62860a5 to
cc9f5f2
Compare
|
|
||
| var _ = Describe("multuscni REST client", func() { | ||
| const ( | ||
| cniVersion = "0.4.0" |
There was a problem hiding this comment.
This cniVersion has nothing to do with the multus-cni.v3 right ?
There was a problem hiding this comment.
Right. It's the CNI spec version used.
| return string(newIfaceString), nil | ||
| } | ||
|
|
||
| func DeleteDynamicIfaceFromStatus(currentPod *corev1.Pod, netName string, ifaceName string) (string, error) { |
There was a problem hiding this comment.
Why we don't pass the netName as NetworkSelectionElement like at the Add counterpart then we use
We have the namespacedName(networkSelectionElement.Namespace, networkSelectionElement.Name) function
There was a problem hiding this comment.
network status features as net name a namespaced name. Check the caller of this function in https://github.com/maiqueb/multus-dynamic-networks-controller/blob/cc9f5f2e9a3f87156f253e139a0d1b566bb01ac3/pkg/controller/pod.go#L321
There was a problem hiding this comment.
I mean changet that function to have the same time for netName as "AddDyn..."
| return fmt.Errorf("failed to compute the updated network status: %v", err) | ||
| } | ||
|
|
||
| if err := pnc.updatePodNetworkStatus(pod, newIfaceStatus); err != nil { |
There was a problem hiding this comment.
We should reconstruct and retry on update conflict error so we are sure we take whatever others are updating.
https://pkg.go.dev/k8s.io/client-go/util/retry#RetryOnConflict
There was a problem hiding this comment.
If we fail, we re-queue the request on the controller queue, and the whole thing will be re-tried.
pkg/controller/pod.go
Outdated
| @@ -307,12 +318,38 @@ func (pnc *PodNetworksController) removeNetworks(dynamicAttachmentRequest *Dynam | |||
| } | |||
| klog.Infof("response: %v", *response) | |||
|
|
|||
| newIfaceStatus, err := annotations.DeleteDynamicIfaceFromStatus( | |||
| pod, | |||
| namespacedName(netToRemove.Namespace, netToRemove.Name), | |||
There was a problem hiding this comment.
Let's just pass netToRemove and do the namespacedName inside it.
There was a problem hiding this comment.
Why ? It doesn't need anything else.
There was a problem hiding this comment.
to make "AddDy" and "DeleteDyn" more symmetric is weird that Add use a proper struct while Delete use a string.
maiqueb
force-pushed
the
mount-multus-socket-and-invoke-delegate
branch
from
cc9f5f2 to
5336b90
Compare
Signed-off-by: Miguel Duarte Barroso <[email protected]>
Add a pkg to contact the multus RESTful API listening to a UNIX socket. This multus client is not invoked; this will happen in follow-up commits. Signed-off-by: Miguel Duarte Barroso <[email protected]>
Signed-off-by: Miguel Duarte Barroso <[email protected]>
maiqueb
force-pushed
the
mount-multus-socket-and-invoke-delegate
branch
from
5336b90 to
8c608bf
Compare
|
Squashed some vendoring changes commits in this forced push. |
This commit updates the pods network-status annotation whenever an interface is hot-plugged to the pod (or removed from it ...). Signed-off-by: Miguel Duarte Barroso <[email protected]>
maiqueb
force-pushed
the
mount-multus-socket-and-invoke-delegate
branch
from
8c608bf to
f8b1577
Compare
This PR mounts the multus-cni server socket into the dynamic network's controller pods (they run as a daemonset).
It introduces a new package -
multuscni- where the multus REST client will be located: it currently invokes a single endpoint -/delegate- and is used to trigger adding / removing interfaces from a running pod.It currently does not have a way to pass runtime config information to the multus server, thus use cases like static IPAM with IP / GW defined in the pod are not possible.
Fixes: #8