controller: Handle snapshotter per handler #303
Conversation
fidencio
force-pushed
the
topic/controller-properly-set-snapshotter-per-runtime-handler
branch
from
b11460f to
5f42870
Compare
|
/test |
stevenhorsman
force-pushed
the
topic/controller-properly-set-snapshotter-per-runtime-handler
branch
from
5f42870 to
b78fbde
Compare
|
During testing of this I found a bug in kata-deploy, so waiting on kata-containers/kata-containers#8721 to be merged to fix this. |
fidencio
force-pushed
the
topic/controller-properly-set-snapshotter-per-runtime-handler
branch
2 times, most recently
from
5c2e16a to
4ee7f81
Compare
|
I found a few more issues on the kata-deploy side, all addressed as part of: kata-containers/kata-containers#8733 I've also created a new PR on the Operator side in order to test with the kata-deploy payload, before this one / and the Kata Containers one gets merged: #310 |
controllers/ccruntime_controller.go
Outdated
| shim := strings.TrimPrefix(runtimeClass.Name, "kata-") | ||
| if snapshotter_handler_mapping == "" { | ||
| snapshotter_handler_mapping += shim + ":" + runtimeClass.Snapshotter | ||
| } else { | ||
| snapshotter_handler_mapping += "," + shim + ":" + runtimeClass.Snapshotter | ||
| } |
There was a problem hiding this comment.
this kata- prefix processing is repeated below. Can we do it only once? I also think preparing snapshotter_handler_mapping can follow the same strings.Join() approach used by runtimeClassNames.
There was a problem hiding this comment.
So, the prefix processing is done for a different reason here, and looking at the code is quite hard to make this only once, as kata-containers explicitly requires the shim name for all the operations, and for the sake of the user's experience the operator requires a full runtime class name (as in kata-qemu).
About the string.Join(), this can be done, I just need to either make sure , is correctly treated on the kata-containers side, or that we remove the last , that would be added by the strings.Join().
There was a problem hiding this comment.
or that we remove the last
,that would be added by the strings.Join().
The doc example suggests that it is not added: https://pkg.go.dev/strings#example-Join
There was a problem hiding this comment.
Cool, I've updated the PR and simplified a little bit the logic.
Thanks for the rveiew, @mythi!
fidencio
force-pushed
the
topic/controller-properly-set-snapshotter-per-runtime-handler
branch
from
4ee7f81 to
0ba092b
Compare
Let's start properly setting a specific snapshotter per runtime handler configured for containerd. This work depends on a work done on the Kata Containers side to better support setting snapshotters per runtime handler. The PR from the Kata Containers side is: kata-containers/kata-containers#8655. Signed-off-by: Fabiano Fidêncio <[email protected]>
fidencio
force-pushed
the
topic/controller-properly-set-snapshotter-per-runtime-handler
branch
from
0ba092b to
a0400af
Compare
Let's start properly setting a specific snapshotter per runtime handler configured for containerd.
This work depends on a work done on the Kata Containers side to better support setting snapshotters per runtime handler.
The PR from the Kata Containers side is:
kata-containers/kata-containers#8655.