diff --git a/Dockerfile b/Dockerfile index 6b338b635..bd8e238c1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -7,7 +7,7 @@ COPY . . RUN --mount=type=cache,target="/root/.cache/go-build" .ci/build ############# base ############# -FROM gcr.io/distroless/static-debian12:nonroot as base +FROM gcr.io/distroless/static-debian12:nonroot AS base WORKDIR / ############# machine-controller-manager ############# diff --git a/docs/documents/apis.md b/docs/documents/apis.md index cfda6b2e1..9b2668d1d 100644 --- a/docs/documents/apis.md +++ b/docs/documents/apis.md @@ -1080,7 +1080,8 @@ Kubernetes meta/v1.Duration (Optional) -

DisableHealthTimeout if set to true, health timeout will be ignored. Leading to machine never being declared failed.

+

DisableHealthTimeout if set to true, health timeout will be ignored. Leading to machine never being declared failed. +This is intended to be used only for in-place updates.

@@ -1542,8 +1543,8 @@ newest MachineSet.

- -[]*github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1.MachineSummary + +[]*../../pkg/apis/machine/v1alpha1.MachineSummary @@ -1987,8 +1988,8 @@ LastOperation - -[]github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1.MachineSummary + +[]../../pkg/apis/machine/v1alpha1.MachineSummary @@ -2120,6 +2121,23 @@ MachineConfiguration +addresses + + + + +[]Kubernetes core/v1.NodeAddress + + + + +(Optional) +

Addresses of this machines. This field is only present if the MCM provider runs without a target cluster and may +be used by clients to determine how to connect to the machine, instead of the Node.status.addresses field.

