Remove cni dependency #7
Conversation
|
@adrianreber PTAL |
|
I am still not sure if this would not be necessary if the proposed fixes from #5 are merged. I was also concerned how this works with restoring IP addresses during container restore, but I just saw that you changed all that code a month ago. Has the format of the file |
Well #5 would be fine for podman because then I could bump the CNI version in podman. However there is no longer a reason for this lib to include CNI because the network status in podman no longer uses CNI structures.
Yes the format has changed, we no longer store a CNI result. We store a The only reason for this lib to use the network status is to show the IP address with
Since this change is for podman 4.0 I see no problems to break old checkpoints, only the network information is lost. Besides the network settings the restore of an old 3.X checkpoint should still work correctly. |
The reason why I created checkpointctl and the library is that I want to use the same code in CRI-O, Podman and Kubernetes as far as possible. As I am able to move checkpoints from Podman to CRI-O it is important to have compatible checkpoint archives.
Would be nice to have a format which is independent of internal changes. But currently in my CRI-O PR I am not using the network part of checkpointctl, so the code can be dumped and later introduced if necessary. But having a format for network information which is independent of an internal struct would be nice.
That does not sound like the best solution. Anyway, if you can make CI happy, this can be merged. |
|
Podman is actively moving away from CNI so using the same code for networking will be difficult. If you want to know about network settings with a stable format I would recommend to define a struct here in this lib with the fields you would like to know about. If there is such a struct the consumers (podman, crio, ...) would be required to fill in the data and dump in into the network status file. With that this lib could read it without having to worry about changes to the format. As long as podman can still dump the raw format into the checkpoint to read advanced options I would be fine with using a fixed struct from this lib for the network status file. |
Podman 4.0 no longer uses CNI to store the network information therefore reading the status file into a cni result will fail. This library should not make any assumptions about the network status format used by Podman. I personally don't see much value in displaying the IP address with checkpointctl for a podman container. A container can have 0-n IP addresses. Which one should be displayed anyway? Also removed the now obsolete network.status file tests. Signed-off-by: Paul Holzinger <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #7 +/- ##
==========================================
+ Coverage 55.97% 58.94% +2.97%
==========================================
Files 4 4
Lines 427 436 +9
==========================================
+ Hits 239 257 +18
+ Misses 149 145 -4
+ Partials 39 34 -5
Continue to review full report at Codecov.
|
Podman 4.0 no longer uses CNI to store the network information therefore
reading the status file into a cni result will fail. This library should
not make any assumptions about the network status format used by Podman.
I personally don't see much value in displaying the IP address with
checkpointctl for a podman container. A container can have 0-n IP
addresses. Which one should be displayed anyway?
Fixes #6
Signed-off-by: Paul Holzinger [email protected]