Kubevirt: Sanitize volume name when copying to container name

[gageorsburn]

Is this a BUG REPORT or FEATURE REQUEST?:
/kind bug

What happened:
The volume name gets copied over into the pod name. If the volume name isn't lowercase the pod will fail to schedule.

What you expected to happen:
KubeVirt to automatically strings.ToLower on the volume name so that the pod gets scheduled even if I have uppercase characters in volume name.

How to reproduce it (as minimally and precisely as possible):
Create a vmi/vm with an uppercase letter in volume name.

Anything else we need to know?:

Environment:

• KubeVirt version (use virtctl version):
• Kubernetes version (use kubectl version):
• VM or VMI specifications:
• Cloud provider or hardware configuration:
• OS (e.g. from /etc/os-release):
• Kernel (e.g. uname -a):
• Install tools:
• Others:

[rmohr]

I think the issue was that the volume name for containerDisk is mapped to the container name of the container disk, right?

I think it is an error that we pass through the volume name here. Instead we should rely on the following:

• The disk order is fixed
• Based on that we can use an internal naming schema and still identify the container, if needed.
• Right now we only need to add them to the pods, they don't need to be referenced again afterwards.

[kubevirt-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

/lifecycle stale

[rmohr]

/remove-lifecycle stale

[gageorsburn]

@rmohr Is this something that still needs to be implemented? I can pick up the implementation if needed.

[rmohr]

@rmohr Is this something that still needs to be implemented? I can pick up the implementation if needed.

@gageorsburn sounds great!

[rmohr]

It still applies that the container name has no meaning for kubevirt.

[SchSeba]

Can we just add a validating webhook to block this??

I think the volume name should have the same restriction as the pod name.

[rmohr]

I think the volume name should have the same restriction as the pod name.

You are right. The issue is that the volume name check is not strict enough ...

[gageorsburn]

Assuming the disk section of the vmi admission hook in virt-api would be the right place to add naming validation for volumes?

https://github.com/kubevirt/kubevirt/blob/master/pkg/virt-api/webhooks/validating-webhook/admitters/vmi-create-admitter.go#L484

[SchSeba]

Hi @gageorsburn this is the right file but you should validate both volumes and disks. just like on pods you have volumes and mounts