Skip to content

feat: Implement optional driver GetMachineStatus method #334

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.

Sign up for GitHub

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

Open
jamand wants to merge 4 commits into
base: master
Choose a base branch
from
Open

feat: Implement optional driver GetMachineStatus method #334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b53165c
feat: Implement GetMachineStatus for GEP-28 (Managed infra flow)
jamand Nov 10, 2025
a7e5895
chore: Refactor GetMachineStatus + other methods
jamand Nov 11, 2025
ed75679
fix: Replace Hostname by Name of Machine in GetMachineStatus
jamand Nov 13, 2025
0459c9a
chore(executor): Rename GetMachineByID to GetMachineByProviderID (PR …
jamand Dec 19, 2025
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
147 changes: 73 additions & 74 deletions pkg/driver/driver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@ package driver

import (
"context"
"errors"
"fmt"

"github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1"
"github.com/gardener/machine-controller-manager/pkg/util/provider/driver"
"github.com/gardener/machine-controller-manager/pkg/util/provider/machinecodes/codes"
"github.com/gardener/machine-controller-manager/pkg/util/provider/machinecodes/status"
"github.com/gophercloud/gophercloud/v2/openstack/compute/v2/servers"
corev1 "k8s.io/api/core/v1"
"k8s.io/klog/v2"

Expand All @@ -28,39 +31,31 @@ const (
// NOTE
//
// The basic working of the controller will work with just implementing the CreateMachine() & DeleteMachine() methods.
// You can first implement these two methods and check the working of the controller.
// Leaving the other methods to NOT_IMPLEMENTED error status.
// Once this works you can implement the rest of the methods.
// The other methods should be implemented according to specs or return the NOT_IMPLEMENTED error status.
//
// Also make sure each method return appropriate errors mentioned in `https://github.com/gardener/machine-controller-manager/blob/master/docs/development/machine_error_codes.md`

// CreateMachine handles a machine creation request
//
// OPTIONAL IMPLEMENTATION LOGIC
// It is optionally expected by the safety controller to use an identification mechanism to map the VM Created by a providerSpec.
// These could be done using tag(s)/resource-groups etc.
// This logic is used by safety controller to delete orphan VMs which are not backed by any machine CRD
func (p *OpenstackDriver) CreateMachine(ctx context.Context, req *driver.CreateMachineRequest) (*driver.CreateMachineResponse, error) {
klog.V(2).Infof("CreateMachine request has been received for %q", req.Machine.Name)
defer klog.V(2).Infof("CreateMachine request has been processed for %q", req.Machine.Name)
// Make sure each method returns the appropriate errors mentioned in
// `https://github.com/gardener/machine-controller-manager/blob/master/docs/development/machine_error_codes.md`

// setupExecutor handles the common validation and setup, returning a ready-to-use executor.
func (p *OpenstackDriver) setupExecutor(ctx context.Context, machineClass *v1alpha1.MachineClass, secret *corev1.Secret) (*executor.Executor, error) {
// Check if incoming provider in the MachineClass is a provider we support
if req.MachineClass.Provider != openstackProvider {
err := fmt.Errorf("requested for Provider '%s', we only support '%s'", req.MachineClass.Provider, openstackProvider)
if machineClass.Provider != openstackProvider {
err := fmt.Errorf("requested for Provider '%s', we only support '%s'", machineClass.Provider, openstackProvider)
return nil, status.Error(codes.InvalidArgument, err.Error())
}

providerConfig, err := p.decodeProviderSpec(req.MachineClass.ProviderSpec)
providerConfig, err := p.decodeProviderSpec(machineClass.ProviderSpec)
if err != nil {
klog.Errorf("decoding provider spec for machine class %q failed with: %v", req.MachineClass.Name, err)
klog.Errorf("decoding provider spec for machine class %q failed with: %v", machineClass.Name, err)
return nil, status.Error(codes.InvalidArgument, err.Error())
}
if err := validation.ValidateRequest(providerConfig, req.Secret); err != nil {
klog.Errorf("validating request for machine %q failed with: %v", req.Machine.Name, err)

if err := validation.ValidateRequest(providerConfig, secret); err != nil {
klog.Errorf("validating request failed with: %v", err)
return nil, status.Error(codes.InvalidArgument, err.Error())
}

factory, err := client.NewFactoryFromSecret(ctx, req.Secret)
factory, err := client.NewFactoryFromSecret(ctx, secret)
if err != nil {
klog.Errorf("failed to construct OpenStack client: %v", err)
return nil, status.Error(mapErrorToCode(err), fmt.Sprintf("failed to construct OpenStack client: %v", err))
Expand All @@ -72,6 +67,24 @@ func (p *OpenstackDriver) CreateMachine(ctx context.Context, req *driver.CreateM
return nil, status.Error(mapErrorToCode(err), fmt.Sprintf("failed to construct context for the request: %v", err))
}

return ex, nil
}

// CreateMachine handles a machine creation request
//
// OPTIONAL IMPLEMENTATION LOGIC
// It is optionally expected by the safety controller to use an identification mechanism to map the VM Created by a providerSpec.
// These could be done using tag(s)/resource-groups etc.
// This logic is used by safety controller to delete orphan VMs which are not backed by any machine CRD
func (p *OpenstackDriver) CreateMachine(ctx context.Context, req *driver.CreateMachineRequest) (*driver.CreateMachineResponse, error) {
klog.V(2).Infof("CreateMachine request has been received for %q", req.Machine.Name)
defer klog.V(2).Infof("CreateMachine request has been processed for %q", req.Machine.Name)

ex, err := p.setupExecutor(ctx, req.MachineClass, req.Secret)
if err != nil {
return nil, err
}

server, err := ex.CreateMachine(ctx, req.Machine.Name, req.Secret.Data[cloudprovider.UserData])
if err != nil {
klog.Errorf("machine creation for machine %q failed with: %v", req.Machine.Name, err)
Expand Down Expand Up @@ -109,48 +122,58 @@ func (p *OpenstackDriver) DeleteMachine(ctx context.Context, req *driver.DeleteM
klog.V(2).Infof("DeleteMachine request has been received for %q", req.Machine.Name)
defer klog.V(2).Infof("DeleteMachine request has been processed for %q", req.Machine.Name)

// Check if incoming provider in the MachineClass is a provider we support
if req.MachineClass.Provider != openstackProvider {
err := fmt.Errorf("requested for Provider '%s', we only support '%s'", req.MachineClass.Provider, openstackProvider)
return nil, status.Error(codes.InvalidArgument, err.Error())
ex, err := p.setupExecutor(ctx, req.MachineClass, req.Secret)
if err != nil {
return nil, err
}

providerConfig, err := p.decodeProviderSpec(req.MachineClass.ProviderSpec)
err = ex.DeleteMachine(ctx, req.Machine.Name, req.Machine.Spec.ProviderID)
if err != nil {
klog.V(2).Infof("decoding provider spec for machine class %q failed with: %v", req.MachineClass.Name, err)
return nil, status.Error(codes.InvalidArgument, err.Error())
}
if err := validation.ValidateRequest(providerConfig, req.Secret); err != nil {
klog.V(2).Infof("validating request for machine class %q failed with: %v", req.MachineClass.Name, err)
return nil, status.Error(codes.InvalidArgument, err.Error())
return nil, status.Error(mapErrorToCode(err), err.Error())
}
return &driver.DeleteMachineResponse{}, nil
}

// GetMachineStatus handles a machine get status request
func (p *OpenstackDriver) GetMachineStatus(ctx context.Context, req *driver.GetMachineStatusRequest) (response *driver.GetMachineStatusResponse, err error) {
// Log messages to track start and end of request
klog.V(2).Infof("GetMachineStatus request has been received for %q", req.Machine.Name)
defer klog.V(2).Infof("GetMachineStatus request has been processed for: %q", req.Machine.Name)

factory, err := client.NewFactoryFromSecret(ctx, req.Secret)
ex, err := p.setupExecutor(ctx, req.MachineClass, req.Secret)
if err != nil {
klog.Errorf("failed to construct OpenStack client: %v", err)
return nil, status.Error(mapErrorToCode(err), fmt.Sprintf("failed to construct OpenStack client: %v", err))
return nil, err
}

ex, err := executor.NewExecutor(factory, providerConfig)
if err != nil {
klog.Errorf("failed to construct context for the request: %v", err)
return nil, status.Error(mapErrorToCode(err), fmt.Sprintf("failed to construct context for the request: %v", err))
var machine *servers.Server
// Finding by ProviderID should be the common path, by name fallback for pre-creation
if req.Machine.Spec.ProviderID != "" {
klog.V(2).Infof("Finding Machine (%q) by ProviderID: %q", req.Machine.Name, req.Machine.Spec.ProviderID)
machine, err = ex.GetMachineByProviderID(ctx, req.Machine.Spec.ProviderID)
} else {
klog.V(2).Infof("Finding Machine by Tags and Name: %q", req.Machine.Name)
machine, err = ex.GetMachineByName(ctx, req.Machine.Name)
}

err = ex.DeleteMachine(ctx, req.Machine.Name, req.Machine.Spec.ProviderID)
if err != nil {
if errors.Is(err, executor.ErrNotFound) {
klog.V(2).Infof("Machine status: did not find Machine: %q", req.Machine.Name)
} else {
klog.Errorf("Failed to get Machine %q: %v", req.Machine.Name, err)
}
return nil, status.Error(mapErrorToCode(err), err.Error())
}
return &driver.DeleteMachineResponse{}, nil
}

// GetMachineStatus handles a machine get status request
func (p *OpenstackDriver) GetMachineStatus(_ context.Context, req *driver.GetMachineStatusRequest) (*driver.GetMachineStatusResponse, error) {
// Log messages to track start and end of request
klog.V(2).Infof("GetMachineStatus request has been received for %q", req.Machine.Name)
defer klog.V(2).Infof("GetMachineStatus is not implemented")
if machine.Name != req.Machine.Name {
klog.Errorf("Name of server with ProviderID %q (%q) does not match req.Machine.Name %q",
req.Machine.Spec.ProviderID, machine.Name, req.Machine.Name)
return nil, status.Error(codes.Internal, "Name and request machine name mismatch")
}

return nil, status.Error(codes.Unimplemented, "method not implemented")
return &driver.GetMachineStatusResponse{
ProviderID: req.Machine.Spec.ProviderID,
NodeName: machine.Name,
}, nil
}

// ListMachines lists all the machines possibly created by a providerSpec
Expand All @@ -161,33 +184,9 @@ func (p *OpenstackDriver) ListMachines(ctx context.Context, req *driver.ListMach
klog.V(2).Infof("ListMachines request has been received for %q", req.MachineClass.Name)
defer klog.V(2).Infof("ListMachines request has been processed for %q", req.MachineClass.Name)

// Check if incoming provider in the MachineClass is a provider we support
if req.MachineClass.Provider != openstackProvider {
err := fmt.Errorf("requested for Provider '%s', we only support '%s'", req.MachineClass.Provider, openstackProvider)
return nil, status.Error(codes.InvalidArgument, err.Error())
}

providerConfig, err := p.decodeProviderSpec(req.MachineClass.ProviderSpec)
if err != nil {
klog.Errorf("decoding provider spec for machine class %q failed with: %v", req.MachineClass.Name, err)
return nil, status.Error(codes.InvalidArgument, err.Error())
}

if err := validation.ValidateRequest(providerConfig, req.Secret); err != nil {
klog.Errorf("validating request for machine class %q failed with: %v", req.MachineClass.Name, err)
return nil, status.Error(codes.InvalidArgument, err.Error())
}

factory, err := client.NewFactoryFromSecret(ctx, req.Secret)
ex, err := p.setupExecutor(ctx, req.MachineClass, req.Secret)
if err != nil {
klog.Errorf("failed to construct OpenStack client: %v", err)
return nil, status.Error(mapErrorToCode(err), fmt.Sprintf("failed to construct OpenStack client: %v", err))
}

ex, err := executor.NewExecutor(factory, providerConfig)
if err != nil {
klog.Errorf("failed to construct context for the request: %v", err)
return nil, status.Error(mapErrorToCode(err), fmt.Sprintf("failed to construct context for the request: %v", err))
return nil, err
}

machines, err := ex.ListMachines(ctx)
Expand Down
18 changes: 9 additions & 9 deletions pkg/driver/executor/executor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func (ex *Executor) CreateMachine(ctx context.Context, machineName string, userD
return err
}

server, err = ex.getMachineByName(ctx, machineName)
server, err = ex.GetMachineByName(ctx, machineName)
if err == nil {
klog.Infof("found existing server [Name=%q, ID=%q]", machineName, server.ID)
} else if !errors.Is(err, ErrNotFound) {
Expand Down Expand Up @@ -510,10 +510,9 @@ func (ex *Executor) DeleteMachine(ctx context.Context, machineName, providerID s
)

if !isEmptyString(ptr.To(providerID)) {
serverID := decodeProviderID(providerID)
server, err = ex.getMachineByID(ctx, serverID)
server, err = ex.GetMachineByProviderID(ctx, providerID)
} else {
server, err = ex.getMachineByName(ctx, machineName)
server, err = ex.GetMachineByName(ctx, machineName)
}

if err == nil {
Expand Down Expand Up @@ -640,9 +639,10 @@ func (ex *Executor) deleteVolume(ctx context.Context, machineName string) error
return nil
}

// getMachineByProviderID fetches the data for a server based on a provider-encoded ID.
func (ex *Executor) getMachineByID(ctx context.Context, serverID string) (*servers.Server, error) {
klog.V(2).Infof("finding server with [ID=%q]", serverID)
// GetMachineByProviderID fetches the data for a server based on a provider-encoded ID.
func (ex *Executor) GetMachineByProviderID(ctx context.Context, providerID string) (*servers.Server, error) {
klog.V(2).Infof("finding server with [ID=%q]", providerID)
serverID := decodeProviderID(providerID)
server, err := ex.Compute.GetServer(ctx, serverID)
if err != nil {
klog.V(2).Infof("error finding server [ID=%q]: %v", serverID, err)
Expand All @@ -669,13 +669,13 @@ func (ex *Executor) getMachineByID(ctx context.Context, serverID string) (*serve
return nil, fmt.Errorf("could not find server [ID=%q]: %w", serverID, ErrNotFound)
}

// getMachineByName returns a server that matches the following criteria:
// GetMachineByName returns a server that matches the following criteria:
// a) has the same name as machineName
// b) has the cluster and role tags as set in the machineClass
// The current approach is weak because the tags are currently stored as server metadata. Later Nova versions allow
// to store tags in a respective field and do a server-side filtering. To avoid incompatibility with older versions
// we will continue making the filtering clientside.
func (ex *Executor) getMachineByName(ctx context.Context, machineName string) (*servers.Server, error) {
func (ex *Executor) GetMachineByName(ctx context.Context, machineName string) (*servers.Server, error) {
Copy link
Member

@aaronfern aaronfern Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please update this method to also return providerID? You can construct it using encodeProviderID()
You can then use this provider id in the GetMachineStatusResponse of GetMachineStatus()

Copy link
Contributor Author

@jamand jamand Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I get the exact change you want, GetMachineByName returns the machine (similar to GetMachineByProviderID) from which I in return extract the providerID of the machine that was found, so it is already part of the returned Server object, and used in the GetMachineStatusResponse.

searchClusterName, searchNodeRole, ok := findMandatoryTags(ex.Config.Spec.Tags)
if !ok {
klog.Warningf("getMachineByName operation can not proceed: cluster/role tags are missing for machine [Name=%q]", machineName)
Expand Down
Loading