Report addresses in machine status

pkg/util/provider/driver/driver.go

@@ -59,6 +59,10 @@ type CreateMachineResponse struct {
 
 	// LastKnownState represents the last state of the VM during an creation/deletion error
 	LastKnownState string
+
+	// Addresses to reach the VM. Returning this field is optional, and only used if the MCM provider runs without a
+	// target cluster.
+	Addresses []corev1.NodeAddress
 }
 
 // InitializeMachineRequest encapsulates params for the VM Initialization operation (Driver.InitializeMachine).
@@ -82,6 +86,10 @@ type InitializeMachineResponse struct {
 
 	// NodeName is the name of the node-object registered to kubernetes.
 	NodeName string
+
+	// Addresses to reach the VM. Returning this field is optional, and only used if the MCM provider runs without a
+	// target cluster.
+	Addresses []corev1.NodeAddress
 }
 
 // DeleteMachineRequest is the delete request for VM deletion
@@ -123,6 +131,10 @@ type GetMachineStatusResponse struct {
 
 	// NodeName is the name of the node-object registered to kubernetes.
 	NodeName string
+
+	// Addresses to reach the VM. Returning this field is optional, and only used if the MCM provider runs without a
+	// target cluster.
+	Addresses []corev1.NodeAddress
 }
 
 // ListMachinesRequest is the request object to get a list of VMs belonging to a machineClass

pkg/util/provider/driver/fake.go

@@ -7,8 +7,10 @@ package driver
 
 import (
 	"context"
+
 	"github.com/gardener/machine-controller-manager/pkg/util/provider/machinecodes/codes"
 	"github.com/gardener/machine-controller-manager/pkg/util/provider/machinecodes/status"
+	corev1 "k8s.io/api/core/v1"
 )
 
 // VMs is the map to hold the VM data
@@ -20,17 +22,19 @@ type FakeDriver struct {
 	ProviderID     string
 	NodeName       string
 	LastKnownState string
+	Addresses      []corev1.NodeAddress
 	Err            error
 	fakeVMs        VMs
 }
 
 // NewFakeDriver returns a new fakedriver object
