feat(disks, images): allow names up to the Kubernetes name limit#2548
Open
fl64 wants to merge 8 commits into
Open
feat(disks, images): allow names up to the Kubernetes name limit#2548fl64 wants to merge 8 commits into
fl64 wants to merge 8 commits into
Conversation
Introduce shortenDiskName in kvbuilder: names within the per-device budget that are valid DNS-1123 labels pass through byte-identically (existing volumes are never renamed), while longer or non-label names are deterministically shortened to a readable prefix plus an FNV-1a 64-bit hash of the full name, keeping the derived KubeVirt volume/disk name a valid label within 63 chars. This is the groundwork for allowing resource names up to the Kubernetes limit: behaviour is unchanged for every name the current validation already accepts. Add a Ginkgo suite bootstrap to the kvbuilder package (it had none, so the existing specs did not run) and golden tests for the new derivation. Signed-off-by: Pavel Tishkov <pavel.tishkov@flant.com>
…x-strip Derived volume/disk names will be shortened/hashed for long resource names, so recovering the user resource from a volume name by stripping the vd-/vi-/cvi- prefix no longer works. Introduce kvbuilder.VolumeNameResolver, seeded with the resources a VM references (spec + VMBDA), which matches by forward-generating each candidate's volume name and falls back to prefix-strip for legacy names. - vm block-device status and hotplug handler now resolve via the seeded resolver; - VMBDA and VirtualDiskSnapshot KVVMI watchers forward-map candidates instead of reversing the volume name (the vdsnapshot watcher dropped a manual vd- strip); - extract GenerateVMBDADiskName and reuse it in VMBDA deletion. Behaviour is unchanged for existing short names. Signed-off-by: Pavel Tishkov <pavel.tishkov@flant.com>
The internal disk name is derived from the resource name; SetDisk replaces an existing disk/volume by that name, so if two distinct block devices ever derived the same internal name one would silently overwrite the other (dropping a disk / mounting the wrong PVC). The derivation uses a 64-bit hash so a collision is astronomically unlikely, but detect it at build time and fail with a clear, user-facing message instead of corrupting the VM. The error names the two conflicting resources in full and asks to recreate one with a different name (resource names are immutable, so renaming is not an option). Signed-off-by: Pavel Tishkov <pavel.tishkov@flant.com>
The tight VirtualDisk/VirtualImage/ClusterVirtualImage name limits (60/49/48) existed only because the name was used verbatim as the KubeVirt volume/disk name (a DNS-1123 label). That name is now derived and shortened independently, so the user-facing name may use the full Kubernetes DNS subdomain length (253). - raise the create limits to 253 (VirtualMachine stays 63: its name still flows into KubeVirt pod names and label values, to be addressed separately); - drop the per-reference length check in the VM block-device validator: it only duplicated limits the referenced resource is already bound by. Signed-off-by: Pavel Tishkov <pavel.tishkov@flant.com>
The 60/49/48-character limits no longer apply: these names may now use the full Kubernetes name length. Update the type doc comments and regenerate CRDs (and the Russian docs). Signed-off-by: Pavel Tishkov <pavel.tishkov@flant.com>
fl64
requested review from
Isteb4k,
goganat,
prismagod and
yaroslavborbat
as code owners
Reuse the existing builder-based e2e tests to validate end-to-end that VirtualDisk names up to the Kubernetes name length work: disk hotplug attach/detach, snapshotting a running VM's disk, and live-migration via eviction now use disk names longer than the former 60-character limit. VirtualMachine names stay short, as that limit is unchanged. Signed-off-by: Pavel Tishkov <pavel.tishkov@flant.com>
The hash suffix length is taken from the generated value directly, so the constant was unused and tripped the linter. Signed-off-by: Pavel Tishkov <pavel.tishkov@flant.com>
…specific name rules The >253 length check for VirtualDisk/VirtualImage/ClusterVirtualImage only duplicated the Kubernetes metadata.name limit (the apiserver rejects >253 first), so it is removed along with the now-unused constants; restore falls back to the 253 default for these kinds. VirtualMachine keeps its DVP-specific 63 limit. Add unit tests for the rules DVP actually owns (Kubernetes does not enforce them): VirtualMachine name length (63 ok / 64 rejected) and the dot ban for VirtualDisk. Signed-off-by: Pavel Tishkov <pavel.tishkov@flant.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
VirtualDisk, VirtualImage and ClusterVirtualImage names had to stay short — 60, 49 and 48 characters — even though Kubernetes allows up to 253. The reason was internal: the resource name was used verbatim as the underlying disk/volume identifier, which must be a much shorter DNS-1123 label, so the user-facing name inherited that tight limit.
The underlying identifier is now derived and shortened independently of the user-facing name, so VirtualDisk/VirtualImage/ClusterVirtualImage names may use the full Kubernetes name length. Existing resources are unaffected: any name that already fits keeps the exact same underlying identifier, so running virtual machines are not disrupted and no migration is required. If two resources ever derived the same identifier, the conflict is reported explicitly instead of silently overwriting a disk.
VirtualMachine names stay limited to 63 characters: a VM name still flows into pod names and label values, which is handled separately.
End-to-end coverage is added by reusing the existing lifecycle suites (disk hotplug attach/detach, VirtualDisk snapshot of a running VM, live-migration via eviction) with disk names longer than the former limit.
Why do we need it, and what problem does it solve?
Operators importing or migrating workloads from other systems regularly hit the tight, inconsistent name limits on disks and images and have to rename resources, which breaks tooling and automation that rely on the original names. After this change disks and images accept the standard Kubernetes name length, while existing virtual machines keep working unchanged.
What is the expected result?
Checklist
Changelog entries