Pass status along ipam #1050
Pass status along ipam #1050
Conversation
We must manually adapt the containernetworking IPAM invoking functions, since as of today, the PRs adding that support are not merged. Status is being added in [0], which was extracted from [1], where GC is added. [0] - containernetworking/plugins#1050 [1] - containernetworking/plugins#1021 Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
There was a problem hiding this comment.
I looked at the Bandwidth tests. Since containernetworking/cni#1039, the behavior of json.Marshall seems to have changed for struct that embeds types.NetConf.
With CNI v1.2.0: https://go.dev/play/p/L4feVRtl5S4
With CNI v1.1.2: https://go.dev/play/p/4RrrgKJUEN3
So, here in bandwidth tests, all parameters are missing when calling ADD.
|
@LionelJouin very good point. Do you think this affects more than just tests? I'm not sure if there's anything that embeds |
|
Could be. I created a ticket for it: containernetworking/cni#1096 |
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
The `skel.PluginMain` functions were deprecated in CNI v1.2.0. Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
Signed-off-by: Miguel Duarte Barroso <mdbarroso@redhat.com>
maiqueb
force-pushed
the
pass-status-along-ipam
branch
from
32d76ea
to
a06671a
Compare
| @@ -74,7 +74,7 @@ var _ = Describe("Loopback", func() { | |||
| session, err := gexec.Start(command, GinkgoWriter, GinkgoWriter) | |||
| Expect(err).NotTo(HaveOccurred()) | |||
|
|
|||
| Eventually(session).Should(gbytes.Say(`{.*}`)) | |||
|
Is this ready to merge, or will you be implementing STATUS passthrough in this PR? |
@squeed did you mean GC ? This PR does just that (i.e. status passthrough). I'd rather defer GC to another PR. |
|
I don't see anything providing an implementation of Status, so this PR lays the groundwork but doesn't, AFAICT, pass STATUS to a delegated plugin. |
I see ! Yeah, let's defer that as well :) |
This PR bumps the plugins to use CNI 1.2.0 and allows delegating the STATUS verb to IPAM plugins.
This way, other CNI plugins implementing STATUS can delegate the STATUS calls to IPAM CNIs.