Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(kubevirt): migration disk serial #690

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions Taskfile.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -216,3 +216,36 @@ tasks:
}'
kubectl -n d8-virtualization port-forward pod/<virt-handler-pod> 2345:2345
EOF

dlv:virt-api:build:
desc: "Build image virt-api with dlv"
cmd: docker build -f ./images/virt-api/debug/dlv.Dockerfile -t "{{ .DLV_IMAGE }}" --platform linux/amd64 .

dlv:virt-api:build-push:
desc: "Build and Push image virt-api with dlv"
cmds:
- task: dlv:virt-api:build
- docker push "{{ .DLV_IMAGE }}"
- task: dlv:virt-api:print

dlv:virt-api:print:
desc: "Print commands for debug"
env:
IMAGE: "{{ .DLV_IMAGE }}"
cmd: |
cat <<EOF
kubectl -n d8-virtualization patch deploy virt-api --type='strategic' -p '{
"spec": {
"template": {
"spec": {
"containers": [ {
"name": "virt-api",
"image": "${IMAGE}",
"ports": [ { "containerPort": 2345, "name": "dlv" } ]
}]
}
}
}
}'
kubectl -n d8-virtualization port-forward deploy/vit-api 2345:2345
EOF
31 changes: 31 additions & 0 deletions images/virt-api/debug/dlv.Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
FROM golang:1.22.7 AS builder

ENV VERSION="1.3.1"
ENV GOVERSION="1.22.7"

RUN go install github.com/go-delve/delve/cmd/dlv@latest

