Merge pull request #98 from openshift-cherrypick-robot/cherry-pick-85…

…-to-release-4.7

[release-4.7] Bug 1939358: extract node machine ipaddress from the engine instead using DNS .

go.mod

@@ -4,6 +4,8 @@ go 1.15
 
 require (
 	github.com/go-logr/logr v0.3.0
+	github.com/go-logr/zapr v0.2.0 // indirect
+	github.com/openshift/client-go v0.0.0-20201214125552-e615e336eb49
 	github.com/openshift/machine-api-operator v0.2.1-0.20210104142355-8e6ae0acdfcf
 	github.com/ovirt/go-ovirt v0.0.0-20210112072624-e4d3b104de71
 	github.com/pkg/errors v0.9.1

pkg/cloud/ovirt/clients/machineservice.go

@@ -9,6 +9,7 @@ import (
 	"context"
 	"fmt"
 	"math"
+	"regexp"
 	"time"
 
 	"github.com/pkg/errors"
@@ -321,3 +322,48 @@ func (is *InstanceService) handleNics(vmService *ovirtsdk.VmService, spec *ovirt
 	}
 	return nil
 }
+
+//Find virtual machine IP Address by ID
+func  (is *InstanceService) FindVirtualMachineIP(id string,excludeAddr map[string]int) (string, error) {
+
+	vmService := is.Connection.SystemService().VmsService().VmService(id)
+
+	// Get the guest reported devices
+	reportedDeviceResp, err := vmService.ReportedDevicesService().List().Send()
+	if err != nil {
+		return "", fmt.Errorf("failed to get reported devices list, reason: %v", err)
+	}
+	reportedDeviceSlice, _ := reportedDeviceResp.ReportedDevice()
+
+	if len(reportedDeviceSlice.Slice()) == 0 {
+		return "", fmt.Errorf("cannot find NICs for vmId: %s", id)
+	}
+
+	var nicRegex = regexp.MustCompile(`^(eth|en).*`)
+
+	for _, reportedDevice := range reportedDeviceSlice.Slice() {
+		nicName, _ := reportedDevice.Name()
+		if !nicRegex.MatchString(nicName) {
+			klog.Infof("ovirt vm id: %s ,  skipped nic %s , naming regex mismatch",id, nicName)
+			continue
+		}
+
+		ips, hasIps := reportedDevice.Ips()
+		if hasIps {
+			for _, ip := range ips.Slice() {
+				ipres, hasAddress := ip.Address()
+
+				if _, ok := excludeAddr[ipres]; ok {
+					klog.Infof("address %s is excluded from usable IPs", ipres)
+					continue
+				}
+
+				if hasAddress {
+					klog.Infof("ovirt vm id: %s , found usable IP %s", id, ipres)
+					return ipres,nil
+				}
+			}
+		}
+	}
+	return "", fmt.Errorf("coudlnt find usable IP address for vm id: %s", id)
+}

pkg/cloud/ovirt/machine/actuator.go

@@ -8,7 +8,7 @@ package machine
 import (
 	"context"
 	"fmt"
-	"net"
+	"k8s.io/client-go/rest"
 	"time"
 
 	machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1"
@@ -23,12 +23,14 @@ import (
 	"k8s.io/client-go/tools/record"
 	"k8s.io/klog"
 
+	osclientset "github.com/openshift/client-go/config/clientset/versioned"
 	ovirtconfigv1 "github.com/openshift/cluster-api-provider-ovirt/pkg/apis/ovirtprovider/v1beta1"
 	"github.com/openshift/cluster-api-provider-ovirt/pkg/cloud/ovirt"
 	"github.com/openshift/cluster-api-provider-ovirt/pkg/cloud/ovirt/clients"
 	ovirtsdk "github.com/ovirt/go-ovirt"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	ctrl "sigs.k8s.io/controller-runtime"
 )
 
 const (
@@ -45,9 +47,15 @@ type OvirtActuator struct {
 	machinesClient v1beta1.MachineV1beta1Interface
 	EventRecorder  record.EventRecorder
 	ovirtApi       *ovirtsdk.Connection
+	OSClient       osclientset.Interface
 }
 
+
+
 func NewActuator(params ovirt.ActuatorParams) (*OvirtActuator, error) {
+	config := ctrl.GetConfigOrDie()
+	osClient := osclientset.NewForConfigOrDie(rest.AddUserAgent(config, "cluster-api-provider-ovirt"))
+
 	return &OvirtActuator{
 		params:         params,
 		client:         params.Client,
@@ -56,10 +64,11 @@ func NewActuator(params ovirt.ActuatorParams) (*OvirtActuator, error) {
 		KubeClient:     params.KubeClient,
 		EventRecorder:  params.EventRecorder,
 		ovirtApi:       nil,
+		OSClient:       osClient,
 	}, nil
 }
 
-func (actuator *OvirtActuator) Create(_ context.Context, machine *machinev1.Machine) error {
+func (actuator *OvirtActuator) Create(ctx context.Context, machine *machinev1.Machine) error {
 	providerSpec, err := ovirtconfigv1.ProviderSpecFromRawExtension(machine.Spec.ProviderSpec.Value)
 	if err != nil {
 		return actuator.handleMachineError(machine, apierrors.InvalidMachineConfiguration(
@@ -130,7 +139,7 @@ func (actuator *OvirtActuator) Create(_ context.Context, machine *machinev1.Mach
 	}
 
 	actuator.EventRecorder.Eventf(machine, corev1.EventTypeNormal, "Created", "Updated Machine %v", machine.Name)
-	return actuator.patchMachine(machine, instance, conditionSuccess())
+	return actuator.patchMachine(ctx,machine, instance, conditionSuccess())
 }
 
 func (actuator *OvirtActuator) Exists(_ context.Context, machine *machinev1.Machine) (bool, error) {
@@ -157,7 +166,7 @@ func (actuator *OvirtActuator) Exists(_ context.Context, machine *machinev1.Mach
 	return vm != nil, err
 }
 
-func (actuator *OvirtActuator) Update(_ context.Context, machine *machinev1.Machine) error {
+func (actuator *OvirtActuator) Update(ctx context.Context, machine *machinev1.Machine) error {
 	// eager update
 	providerSpec, err := ovirtconfigv1.ProviderSpecFromRawExtension(machine.Spec.ProviderSpec.Value)
 	if err != nil {
@@ -189,7 +198,7 @@ func (actuator *OvirtActuator) Update(_ context.Context, machine *machinev1.Mach
 				"Cannot find a VM by id: %v", err))
 		}
 	}
-	return actuator.patchMachine(machine, vm, conditionSuccess())
+	return actuator.patchMachine(ctx,machine, vm, conditionSuccess())
 }
 
 func (actuator *OvirtActuator) Delete(_ context.Context, machine *machinev1.Machine) error {
@@ -245,11 +254,11 @@ func (actuator *OvirtActuator) handleMachineError(machine *machinev1.Machine, er
 	return err
 }
 
-func (actuator *OvirtActuator) patchMachine(machine *machinev1.Machine, instance *clients.Instance, condition ovirtconfigv1.OvirtMachineProviderCondition) error {
+func (actuator *OvirtActuator) patchMachine(ctx context.Context,machine *machinev1.Machine, instance *clients.Instance, condition ovirtconfigv1.OvirtMachineProviderCondition) error {
 	actuator.reconcileProviderID(machine, instance)
 	klog.V(5).Infof("Machine %s provider status %s", instance.MustName(), instance.MustStatus())
 
-	err := actuator.reconcileNetwork(machine, instance)
+	err := actuator.reconcileNetwork(ctx,machine, instance)
 	if err != nil {
 		return err
 	}
@@ -279,7 +288,21 @@ func (actuator *OvirtActuator) patchMachine(machine *machinev1.Machine, instance
 	return nil
 }
 
-func (actuator *OvirtActuator) reconcileNetwork(machine *machinev1.Machine, instance *clients.Instance) error {
+func (actuator *OvirtActuator) getClusterAddress(ctx context.Context) (map[string]int,error){
+		infra,err := actuator.OSClient.ConfigV1().Infrastructures().Get(ctx,"cluster",metav1.GetOptions{})
+		if err != nil {
+			klog.Error(err, "Failed to retrieve Cluster details")
+			return nil,err
+		}
+
+		var clusterAddr = make(map[string]int)
+		clusterAddr[ infra.Status.PlatformStatus.Ovirt.APIServerInternalIP ] = 1
+		clusterAddr[ infra.Status.PlatformStatus.Ovirt.IngressIP ] = 1
+
+		return clusterAddr,nil
+	}
+
+func (actuator *OvirtActuator) reconcileNetwork(ctx context.Context,machine *machinev1.Machine, instance *clients.Instance) error {
 	switch instance.MustStatus() {
 	// expect IP addresses only on those statuses.
 	// in those statuses we 'll try reconciling
@@ -298,21 +321,28 @@ func (actuator *OvirtActuator) reconcileNetwork(machine *machinev1.Machine, inst
 	}
 	name := instance.MustName()
 	addresses := []corev1.NodeAddress{{Address: name, Type: corev1.NodeInternalDNS}}
-	// TODO rgolan - RHCOS QEMU guest agent isn't available yet - https://bugzilla.redhat.com/show_bug.cgi?id=1764804
-	// Till we have one we must get the IPs from the worker by trying to resolve it by its name.
-	klog.V(5).Infof("using hostname %s to resolve addresses", name)
-	ips, err := net.LookupIP(name)
+	machineService, err := clients.NewInstanceServiceFromMachine(machine, actuator.ovirtApi)
+	if err != nil {
+		return err
+	}
+	vmId := instance.MustId()
+	klog.V(5).Infof("using oVirt SDK to find % IP addresses", name)
+
+	//get API and ingress addresses that will be excluded from the node address selection
+	excludeAddr, err := actuator.getClusterAddress(ctx)
+	if err != nil {
+		return err
+	}
+
+	ip, err := machineService.FindVirtualMachineIP(vmId,excludeAddr)
+
 	if err != nil {
 		// stop reconciliation till we get IP addresses - otherwise the state will be considered stable.
 		klog.Errorf("failed to lookup the VM IP %s - skip setting addresses for this machine", err)
 		return err
 	} else {
-		klog.V(5).Infof("resolved IP address %v from DNS", ips)
-		for _, ip := range ips {
-			if ip.To4() != nil {
-				addresses = append(addresses, corev1.NodeAddress{Type: corev1.NodeInternalIP, Address: ip.String()})
-			}
-		}
+		klog.V(5).Infof("received IP address %v from engine", ip)
+		addresses = append(addresses, corev1.NodeAddress{Type: corev1.NodeInternalIP, Address: ip})
 	}
 	machine.Status.Addresses = addresses
 	return nil

vendor/modules.txt

@@ -33,6 +33,7 @@ github.com/ghodss/yaml
 ## explicit
 github.com/go-logr/logr
 # github.com/go-logr/zapr v0.2.0
+## explicit
 github.com/go-logr/zapr
 # github.com/go-openapi/jsonpointer v0.19.3
 github.com/go-openapi/jsonpointer
@@ -108,6 +109,7 @@ github.com/modern-go/reflect2
 # github.com/openshift/api v0.0.0-20201216151826-78a19e96f9eb
 github.com/openshift/api/config/v1
 # github.com/openshift/client-go v0.0.0-20201214125552-e615e336eb49
+## explicit
 github.com/openshift/client-go/config/clientset/versioned
 github.com/openshift/client-go/config/clientset/versioned/scheme
 github.com/openshift/client-go/config/clientset/versioned/typed/config/v1