Merge pull request kubevirt#10600 from jean-edouard/givetimetodapi

virt-handler: check for dedication migration interface instead of relying on an annotation

cmd/virt-handler/virt-handler.go

@@ -126,9 +126,6 @@ const (
 	defaultClientKeyFilePath  = "/etc/virt-handler/clientcertificates/tls.key"
 	defaultTlsCertFilePath    = "/etc/virt-handler/servercertificates/tls.crt"
 	defaultTlsKeyFilePath     = "/etc/virt-handler/servercertificates/tls.key"
-
-	// Default network-status downward API file path
-	defaultNetworkStatusFilePath = "/etc/podinfo/network-status"
 )
 
 type virtHandlerApp struct {
@@ -352,10 +349,9 @@ func (app *virtHandlerApp) Run() {
 	}
 
 	migrationIpAddress := app.PodIpAddress
-	migrationIpAddress, err = virthandler.FindMigrationIP(defaultNetworkStatusFilePath, migrationIpAddress)
+	migrationIpAddress, err = virthandler.FindMigrationIP(migrationIpAddress)
 	if err != nil {
-		log.Log.Reason(err)
-		return
+		panic(err)
 	}
 
 	downwardMetricsManager := dmetricsmanager.NewDownwardMetricsManager(app.HostOverride)

go.mod

@@ -67,7 +67,6 @@ require (
 	golang.org/x/tools v0.13.0
 	google.golang.org/grpc v1.56.3
 	gopkg.in/cheggaaa/pb.v1 v1.0.28
-	gopkg.in/yaml.v2 v2.4.0
 	k8s.io/api v0.27.1
 	k8s.io/apiextensions-apiserver v0.26.4
 	k8s.io/apimachinery v0.27.1
@@ -153,6 +152,7 @@ require (
 	google.golang.org/protobuf v1.30.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect
+	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 	mvdan.cc/editorconfig v0.1.1-0.20200121172147-e40951bde157 // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect

pkg/virt-handler/BUILD.bazel

@@ -58,7 +58,6 @@ go_library(
         "//vendor/github.com/mitchellh/go-ps:go_default_library",
         "//vendor/github.com/opencontainers/runc/libcontainer/cgroups:go_default_library",
         "//vendor/golang.org/x/sys/unix:go_default_library",
-        "//vendor/gopkg.in/yaml.v2:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",

pkg/virt-handler/migration.go

@@ -21,28 +21,31 @@ package virthandler
 
 import (
 	"fmt"
-	"os"
+	"net"
 
-	"gopkg.in/yaml.v2"
+	v1 "kubevirt.io/api/core/v1"
 )
 
-// FindMigrationIP looks for dedicated migration network migration0 using the downward API and, if found, sets migration IP to it
-func FindMigrationIP(networkStatusPath string, migrationIp string) (string, error) {
-	var networkStatus []NetworkStatus
-
-	dat, err := os.ReadFile(networkStatusPath)
+// FindMigrationIP looks for dedicated migration network migration0. If found, sets migration IP to it
+func FindMigrationIP(migrationIp string) (string, error) {
+	ief, err := net.InterfaceByName(v1.MigrationInterfaceName)
 	if err != nil {
-		return "", fmt.Errorf("failed to read network status from downwards API")
+		return migrationIp, nil
 	}
-	err = yaml.Unmarshal(dat, &networkStatus)
-	if err != nil {
-		return "", fmt.Errorf("failed to un-marshall network status")
+	addrs, err := ief.Addrs()
+	if err != nil { // get addresses
+		return migrationIp, fmt.Errorf("%s present but doesn't have an IP", v1.MigrationInterfaceName)
 	}
-	for _, ns := range networkStatus {
-		if ns.Interface == "migration0" && len(ns.Ips) > 0 {
-			migrationIp = ns.Ips[0]
+	for _, addr := range addrs {
+		if !addr.(*net.IPNet).IP.IsGlobalUnicast() {
+			// skip local/multicast IPs
+			continue
+		}
+		ip := addr.(*net.IPNet).IP.To16()
+		if ip != nil {
+			return ip.String(), nil
 		}
 	}
 
-	return migrationIp, nil
+	return migrationIp, fmt.Errorf("no IP found on %s", v1.MigrationInterfaceName)
 }

pkg/virt-handler/migration_test.go

@@ -20,70 +20,20 @@
 package virthandler
 
 import (
-	"os"
-
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
 
 const (
-	originalIP  = "1.1.1.1"
-	migrationIP = "2.2.2.2"
-
-	mainNetwork = `{
-    "name": "k8s-pod-network",
-    "ips": [
-        "` + originalIP + `"
-    ],
-    "default": true,
-    "dns": {}
-}`
-
-	migrationNetwork = `{
-    "name": "migration-bridge",
-    "interface": "migration0",
-    "ips": [
-        "` + migrationIP + `"
-    ],
-    "mac": "ae:33:70:a7:3a:8c",
-    "dns": {}
-}`
+	originalIP = "1.1.1.1"
 )
 
 var _ = Describe("virt-handler", func() {
 	Context("findMigrationIp", func() {
-		It("Should error on missing file", func() {
-			_, err := FindMigrationIP("/not-a-real-file", originalIP)
-			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(ContainSubstring("failed to read network status from downwards API"))
-		})
-		It("Should handle the empty file case", func() {
-			file, err := os.CreateTemp("", "test")
-			Expect(err).ToNot(HaveOccurred())
-			defer os.Remove(file.Name())
-			newIP, err := FindMigrationIP(file.Name(), originalIP)
-			Expect(err).ToNot(HaveOccurred())
-			Expect(newIP).To(Equal(originalIP))
-		})
-		It("Should return the original IP if migration0 doesn't exist", func() {
-			file, err := os.CreateTemp("", "test")
-			Expect(err).ToNot(HaveOccurred())
-			defer os.Remove(file.Name())
-			err = os.WriteFile(file.Name(), []byte(`[`+mainNetwork+`]`), 0644)
-			Expect(err).ToNot(HaveOccurred())
-			newIP, err := FindMigrationIP(file.Name(), originalIP)
-			Expect(err).ToNot(HaveOccurred())
-			Expect(newIP).To(Equal(originalIP))
-		})
-		It("Should return the migration IP if migration0 exists", func() {
-			file, err := os.CreateTemp("", "test")
-			Expect(err).ToNot(HaveOccurred())
-			defer os.Remove(file.Name())
-			err = os.WriteFile(file.Name(), []byte(`[`+mainNetwork+`,`+migrationNetwork+`]`), 0644)
-			Expect(err).ToNot(HaveOccurred())
-			newIP, err := FindMigrationIP(file.Name(), originalIP)
-			Expect(err).ToNot(HaveOccurred())
-			Expect(newIP).To(Equal(migrationIP))
+		It("Should return the IP passed to it when no migration0 interface exists", func() {
+			newIp, err := FindMigrationIP(originalIP)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(newIp).To(Equal(originalIP))
 		})
 	})
 })

pkg/virt-handler/vm.go

@@ -720,12 +720,6 @@ func (d *VirtualMachineController) migrationSourceUpdateVMIStatus(origVMI *v1.Vi
 	return nil
 }
 
-type NetworkStatus struct {
-	Name      string   `yaml:"name"`
-	Ips       []string `yaml:"ips"`
-	Interface string   `yaml:"interface"`
-}
-
 func domainIsActiveOnTarget(domain *api.Domain) bool {
 
 	if domain == nil {

pkg/virt-operator/resource/generate/components/daemonsets.go

@@ -67,7 +67,7 @@ func NewHandlerDaemonSet(namespace, repository, imagePrefix, version, launcherVe
 			podTemplateSpec.ObjectMeta.Annotations = make(map[string]string)
 		}
 		// Join the pod to the migration network and name the corresponding interface "migration0"
-		podTemplateSpec.ObjectMeta.Annotations["k8s.v1.cni.cncf.io/networks"] = *migrationNetwork + "@migration0"
+		podTemplateSpec.ObjectMeta.Annotations["k8s.v1.cni.cncf.io/networks"] = *migrationNetwork + "@" + virtv1.MigrationInterfaceName
 	}
 
 	daemonset := &appsv1.DaemonSet{
@@ -294,6 +294,10 @@ func NewHandlerDaemonSet(namespace, repository, imagePrefix, version, launcherVe
 	}
 
 	// Use the downward API to access the network status annotations
+	// TODO: This is not used anymore, but can't be removed because of https://github.com/kubevirt/kubevirt/issues/10632
+	//   Since CR-based updates use the wrong install strategy, removing this volume and downgrading via CR will try to
+	//   run the previous version of virt-handler without the volume, which will fail and CrashLoop.
+	//   Please remove the volume once the above issue is fixed.
 	container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{
 		Name:      "podinfo",
 		MountPath: "/etc/podinfo",

staging/src/kubevirt.io/api/core/v1/types.go

@@ -994,6 +994,9 @@ const (
 	// AutoMemoryLimitsRatioLabel allows to use a custom ratio for auto memory limits calculation.
 	// Must be a float >= 1.
 	AutoMemoryLimitsRatioLabel string = "alpha.kubevirt.io/auto-memory-limits-ratio"
+
+	// MigrationInterfaceName is an arbitrary name used in virt-handler to connect it to a dedicated migration network
+	MigrationInterfaceName string = "migration0"
 )
 
 func NewVMI(name string, uid types.UID) *VirtualMachineInstance {