Add support Edit ContainerMount for kata/direct volume #169
Conversation
…t volume Try to use CDI to 'patch up' the Container Mount for kata/runtime-rs using direct volume in K8S/CSI scenario. related issue and PR as below: CDI issue: cncf-tags/container-device-interface#162 CDI Mount PR: cncf-tags/container-device-interface#169 Fixes: containerd#9203 Signed-off-by: alex.lyn <alex.lyn@antgroup.com>
There was a problem hiding this comment.
@Apokleos thanks for the PR.
As already pointed out in the linked issue, misusing the device name to pass data is not something that we can support in the specification.
Let us continue the discussion there before we make a decision on this.
pkg/cdi/cache.go
Outdated
| @@ -254,6 +259,19 @@ func (c *Cache) InjectDevices(ociSpec *oci.Spec, devices ...string) ([]string, e | |||
| return nil, nil | |||
| } | |||
|
|
|||
| func (c *Cache) UpdateMounts(ociSpec *oci.Spec, mounts ...string) ([]string, error) { | |||
There was a problem hiding this comment.
This function is incorrectly named. It does not only update the mounts. It performs the same operations as InjectDevices with the exception that unresolved devices are output instead of returning an error.
In theory, I would not be against being able to control whether allowing unresolved devices when performing injection, but this is not the way I would propose to do it. Exposing an option at construction time and allowing a caller to retrieve the list of unresolved devices could be a possible solution.
There was a problem hiding this comment.
Thx @elezar I have updated my code. Please review it again
pkg/cdi/cache.go
Outdated
| func (c *Cache) UpdateMounts(ociSpec *oci.Spec, mounts ...string) ([]string, error) { | ||
| unresolved, edits, _ := c.filterDevices(ociSpec, mounts) | ||
| if unresolved != nil { | ||
| fmt.Printf("unresolvable CDI mount %s", strings.Join(mounts, ", ")) |
There was a problem hiding this comment.
We should not just print to STDOUT here.
pkg/parser/parser.go
Outdated
| @@ -184,7 +184,7 @@ func ValidateDeviceName(name string) error { | |||
| for _, c := range string(name[1 : len(name)-1]) { | |||
| switch { | |||
| case IsAlphaNumeric(c): | |||
| case c == '_' || c == '-' || c == '.' || c == ':': | |||
| case c == '_' || c == '-' || c == '.' || c == ':' || c == '+' || c == '=': | |||
There was a problem hiding this comment.
Are we concerned with the fact that allowing an = could confict with the vendor/class=name pattern that we have for a fully-qualified CDI device name?
In addition, this was promted by the fact that we want to abuse the device name to pass base64-encoded data to something. These characters don't represent the set of valid base64 characters which includes /.
Note that this would also require a spec version bump.
There was a problem hiding this comment.
Fixed
IMO, the ParseDevice will ensure that deviceName will be split at the first =.
But in my case, the = is padded at the tail of the encoded hostpath.
…t volume Try to use CDI to 'patch up' the Container Mount for kata/runtime-rs using direct volume in K8S/CSI scenario. related issue and PR as below: CDI issue: cncf-tags/container-device-interface#162 CDI Mount PR: cncf-tags/container-device-interface#169 Fixes: containerd#9203 Signed-off-by: alex.lyn <alex.lyn@antgroup.com> Signed-off-by: alex.lyn <alex.lyn@antgroup.com>
…t volume Try to use CDI to 'patch up' the Container Mount for kata/runtime-rs using direct volume in K8S/CSI scenario. related issue and PR as below: CDI issue: cncf-tags/container-device-interface#162 CDI Mount PR: cncf-tags/container-device-interface#169 Fixes: containerd#9203 Signed-off-by: alex.lyn <alex.lyn@antgroup.com> Signed-off-by: alex.lyn <alex.lyn@antgroup.com>
…t volume Try to use CDI to 'patch up' the Container Mount for kata/runtime-rs using direct volume in K8S/CSI scenario. related issue and PR as below: CDI issue: cncf-tags/container-device-interface#162 CDI Mount PR: cncf-tags/container-device-interface#169 Fixes: containerd#9203 Signed-off-by: alex.lyn <alex.lyn@antgroup.com> Signed-off-by: alex.lyn <alex.lyn@antgroup.com>
Add support Edit Container Mount for kata/runtime-rs using direct volume in K8S/CSI scenario. The added vendor/class: "kata.direct.volume/direct-volume" Fixes: cncf-tags#162 Signed-off-by: alex.lyn <alex.lyn@antgroup.com>
…t volume Try to use CDI to 'patch up' the Container Mount for kata/runtime-rs using direct volume in K8S/CSI scenario. related issue and PR as below: CDI issue: cncf-tags/container-device-interface#162 CDI Mount PR: cncf-tags/container-device-interface#169 Fixes: containerd#9203 Signed-off-by: alex.lyn <alex.lyn@antgroup.com>
|
/retest |
|
Rebased |
|
This PR is stale because it has been open 90 days with no activity. This PR will be closed in 30 days unless new comments are made or the stale label is removed. |
Add support Edit Container Mount for kata/runtime-rs using direct volume in K8S/CSI scenario.
Fixes: #162