RUN git clone --depth 1 --branch v$VERSION https://github.com/kubevirt/kubevirt.git /kubevirt
COPY ./images/virt-artifact/patches /patches
WORKDIR /kubevirt
RUN for p in /patches/*.patch ; do git apply --ignore-space-change --ignore-whitespace ${p} && echo OK || (echo FAIL ; exit 1) ; done

RUN go mod edit -go=$GOVERSION && \
go mod download

RUN go mod vendor

ENV GO111MODULE=on
ENV GOOS=linux
ENV CGO_ENABLED=0
ENV GOARCH=amd64

RUN go build -o /kubevirt-binaries/virt-api ./cmd/virt-api/

FROM busybox

WORKDIR /app
COPY --from=builder /kubevirt-binaries/virt-api /app/virt-api
COPY --from=builder /go/bin/dlv /app/dlv
USER 65532:65532
ENTRYPOINT ["./dlv", "--listen=:2345", "--headless=true", "--continue", "--log=true", "--log-output=debugger,debuglineerr,gdbwire,lldbout,rpc", "--accept-multiclient", "--api-version=2", "exec", "/app/virt-api", "--"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
diff --git a/pkg/virt-api/webhooks/utils.go b/pkg/virt-api/webhooks/utils.go
index e6ee54431f..5c68ce992d 100644
--- a/pkg/virt-api/webhooks/utils.go
+++ b/pkg/virt-api/webhooks/utils.go
@@ -100,7 +100,9 @@ func IsKubeVirtServiceAccount(serviceAccount string) bool {

return IsComponentServiceAccount(serviceAccount, ns, components.ApiServiceAccountName) ||
IsComponentServiceAccount(serviceAccount, ns, components.HandlerServiceAccountName) ||
- IsComponentServiceAccount(serviceAccount, ns, components.ControllerServiceAccountName)
+ IsComponentServiceAccount(serviceAccount, ns, components.ControllerServiceAccountName) ||
+ IsComponentServiceAccount(serviceAccount, ns, components.VirtualizationController) ||
+ IsComponentServiceAccount(serviceAccount, ns, components.VirtualizationApi)
}

func IsARM64(vmiSpec *v1.VirtualMachineInstanceSpec) bool {
diff --git a/pkg/virt-operator/resource/generate/components/serviceaccountnames.go b/pkg/virt-operator/resource/generate/components/serviceaccountnames.go
index 9aca3b3bd2..4ed51d98b5 100644
--- a/pkg/virt-operator/resource/generate/components/serviceaccountnames.go
+++ b/pkg/virt-operator/resource/generate/components/serviceaccountnames.go
@@ -6,4 +6,7 @@ const (
ExportProxyServiceAccountName = "kubevirt-internal-virtualization-exportproxy"
HandlerServiceAccountName = "kubevirt-internal-virtualization-handler"
OperatorServiceAccountName = "kubevirt-operator"
+
+ VirtualizationController = "virtualization-controller"
+ VirtualizationApi = "virtualization-api"
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
diff --git a/pkg/virt-api/webhooks/validating-webhook/admitters/util.go b/pkg/virt-api/webhooks/validating-webhook/admitters/util.go
new file mode 100644
index 0000000000..6d0699b12d
--- /dev/null
+++ b/pkg/virt-api/webhooks/validating-webhook/admitters/util.go
@@ -0,0 +1,19 @@
+package admitters
+
+import (
+ "k8s.io/apimachinery/pkg/api/equality"
+ v1 "kubevirt.io/api/core/v1"
+)
+
+func equalDiskIgnoreSerial(newDisk, oldDisk v1.Disk) bool {
+ return equality.Semantic.DeepEqual(newDisk.Name, oldDisk.Name) &&
+ equality.Semantic.DeepEqual(newDisk.DiskDevice, oldDisk.DiskDevice) &&
+ equality.Semantic.DeepEqual(newDisk.BootOrder, oldDisk.BootOrder) &&
+ equality.Semantic.DeepEqual(newDisk.DedicatedIOThread, oldDisk.DedicatedIOThread) &&
+ equality.Semantic.DeepEqual(newDisk.Cache, oldDisk.Cache) &&
+ equality.Semantic.DeepEqual(newDisk.IO, oldDisk.IO) &&
+ equality.Semantic.DeepEqual(newDisk.Tag, oldDisk.Tag) &&
+ equality.Semantic.DeepEqual(newDisk.BlockSize, oldDisk.BlockSize) &&
+ equality.Semantic.DeepEqual(newDisk.Shareable, oldDisk.Shareable) &&
+ equality.Semantic.DeepEqual(newDisk.ErrorPolicy, oldDisk.ErrorPolicy)
+}
diff --git a/pkg/virt-api/webhooks/validating-webhook/admitters/vmi-update-admitter.go b/pkg/virt-api/webhooks/validating-webhook/admitters/vmi-update-admitter.go
index b984ff4262..8201d9375b 100644
--- a/pkg/virt-api/webhooks/validating-webhook/admitters/vmi-update-admitter.go
+++ b/pkg/virt-api/webhooks/validating-webhook/admitters/vmi-update-admitter.go
@@ -189,7 +189,8 @@ func verifyHotplugVolumes(newHotplugVolumeMap, oldHotplugVolumeMap map[string]v1
},
})
}
- if !equality.Semantic.DeepEqual(newDisks[k], oldDisks[k]) {
+
+ if !equalDiskIgnoreSerial(newDisks[k], oldDisks[k]) {
return webhookutils.ToAdmissionResponse([]metav1.StatusCause{
{
Type: metav1.CauseTypeFieldValueInvalid,
@@ -292,7 +293,8 @@ func verifyPermanentVolumes(newPermanentVolumeMap, oldPermanentVolumeMap map[str
},
})
}
- if !equality.Semantic.DeepEqual(newDisks[k], oldDisks[k]) {
+
+ if !equalDiskIgnoreSerial(newDisks[k], oldDisks[k]) {
return webhookutils.ToAdmissionResponse([]metav1.StatusCause{
{
Type: metav1.CauseTypeFieldValueInvalid,
9 changes: 9 additions & 0 deletions images/virt-artifact/patches/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -202,3 +202,12 @@ Network traffic will be directed to the pod with the higher priority. Absence of
while the source pod retains the default behavior (no label).

Thus, packets are delivered as expected: initially only to the source pod during migration, and after migration completes, only to the target pod.

#### `034-allow-update-kvvmi-for-virtualization-sas.patch`

By default, the KVVMI spec can update only KubeVirt service accounts. This patch adds our virtualization accounts to the allowed list.
(`virtualization-controller`, `virtualization-api`)

#### `035-allow-change-serial-on-kvvmi.patch`

By default, the disk specification is immutable, but for backward compatibility, we need to allow modifying the serial.
30 changes: 15 additions & 15 deletions images/virt-artifact/werf.inc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,15 @@ shell:

- echo "Create .version file"
- echo "v{{ $version }}-dirty" > /kubevirt-config-files/.version

- echo "Create group file"
- |
GROUP_FILE=/kubevirt-config-files/group
echo "qemu:x:107:" > $GROUP_FILE
echo "root:x:0:" >> $GROUP_FILE
echo "nonroot-user:x:1001:" >> $GROUP_FILE
chmod 0644 $GROUP_FILE

- echo "Create passwd file"
- |
PASSWD_FILE=/kubevirt-config-files/passwd
Expand All @@ -93,42 +93,42 @@ shell:

- echo ============== Build virt-handler =====================
- CGO_ENABLED=1 go build -o /kubevirt-binaries/virt-handler ./cmd/virt-handler/

- echo ============== Build virt-launcher-monitor ============
- go build -o /kubevirt-binaries/virt-launcher-monitor ./cmd/virt-launcher-monitor/

- echo ============== Build virt-tail ========================
- go build -o /kubevirt-binaries/virt-tail ./cmd/virt-tail/

- echo ============== Build virt-freezer =====================
- go build -o /kubevirt-binaries/virt-freezer ./cmd/virt-freezer/

- echo ============== Build virt-probe =======================
- go build -o /kubevirt-binaries/virt-probe ./cmd/virt-probe/

- echo ============== Build virt-api =========================
- go build -o /kubevirt-binaries/virt-api ./cmd/virt-api/

- echo ============== Build virt-chroot ======================
- go build -o /kubevirt-binaries/virt-chroot ./cmd/virt-chroot/

- echo ============== Build virt-exportproxy =================
- go build -o /kubevirt-binaries/virt-exportproxy ./cmd/virt-exportproxy/

- echo ============== Build virt-exportserver ================
- go build -o /kubevirt-binaries/virt-exportserver ./cmd/virt-exportserver/

- echo ============== Build virt-controller ==================
- go build -o /kubevirt-binaries/virt-controller ./cmd/virt-controller/

- echo ============== Build virt-operator ====================
- go build -o /kubevirt-binaries/virt-operator ./cmd/virt-operator/

- echo ============== Build csv-generator ====================
- go build -o /kubevirt-binaries/csv-generator ./tools/csv-generator

- echo ============== Build sidecars =========================
- go build -o /kubevirt-binaries/sidecars ./cmd/sidecars/

- echo ============== Build virtctl ==========================
- go build -o /kubevirt-binaries/virtctl ./cmd/virtctl/
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
virtv1 "kubevirt.io/api/core/v1"
cdiv1beta1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/config"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/manager/signals"
Expand All @@ -54,6 +55,7 @@ import (
"github.com/deckhouse/virtualization-controller/pkg/controller/vmrestore"
"github.com/deckhouse/virtualization-controller/pkg/controller/vmsnapshot"
"github.com/deckhouse/virtualization-controller/pkg/logger"
"github.com/deckhouse/virtualization-controller/pkg/migration"
"github.com/deckhouse/virtualization-controller/pkg/version"
"github.com/deckhouse/virtualization/api/client/kubeclient"
virtv2alpha1 "github.com/deckhouse/virtualization/api/core/v1alpha2"
Expand Down Expand Up @@ -223,6 +225,18 @@ func main() {
// Setup context to gracefully handle termination.
ctx := signals.SetupSignalHandler()

onlyMigrationClient, err := client.New(cfg, client.Options{Scheme: scheme})
if err != nil {
log.Error(err.Error())
os.Exit(1)
}
mCtrl, err := migration.NewController(onlyMigrationClient, log)
if err != nil {
log.Error(err.Error())
os.Exit(1)
}
mCtrl.Run(ctx)

if err = indexer.IndexALL(ctx, mgr); err != nil {
log.Error(err.Error())
os.Exit(1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"k8s.io/apiserver/pkg/registry/rest"
virtv1 "kubevirt.io/api/core/v1"

"github.com/deckhouse/virtualization-controller/pkg/controller/kvbuilder"
"github.com/deckhouse/virtualization-controller/pkg/tls/certmanager"
virtlisters "github.com/deckhouse/virtualization/api/client/generated/listers/core/v1alpha2"
"github.com/deckhouse/virtualization/api/subresources"
Expand Down Expand Up @@ -119,7 +120,7 @@ func (r AddVolumeREST) genMutateRequestHook(opts *subresources.VirtualMachineAdd
Disk: &virtv1.Disk{
Name: opts.Name,
DiskDevice: dd,
Serial: opts.Name,
Serial: kvbuilder.GenerateSerial(opts.Name),
},
}
switch opts.VolumeKind {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package kvbuilder

import (
"crypto/md5"
"encoding/hex"
"fmt"
"strings"

Expand Down Expand Up @@ -62,6 +64,13 @@ func GetOriginalDiskName(prefixedName string) (string, bool) {
return prefixedName, false
}

func GenerateSerial(input string) string {
hasher := md5.New()
hasher.Write([]byte(input))
hashInBytes := hasher.Sum(nil)
return hex.EncodeToString(hashInBytes)
}

type HotPlugDeviceSettings struct {
VolumeName string
PVCName string
Expand Down Expand Up @@ -139,7 +148,7 @@ func ApplyVirtualMachineSpec(
if err := kvvm.SetDisk(name, SetDiskOptions{
PersistentVolumeClaim: pointer.GetPointer(vi.Status.Target.PersistentVolumeClaim),
IsEphemeral: true,
Serial: name,
Serial: GenerateSerial(name),
BootOrder: bootOrder,
}); err != nil {
return err
Expand All @@ -148,7 +157,7 @@ func ApplyVirtualMachineSpec(
if err := kvvm.SetDisk(name, SetDiskOptions{
ContainerDisk: pointer.GetPointer(vi.Status.Target.RegistryURL),
IsCdrom: imageformat.IsISO(vi.Status.Format),
Serial: name,
Serial: GenerateSerial(name),
BootOrder: bootOrder,
}); err != nil {
return err
Expand All @@ -167,7 +176,7 @@ func ApplyVirtualMachineSpec(
if err := kvvm.SetDisk(name, SetDiskOptions{
ContainerDisk: pointer.GetPointer(cvi.Status.Target.RegistryURL),
IsCdrom: imageformat.IsISO(cvi.Status.Format),
Serial: name,
Serial: GenerateSerial(name),
BootOrder: bootOrder,
}); err != nil {
return err
Expand All @@ -186,7 +195,7 @@ func ApplyVirtualMachineSpec(
name := GenerateVMDDiskName(bd.Name)
if err := kvvm.SetDisk(name, SetDiskOptions{
PersistentVolumeClaim: pointer.GetPointer(vd.Status.Target.PersistentVolumeClaim),
Serial: name,
Serial: GenerateSerial(name),
BootOrder: bootOrder,
}); err != nil {
return err
Expand Down
13 changes: 12 additions & 1 deletion images/virtualization-artifact/pkg/logger/attrs.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ limitations under the License.

package logger

import "log/slog"
import (
"encoding/json"
"log/slog"
)

const (
errAttr = "err"
Expand Down Expand Up @@ -55,3 +58,11 @@ func SlogController(controller string) slog.Attr {
func SlogCollector(collector string) slog.Attr {
return slog.String(collectorAttr, collector)
}

func SlogTryJson(key string, i interface{}) slog.Attr {
bytes, err := json.Marshal(i)
if err == nil {
return slog.String(key, string(bytes))
}
return slog.Any(key, i)
}
9 changes: 9 additions & 0 deletions images/virtualization-artifact/pkg/migration/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Migrations

## qemu-max-length-36

Fix disk serial handling for successful migration:

Previously, we set the disk serial to match the disk name, relying on Kubernetes to ensure the uniqueness of names. However, after this [commit](https://github.com/qemu/qemu/commit/75997e182b695f2e3f0a2d649734952af5caf3ee) on QEMU, we can no longer do this, as QEMU no longer truncates the serial to 36 characters and now returns an error. We must handle the truncation ourselves. However, truncation does not guarantee uniqueness, so we switched to generating an MD5 hash of the disk name. The MD5 hash is easy to reproduce and fits the required 32-character length.

To ensure a successful migration, existing `kvvm` and `kvvmi` resources need to be patched to reflect the new serial format.
Loading
Loading