+ + + + conditions diff --git a/kubernetes/crds/machine.sapcloud.io_machinedeployments.yaml b/kubernetes/crds/machine.sapcloud.io_machinedeployments.yaml index e1e155902..d836282a2 100644 --- a/kubernetes/crds/machine.sapcloud.io_machinedeployments.yaml +++ b/kubernetes/crds/machine.sapcloud.io_machinedeployments.yaml @@ -280,9 +280,9 @@ spec: machinie creation is declared failed. type: string disableHealthTimeout: - description: DisableHealthTimeout if set to true, health timeout - will be ignored. Leading to machine never being declared - failed. + description: |- + DisableHealthTimeout if set to true, health timeout will be ignored. Leading to machine never being declared failed. + This is intended to be used only for in-place updates. type: boolean drainTimeout: description: MachineDraintimeout is the timeout after which diff --git a/kubernetes/crds/machine.sapcloud.io_machines.yaml b/kubernetes/crds/machine.sapcloud.io_machines.yaml index 1e8c54bf1..9378f0274 100644 --- a/kubernetes/crds/machine.sapcloud.io_machines.yaml +++ b/kubernetes/crds/machine.sapcloud.io_machines.yaml @@ -79,8 +79,9 @@ spec: creation is declared failed. type: string disableHealthTimeout: - description: DisableHealthTimeout if set to true, health timeout will - be ignored. Leading to machine never being declared failed. + description: |- + DisableHealthTimeout if set to true, health timeout will be ignored. Leading to machine never being declared failed. + This is intended to be used only for in-place updates. type: boolean drainTimeout: description: MachineDraintimeout is the timeout after which machine @@ -223,6 +224,25 @@ spec: status: description: Status contains fields depicting the status properties: + addresses: + description: |- + Addresses of this machines. This field is only present if the MCM provider runs without a target cluster and may + be used by clients to determine how to connect to the machine, instead of the `Node.status.addresses` field. + items: + description: NodeAddress contains information for the node's address. + properties: + address: + description: The node address. + type: string + type: + description: Node address type, one of Hostname, ExternalIP + or InternalIP. + type: string + required: + - address + - type + type: object + type: array conditions: description: Conditions of this machine, same as node items: diff --git a/kubernetes/crds/machine.sapcloud.io_machinesets.yaml b/kubernetes/crds/machine.sapcloud.io_machinesets.yaml index bb57adf60..c569dffe1 100644 --- a/kubernetes/crds/machine.sapcloud.io_machinesets.yaml +++ b/kubernetes/crds/machine.sapcloud.io_machinesets.yaml @@ -162,9 +162,9 @@ spec: machinie creation is declared failed. type: string disableHealthTimeout: - description: DisableHealthTimeout if set to true, health timeout - will be ignored. Leading to machine never being declared - failed. + description: |- + DisableHealthTimeout if set to true, health timeout will be ignored. Leading to machine never being declared failed. + This is intended to be used only for in-place updates. type: boolean drainTimeout: description: MachineDraintimeout is the timeout after which diff --git a/pkg/apis/machine/types.go b/pkg/apis/machine/types.go index d0ff57388..8ac47c2f0 100644 --- a/pkg/apis/machine/types.go +++ b/pkg/apis/machine/types.go @@ -162,6 +162,10 @@ type CurrentStatus struct { // MachineStatus holds the most recently observed status of Machine. type MachineStatus struct { + // Addresses of this machines. This field is only present if the MCM provider runs without a target cluster and may + // be used by clients to determine how to connect to the machine, instead of the `Node.status.addresses` field. + Addresses []corev1.NodeAddress + // Conditions of this machine, same as node Conditions []corev1.NodeCondition diff --git a/pkg/apis/machine/v1alpha1/machine_types.go b/pkg/apis/machine/v1alpha1/machine_types.go index ffc381437..19e017925 100644 --- a/pkg/apis/machine/v1alpha1/machine_types.go +++ b/pkg/apis/machine/v1alpha1/machine_types.go @@ -92,6 +92,11 @@ type NodeTemplateSpec struct { // MachineStatus holds the most recently observed status of Machine. type MachineStatus struct { + // Addresses of this machines. This field is only present if the MCM provider runs without a target cluster and may + // be used by clients to determine how to connect to the machine, instead of the `Node.status.addresses` field. + // +optional + Addresses []corev1.NodeAddress `json:"addresses,omitempty"` + // Conditions of this machine, same as node Conditions []corev1.NodeCondition `json:"conditions,omitempty"` diff --git a/pkg/apis/machine/v1alpha1/zz_generated.conversion.go b/pkg/apis/machine/v1alpha1/zz_generated.conversion.go index 385ab9aae..1ef0f3aeb 100644 --- a/pkg/apis/machine/v1alpha1/zz_generated.conversion.go +++ b/pkg/apis/machine/v1alpha1/zz_generated.conversion.go @@ -959,6 +959,7 @@ func Convert_machine_MachineSpec_To_v1alpha1_MachineSpec(in *machine.MachineSpec } func autoConvert_v1alpha1_MachineStatus_To_machine_MachineStatus(in *MachineStatus, out *machine.MachineStatus, s conversion.Scope) error { + out.Addresses = *(*[]v1.NodeAddress)(unsafe.Pointer(&in.Addresses)) out.Conditions = *(*[]v1.NodeCondition)(unsafe.Pointer(&in.Conditions)) if err := Convert_v1alpha1_LastOperation_To_machine_LastOperation(&in.LastOperation, &out.LastOperation, s); err != nil { return err @@ -976,6 +977,7 @@ func Convert_v1alpha1_MachineStatus_To_machine_MachineStatus(in *MachineStatus, } func autoConvert_machine_MachineStatus_To_v1alpha1_MachineStatus(in *machine.MachineStatus, out *MachineStatus, s conversion.Scope) error { + out.Addresses = *(*[]v1.NodeAddress)(unsafe.Pointer(&in.Addresses)) out.Conditions = *(*[]v1.NodeCondition)(unsafe.Pointer(&in.Conditions)) if err := Convert_machine_LastOperation_To_v1alpha1_LastOperation(&in.LastOperation, &out.LastOperation, s); err != nil { return err diff --git a/pkg/apis/machine/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/machine/v1alpha1/zz_generated.deepcopy.go index b3ed2a469..a5f4b0571 100644 --- a/pkg/apis/machine/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/machine/v1alpha1/zz_generated.deepcopy.go @@ -614,6 +614,11 @@ func (in *MachineSpec) DeepCopy() *MachineSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MachineStatus) DeepCopyInto(out *MachineStatus) { *out = *in + if in.Addresses != nil { + in, out := &in.Addresses, &out.Addresses + *out = make([]v1.NodeAddress, len(*in)) + copy(*out, *in) + } if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make([]v1.NodeCondition, len(*in)) diff --git a/pkg/apis/machine/zz_generated.deepcopy.go b/pkg/apis/machine/zz_generated.deepcopy.go index d745dec9c..dd0487331 100644 --- a/pkg/apis/machine/zz_generated.deepcopy.go +++ b/pkg/apis/machine/zz_generated.deepcopy.go @@ -647,6 +647,11 @@ func (in *MachineSpec) DeepCopy() *MachineSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MachineStatus) DeepCopyInto(out *MachineStatus) { *out = *in + if in.Addresses != nil { + in, out := &in.Addresses, &out.Addresses + *out = make([]v1.NodeAddress, len(*in)) + copy(*out, *in) + } if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make([]v1.NodeCondition, len(*in)) diff --git a/pkg/openapi/api_violations.report b/pkg/openapi/api_violations.report index 6634216bb..0861c8d4c 100644 --- a/pkg/openapi/api_violations.report +++ b/pkg/openapi/api_violations.report @@ -1,6 +1,7 @@ API rule violation: list_type_missing,github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1,MachineDeploymentStatus,Conditions API rule violation: list_type_missing,github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1,MachineDeploymentStatus,FailedMachines API rule violation: list_type_missing,github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1,MachineSetStatus,Conditions +API rule violation: list_type_missing,github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1,MachineStatus,Addresses API rule violation: list_type_missing,github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1,MachineStatus,Conditions API rule violation: names_match,github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1,MachineConfiguration,MachineCreationTimeout API rule violation: names_match,github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1,MachineConfiguration,MachineDrainTimeout diff --git a/pkg/openapi/openapi_generated.go b/pkg/openapi/openapi_generated.go index a7f8ba82d..f602824f5 100644 --- a/pkg/openapi/openapi_generated.go +++ b/pkg/openapi/openapi_generated.go @@ -684,7 +684,7 @@ func schema_pkg_apis_machine_v1alpha1_MachineConfiguration(ref common.ReferenceC }, "disableHealthTimeout": { SchemaProps: spec.SchemaProps{ - Description: "DisableHealthTimeout if set to true, health timeout will be ignored. Leading to machine never being declared failed.", + Description: "DisableHealthTimeout if set to true, health timeout will be ignored. Leading to machine never being declared failed. This is intended to be used only for in-place updates.", Type: []string{"boolean"}, Format: "", }, @@ -1464,7 +1464,7 @@ func schema_pkg_apis_machine_v1alpha1_MachineSpec(ref common.ReferenceCallback) }, "disableHealthTimeout": { SchemaProps: spec.SchemaProps{ - Description: "DisableHealthTimeout if set to true, health timeout will be ignored. Leading to machine never being declared failed.", + Description: "DisableHealthTimeout if set to true, health timeout will be ignored. Leading to machine never being declared failed. This is intended to be used only for in-place updates.", Type: []string{"boolean"}, Format: "", }, @@ -1498,6 +1498,20 @@ func schema_pkg_apis_machine_v1alpha1_MachineStatus(ref common.ReferenceCallback Description: "MachineStatus holds the most recently observed status of Machine.", Type: []string{"object"}, Properties: map[string]spec.Schema{ + "addresses": { + SchemaProps: spec.SchemaProps{ + Description: "Addresses of this machines. This field is only present if the MCM provider runs without a target cluster and may be used by clients to determine how to connect to the machine, instead of the `Node.status.addresses` field.", + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: map[string]interface{}{}, + Ref: ref("k8s.io/api/core/v1.NodeAddress"), + }, + }, + }, + }, + }, "conditions": { SchemaProps: spec.SchemaProps{ Description: "Conditions of this machine, same as node", @@ -1537,7 +1551,7 @@ func schema_pkg_apis_machine_v1alpha1_MachineStatus(ref common.ReferenceCallback }, }, Dependencies: []string{ - "github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1.CurrentStatus", "github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1.LastOperation", "k8s.io/api/core/v1.NodeCondition"}, + "github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1.CurrentStatus", "github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1.LastOperation", "k8s.io/api/core/v1.NodeAddress", "k8s.io/api/core/v1.NodeCondition"}, } } diff --git a/pkg/util/provider/driver/driver.go b/pkg/util/provider/driver/driver.go index 2177d72f4..b1c976d2e 100644 --- a/pkg/util/provider/driver/driver.go +++ b/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 diff --git a/pkg/util/provider/driver/fake.go b/pkg/util/provider/driver/fake.go index 8f8326711..6c5387a43 100644 --- a/pkg/util/provider/driver/fake.go +++ b/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 } diff --git a/pkg/util/provider/machinecontroller/controller_suite_test.go b/pkg/util/provider/machinecontroller/controller_suite_test.go index cd8fb53a6..f0e94db7c 100644 --- a/pkg/util/provider/machinecontroller/controller_suite_test.go +++ b/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, diff --git a/pkg/util/provider/machinecontroller/machine.go b/pkg/util/provider/machinecontroller/machine.go index f32d9ed46..a15710f10 100644 --- a/pkg/util/provider/machinecontroller/machine.go +++ b/pkg/util/provider/machinecontroller/machine.go @@ -9,6 +9,8 @@ import ( "context" "errors" "fmt" + "maps" + "slices" "strings" "time" @@ -490,8 +492,14 @@ func (c *controller) triggerCreationFlow(ctx context.Context, createMachineReque machine = createMachineRequest.Machine machineName = createMachineRequest.Machine.Name uninitializedMachine = false + addresses = sets.New[corev1.NodeAddress]() ) + // 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.Insert(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 +546,7 @@ func (c *controller) triggerCreationFlow(ctx context.Context, createMachineReque } nodeName = createMachineResponse.NodeName providerID = createMachineResponse.ProviderID + addresses.Insert(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,17 +628,34 @@ func (c *controller) triggerCreationFlow(ctx context.Context, createMachineReque } nodeName = getMachineStatusResponse.NodeName providerID = getMachineStatusResponse.ProviderID + addresses.Insert(getMachineStatusResponse.Addresses...) } + //Update labels, providerID var clone *v1alpha1.Machine clone, err = c.updateLabels(ctx, createMachineRequest.Machine, nodeName, providerID) + if err != nil { + klog.Errorf("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 initResponse *driver.InitializeMachineResponse + initResponse, retryPeriod, 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 responses + clone := clone.DeepCopy() + addresses.Insert(initResponse.Addresses...) + clone.Status.Addresses = buildAddressStatus(addresses, nodeName) + if _, err := c.controlMachineClient.Machines(clone.Namespace).UpdateStatus(ctx, clone, metav1.UpdateOptions{}); err != nil { + return machineutils.ShortRetry, fmt.Errorf("failed to persist status addresses after initialization was successful: %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 @@ -637,8 +663,9 @@ func (c *controller) triggerCreationFlow(ctx context.Context, createMachineReque 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 +678,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,23 +732,23 @@ 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) (resp *driver.InitializeMachineResponse, retry machineutils.RetryPeriod, err error) { req := &driver.InitializeMachineRequest{ Machine: machine, MachineClass: machineClass, Secret: secret, } klog.V(3).Infof("Initializing VM instance for Machine %q", machine.Name) - resp, err := c.driver.InitializeMachine(ctx, req) + resp, err = c.driver.InitializeMachine(ctx, req) if err != nil { 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 nil, machineutils.LongRetry, 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 nil, 0, nil } klog.Errorf("Error occurred while initializing VM instance for machine %q: %s", machine.Name, err) updateRetryPeriod, updateErr := c.machineStatusUpdate( @@ -740,12 +768,12 @@ func (c *controller) initializeMachine(ctx context.Context, machine *v1alpha1.Ma machine.Status.LastKnownState, ) if updateErr != nil { - return updateRetryPeriod, updateErr + return nil, updateRetryPeriod, updateErr } - return machineutils.ShortRetry, err + return nil, machineutils.ShortRetry, err } klog.V(3).Infof("VM instance %q for machine %q was initialized", resp.ProviderID, machine.Name) - return 0, nil + return resp, 0, nil } func (c *controller) triggerDeletionFlow(ctx context.Context, deleteMachineRequest *driver.DeleteMachineRequest) (machineutils.RetryPeriod, error) { @@ -811,3 +839,19 @@ 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 sets.Set[corev1.NodeAddress], nodeName string) []corev1.NodeAddress { + if nodeName != "" { + addresses.Insert(corev1.NodeAddress{ + Type: corev1.NodeHostName, + Address: nodeName, + }) + } + res := slices.Collect(maps.Keys(addresses)) + slices.SortStableFunc(res, func(a, b corev1.NodeAddress) int { + return strings.Compare(string(a.Type)+a.Address, string(b.Type)+b.Address) + }) + return res +} diff --git a/pkg/util/provider/machinecontroller/machine_safety_test.go b/pkg/util/provider/machinecontroller/machine_safety_test.go index e2a1251c6..e1914b0f8 100644 --- a/pkg/util/provider/machinecontroller/machine_safety_test.go +++ b/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() diff --git a/pkg/util/provider/machinecontroller/machine_test.go b/pkg/util/provider/machinecontroller/machine_test.go index 4b735fb88..d293988df 100644 --- a/pkg/util/provider/machinecontroller/machine_test.go +++ b/pkg/util/provider/machinecontroller/machine_test.go @@ -531,6 +531,7 @@ var _ = Describe("machine", func() { data.action.fakeDriver.ProviderID, data.action.fakeDriver.NodeName, data.action.fakeDriver.LastKnownState, + data.action.fakeDriver.Addresses, data.action.fakeDriver.Err, nil, ) @@ -571,6 +572,8 @@ var _ = Describe("machine", func() { Expect(actual.Finalizers).To(Equal(data.expect.machine.Finalizers)) Expect(retry).To(Equal(data.expect.retry)) Expect(actual.Status.CurrentStatus.Phase).To(Equal(data.expect.machine.Status.CurrentStatus.Phase)) + Expect(actual.Status.Addresses).To(Equal(data.expect.machine.Status.Addresses)) + if data.expect.machine.Labels == nil { Expect(actual.Labels).To(BeNil()) } else { @@ -1254,20 +1257,39 @@ var _ = Describe("machine", func() { VMExists: false, ProviderID: "fakeID-0", NodeName: "fakeNode-0", - Err: nil, + Addresses: []corev1.NodeAddress{{ + Type: corev1.NodeInternalIP, + Address: "10.0.0.1", + }}, + Err: nil, }, }, expect: expect{ - machine: newMachine(&v1alpha1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - Spec: v1alpha1.MachineSpec{ - Class: v1alpha1.ClassSpec{ - Kind: "MachineClass", - Name: "machineClass", + machine: newMachine( + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machineClass", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Addresses: []corev1.NodeAddress{ + { + Type: corev1.NodeHostName, + Address: "fakeNode-0", + }, + { + Type: corev1.NodeInternalIP, + Address: "10.0.0.1", + }, }, - ProviderID: "fakeID", }, - }, nil, nil, nil, map[string]string{}, true, metav1.Now()), + nil, nil, map[string]string{}, true, metav1.Now(), + ), err: fmt.Errorf("machine creation in process. Machine initialization (if required) is successful"), retry: machineutils.ShortRetry, }, @@ -1338,6 +1360,10 @@ var _ = Describe("machine", func() { State: v1alpha1.MachineStateSuccessful, Description: "Created machine on cloud provider", }, + Addresses: []corev1.NodeAddress{{ + Type: corev1.NodeHostName, + Address: "fakeNode-0", + }}, }, nil, map[string]string{ @@ -1389,6 +1415,10 @@ var _ = Describe("machine", func() { Type: v1alpha1.MachineOperationCreate, LastUpdateTime: metav1.Now(), }, + Addresses: []corev1.NodeAddress{{ + Type: corev1.NodeHostName, + Address: "fakeNode-0", + }}, }, nil, map[string]string{ @@ -1432,6 +1462,10 @@ var _ = Describe("machine", func() { Type: v1alpha1.MachineOperationCreate, LastUpdateTime: metav1.Now(), }, + Addresses: []corev1.NodeAddress{{ + Type: corev1.NodeHostName, + Address: "fakeNode-0", + }}, }, nil, map[string]string{ @@ -1508,6 +1542,7 @@ var _ = Describe("machine", func() { data.action.fakeDriver.ProviderID, data.action.fakeDriver.NodeName, data.action.fakeDriver.LastKnownState, + data.action.fakeDriver.Addresses, data.action.fakeDriver.Err, nil, ) diff --git a/pkg/util/provider/machinecontroller/machineclass_test.go b/pkg/util/provider/machinecontroller/machineclass_test.go index bffa81066..edf223cf8 100644 --- a/pkg/util/provider/machinecontroller/machineclass_test.go +++ b/pkg/util/provider/machinecontroller/machineclass_test.go @@ -68,6 +68,7 @@ var _ = Describe("machineclass", func() { data.action.fakeDriver.ProviderID, data.action.fakeDriver.NodeName, data.action.fakeDriver.LastKnownState, + data.action.fakeDriver.Addresses, data.action.fakeDriver.Err, nil, )