-func NewFakeDriver(vmExists bool, providerID, nodeName, lastKnownState string, err error, fakeVMs VMs) Driver {
+func NewFakeDriver(vmExists bool, providerID, nodeName, lastKnownState string, addresses []corev1.NodeAddress, err error, fakeVMs VMs) Driver {
 	fakeDriver := &FakeDriver{
 		VMExists:       vmExists,
 		ProviderID:     providerID,
 		NodeName:       nodeName,
 		LastKnownState: lastKnownState,
+		Addresses:      addresses,
 		Err:            err,
 		fakeVMs:        make(VMs),
 	}
@@ -54,6 +58,7 @@ func (d *FakeDriver) CreateMachine(_ context.Context, _ *CreateMachineRequest) (
 			ProviderID:     d.ProviderID,
 			NodeName:       d.NodeName,
 			LastKnownState: d.LastKnownState,
+			Addresses:      d.Addresses,
 		}, nil
 	}
 
@@ -78,6 +83,7 @@ func (d *FakeDriver) InitializeMachine(_ context.Context, _ *InitializeMachineRe
 	return &InitializeMachineResponse{
 		ProviderID: d.ProviderID,
 		NodeName:   d.NodeName,
+		Addresses:  d.Addresses,
 	}, d.Err
 }
 
@@ -99,6 +105,7 @@ func (d *FakeDriver) GetMachineStatus(_ context.Context, _ *GetMachineStatusRequ
 	return &GetMachineStatusResponse{
 		ProviderID: d.ProviderID,
 		NodeName:   d.NodeName,
+		Addresses:  d.Addresses,
 	}, d.Err
 }
 

pkg/util/provider/machinecontroller/controller_suite_test.go

@@ -8,7 +8,6 @@ import (
 	"context"
 	"flag"
 	"fmt"
-	"io"
 	"strconv"
 	"strings"
 	"testing"
@@ -37,22 +36,27 @@ import (
 	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/workqueue"
 	"k8s.io/klog/v2"
-	"k8s.io/utils/pointer"
+	"k8s.io/utils/ptr"
 )
 
 func TestMachineControllerSuite(t *testing.T) {
 	//to enable goroutine leak check , currently not using as it fails tests for less important leaks
 	//defer goleak.VerifyNone(t)
 
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Machine Controller Suite")
+}
+
+var _ = BeforeSuite(func() {
+	klog.SetOutput(GinkgoWriter)
 	//for filtering out warning logs. Reflector short watch warning logs won't print now
-	klog.SetOutput(io.Discard)
+	klog.LogToStderr(false)
 	flags := &flag.FlagSet{}
 	klog.InitFlags(flags)
-	_ = flags.Set("logtostderr", "false")
+	Expect(flags.Set("v", "10")).To(Succeed())
 
-	RegisterFailHandler(Fail)
-	RunSpecs(t, "Machine Controller Suite")
-}
+	DeferCleanup(klog.Flush)
+})
 
 var (
 	controllerKindMachine = v1alpha1.SchemeGroupVersion.WithKind("Machine")
@@ -171,8 +175,8 @@ func newMachineSetsFromMachineDeployment(
 			Kind:               t.Kind,
 			Name:               machineDeployment.Name,
 			UID:                machineDeployment.UID,
-			BlockOwnerDeletion: pointer.BoolPtr(true),
-			Controller:         pointer.BoolPtr(true),
+			BlockOwnerDeletion: ptr.To(true),
+			Controller:         ptr.To(true),
 		},
 		annotations,
 		finalLabels,

pkg/util/provider/machinecontroller/machine.go

@@ -9,6 +9,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"slices"
 	"strings"
 	"time"
 
@@ -485,13 +486,19 @@ func (c *controller) triggerCreationFlow(ctx context.Context, createMachineReque
 	var (
 		// Declarations
 		nodeName, providerID string
+		addresses            []corev1.NodeAddress
 
 		// Initializations
 		machine              = createMachineRequest.Machine
 		machineName          = createMachineRequest.Machine.Name
 		uninitializedMachine = false
 	)
 
+	// This field is only modified during the creation flow. We have to assume that every Address that was added to the
+	// status in the past remains valid, otherwise we have no way of keeping the addresses that might be only returned
+	// by the `CreateMachine` or `InitializeMachine` calls. Before persisting, the addresses are deduplicated.
+	addresses = append(addresses, createMachineRequest.Machine.Status.Addresses...)
+
 	// we should avoid mutating Secret, since it goes all the way into the Informer's store
 	secretCopy := createMachineRequest.Secret.DeepCopy()
 	if err := c.addBootstrapTokenToUserData(ctx, machine, secretCopy); err != nil {
@@ -538,6 +545,7 @@ func (c *controller) triggerCreationFlow(ctx context.Context, createMachineReque
 				}
 				nodeName = createMachineResponse.NodeName
 				providerID = createMachineResponse.ProviderID
+				addresses = append(addresses, createMachineResponse.Addresses...)
 				// Creation was successful
 				klog.V(2).Infof("Created new VM for machine: %q with ProviderID: %q and backing node: %q", machine.Name, providerID, nodeName)
 
@@ -619,26 +627,44 @@ func (c *controller) triggerCreationFlow(ctx context.Context, createMachineReque
 		}
 		nodeName = getMachineStatusResponse.NodeName
 		providerID = getMachineStatusResponse.ProviderID
+		addresses = append(addresses, getMachineStatusResponse.Addresses...)
 	}
+
 	//Update labels, providerID
 	var clone *v1alpha1.Machine
 	clone, err = c.updateLabels(ctx, createMachineRequest.Machine, nodeName, providerID)
+	if err != nil {
+		klog.V(2).Infof("failed to update labels and providerID for machine %q. err=%q", machine.Name, err.Error())
+	}
 	//initialize VM if not initialized
 	if uninitializedMachine {
 		var retryPeriod machineutils.RetryPeriod
-		retryPeriod, err = c.initializeMachine(ctx, clone, createMachineRequest.MachineClass, createMachineRequest.Secret)
+		var initAddresses []corev1.NodeAddress
+		retryPeriod, initAddresses, err = c.initializeMachine(ctx, clone, createMachineRequest.MachineClass, createMachineRequest.Secret)
 		if err != nil {
 			return retryPeriod, err
 		}
+
+		if c.targetCoreClient == nil {
+			// persist addresses from the InitializeMachine and CreateMachine responoses
+			clone := clone.DeepCopy()
+			addresses = append(addresses, initAddresses...)
+			clone.Status.Addresses = buildAddressStatus(addresses, nodeName)
+			if _, err := c.controlMachineClient.Machines(clone.Namespace).UpdateStatus(ctx, clone, metav1.UpdateOptions{}); err != nil {
+				return 0, fmt.Errorf("failed to persist status addresse after initialization was successfull: %w", err)
+			}
+		}
+
 		// Return error even when machine object is updated
 		err = fmt.Errorf("machine creation in process. Machine initialization (if required) is successful")
 		return machineutils.ShortRetry, err
 	}
 	if err != nil {
 		return machineutils.ShortRetry, err
 	}
+
 	if machine.Status.CurrentStatus.Phase == "" || machine.Status.CurrentStatus.Phase == v1alpha1.MachineCrashLoopBackOff {
-		clone := machine.DeepCopy()
+		clone := clone.DeepCopy()
 		clone.Status.LastOperation = v1alpha1.LastOperation{
 			Description:    "Creating machine on cloud provider",
 			State:          v1alpha1.MachineStateProcessing,
@@ -651,13 +677,14 @@ func (c *controller) triggerCreationFlow(ctx context.Context, createMachineReque
 			LastUpdateTime: metav1.Now(),
 		}
 
-		// If running without a target cluster, set the Machine to Available immediately after a successful VM creation.
+		// If running without a target cluster, set the Machine to Available immediately after a successful VM creation and report its addresses.
 		// Skip waiting for the Node object to get registered.
 		if c.targetCoreClient == nil {
 			clone.Status.LastOperation.Description = "Created machine on cloud provider"
 			clone.Status.LastOperation.State = v1alpha1.MachineStateSuccessful
 			clone.Status.CurrentStatus.Phase = v1alpha1.MachineAvailable
 			clone.Status.CurrentStatus.TimeoutActive = false
+			clone.Status.Addresses = buildAddressStatus(addresses, nodeName)
 		}
 
 		_, err := c.controlMachineClient.Machines(clone.Namespace).UpdateStatus(ctx, clone, metav1.UpdateOptions{})
@@ -704,7 +731,7 @@ func (c *controller) updateLabels(ctx context.Context, machine *v1alpha1.Machine
 	return clone, err
 }
 
-func (c *controller) initializeMachine(ctx context.Context, machine *v1alpha1.Machine, machineClass *v1alpha1.MachineClass, secret *corev1.Secret) (machineutils.RetryPeriod, error) {
+func (c *controller) initializeMachine(ctx context.Context, machine *v1alpha1.Machine, machineClass *v1alpha1.MachineClass, secret *corev1.Secret) (retry machineutils.RetryPeriod, addresses []corev1.NodeAddress, err error) {
 	req := &driver.InitializeMachineRequest{
 		Machine:      machine,
 		MachineClass: machineClass,
@@ -716,11 +743,11 @@ func (c *controller) initializeMachine(ctx context.Context, machine *v1alpha1.Ma
 		errStatus, ok := status.FromError(err)
 		if !ok {
 			klog.Errorf("Cannot decode Driver error for machine %q: %s. Unexpected behaviour as Driver errors are expected to be of type status.Status", machine.Name, err)
-			return machineutils.LongRetry, err
+			return machineutils.LongRetry, nil, err
 		}
 		if errStatus.Code() == codes.Unimplemented {
 			klog.V(3).Infof("Provider does not support Driver.InitializeMachine - skipping VM instance initialization for %q.", machine.Name)
-			return 0, nil
+			return 0, nil, nil
 		}
 		klog.Errorf("Error occurred while initializing VM instance for machine %q: %s", machine.Name, err)
 		updateRetryPeriod, updateErr := c.machineStatusUpdate(
@@ -740,12 +767,12 @@ func (c *controller) initializeMachine(ctx context.Context, machine *v1alpha1.Ma
 			machine.Status.LastKnownState,
 		)
 		if updateErr != nil {
-			return updateRetryPeriod, updateErr
+			return updateRetryPeriod, nil, updateErr
 		}
-		return machineutils.ShortRetry, err
+		return machineutils.ShortRetry, nil, err
 	}
 	klog.V(3).Infof("VM instance %q for machine %q was initialized", resp.ProviderID, machine.Name)
-	return 0, nil
+	return 0, resp.Addresses, nil
 }
 
 func (c *controller) triggerDeletionFlow(ctx context.Context, deleteMachineRequest *driver.DeleteMachineRequest) (machineutils.RetryPeriod, error) {
@@ -811,3 +838,18 @@ func (c *controller) triggerDeletionFlow(ctx context.Context, deleteMachineReque
 	klog.V(2).Infof("Machine %q with providerID %q and nodeName %q deleted successfully", machine.Name, getProviderID(machine), getNodeName(machine))
 	return machineutils.LongRetry, nil
 }
+
+// buildAddressStatus adds the nodeName as a HostName address, if it is not empty, and returns a sorted and deduplicated
+// slice.
+func buildAddressStatus(addresses []corev1.NodeAddress, nodeName string) []corev1.NodeAddress {
+	if nodeName != "" {
+		addresses = append(addresses, corev1.NodeAddress{
+			Type:    corev1.NodeHostName,
+			Address: nodeName,
+		})
+	}
+	slices.SortStableFunc(addresses, func(a, b corev1.NodeAddress) int {
+		return strings.Compare(string(a.Type)+a.Address, string(b.Type)+b.Address)
+	})
+	return slices.Compact(addresses)
+}

pkg/util/provider/machinecontroller/machine_safety_test.go

@@ -305,7 +305,7 @@ var _ = Describe("safety_logic", func() {
 				controlMachineObjects = append(controlMachineObjects, obj)
 			}
 
-			fakeDriver := driver.NewFakeDriver(false, "", "", "", nil, nil)
+			fakeDriver := driver.NewFakeDriver(false, "", "", "", nil, nil, nil)
 
 			c, trackers := createController(stop, testNamespace, controlMachineObjects, controlCoreObjects, nil, fakeDriver, false)
 			defer trackers.Stop()