Skip to content

NO-JIRA: conversion: CAPI2MAPI set openshift labels for zone, region and instance-type #393

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
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

NO-JIRA: conversion: CAPI2MAPI set openshift labels for zone, region and instance-type #393

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
e9dab7d
conversion: capi2mapi set mapi labels and annotations for instance-ty…
chrischdi Oct 21, 2025
3ef3094
fix vap tests to add the sentinel not replace all labels
chrischdi Oct 21, 2025
3a3b740
ignore mapi labels when converting to CAPI
chrischdi Oct 21, 2025
ad801fd
conversion: fix fuzz tests to consider labels
chrischdi Oct 21, 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
5 changes: 4 additions & 1 deletion pkg/controllers/machinesync/machine_sync_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,10 @@ var _ = Describe("With a running MachineSync Reconciler", func() {
HaveField("Status.AuthoritativeAPI", Equal(mapiv1beta1.MachineAuthorityClusterAPI)))

Eventually(k.Update(testMachine, func() {
testMachine.ObjectMeta.Labels = map[string]string{"test-sentinel": "fubar"}
if testMachine.ObjectMeta.Labels == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yep, this is now needed as otherwise you're removing a protected label by putting the sentinel in :)

testMachine.ObjectMeta.Labels = map[string]string{}
}
testMachine.ObjectMeta.Labels["test-sentinel"] = "fubar"
}), timeout).Should(MatchError(ContainSubstring("policy in place")))
})
Context("with status.AuthoritativeAPI: Machine API", func() {
Expand Down
25 changes: 20 additions & 5 deletions pkg/conversion/capi2mapi/aws.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,10 @@ const (

// machineAndAWSMachineAndAWSCluster stores the details of a Cluster API Machine and AWSMachine and AWSCluster.
type machineAndAWSMachineAndAWSCluster struct {
machine *clusterv1.Machine
awsMachine *awsv1.AWSMachine
awsCluster *awsv1.AWSCluster
machine *clusterv1.Machine
awsMachine *awsv1.AWSMachine
awsCluster *awsv1.AWSCluster
excludeMachineAPILabelsAndAnnotations bool
}

// machineSetAndAWSMachineTemplateAndAWSCluster stores the details of a Cluster API MachineSet and AWSMachineTemplate and AWSCluster.
Expand Down Expand Up @@ -84,7 +85,8 @@ func FromMachineSetAndAWSMachineTemplateAndAWSCluster(ms *clusterv1.MachineSet,
awsMachine: &awsv1.AWSMachine{
Spec: mts.Spec.Template.Spec,
},
awsCluster: ac,
awsCluster: ac,
excludeMachineAPILabelsAndAnnotations: true,
},
}
}
Expand Down Expand Up @@ -258,7 +260,20 @@ func (m machineAndAWSMachineAndAWSCluster) ToMachine() (*mapiv1beta1.Machine, []

warnings = append(warnings, warn...)

mapiMachine, err := fromCAPIMachineToMAPIMachine(m.machine)
var additionalMachineAPILabels, additionalMachineAPIAnnotations map[string]string
if !m.excludeMachineAPILabelsAndAnnotations {
additionalMachineAPILabels = map[string]string{
"machine.openshift.io/instance-type": m.awsMachine.Spec.InstanceType,
"machine.openshift.io/region": m.awsCluster.Spec.Region,
"machine.openshift.io/zone": ptr.Deref(m.machine.Spec.FailureDomain, ""),
}

additionalMachineAPIAnnotations = map[string]string{
"machine.openshift.io/instance-state": string(ptr.Deref(m.awsMachine.Status.InstanceState, "")),
}
}

mapiMachine, err := fromCAPIMachineToMAPIMachine(m.machine, additionalMachineAPILabels, additionalMachineAPIAnnotations)
if err != nil {
errors = append(errors, err...)
}
Expand Down
32 changes: 28 additions & 4 deletions pkg/conversion/capi2mapi/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,13 @@ func RawExtensionFromInterface(spec interface{}) (*runtime.RawExtension, error)

func convertCAPIMachineSetSelectorToMAPI(capiSelector metav1.LabelSelector) metav1.LabelSelector {
mapiSelector := capiSelector.DeepCopy()
mapiSelector.MatchLabels = convertCAPILabelsToMAPILabels(capiSelector.MatchLabels)
mapiSelector.MatchLabels = convertCAPILabelsToMAPILabels(capiSelector.MatchLabels, nil)

return *mapiSelector
}

func convertCAPILabelsToMAPILabels(capiLabels map[string]string) map[string]string {
if len(capiLabels) == 0 {
func convertCAPILabelsToMAPILabels(capiLabels map[string]string, machineAPILabels map[string]string) map[string]string {
if len(capiLabels) == 0 && len(machineAPILabels) == 0 {
return nil
}

Expand All @@ -77,6 +77,15 @@ func convertCAPILabelsToMAPILabels(capiLabels map[string]string) map[string]stri
mapiLabels[k] = v
}

for k, v := range machineAPILabels {
// Ignore empty labels to ensure to not overwrite potentially existing labels with empty values.
if v == "" {
continue
}

mapiLabels[k] = v
}

// On the original MAPI object some label fields are nil rather than empty.
// So return nil instead to avoid unnecessary diff being picked up by the diff checker.
if len(mapiLabels) == 0 {
Expand Down Expand Up @@ -124,7 +133,7 @@ func convertCAPIMachineAnnotationsToMAPIMachineSpecObjectMetaAnnotations(capiAnn
return mapiAnnotations
}

func convertCAPIAnnotationsToMAPIAnnotations(capiAnnotations map[string]string) map[string]string {
func convertCAPIAnnotationsToMAPIAnnotations(capiAnnotations map[string]string, machineAPIAnnotations map[string]string) map[string]string {
if len(capiAnnotations) == 0 {
return nil
}
Expand Down Expand Up @@ -155,5 +164,20 @@ func convertCAPIAnnotationsToMAPIAnnotations(capiAnnotations map[string]string)
mapiAnnotations[k] = v
}

for k, v := range machineAPIAnnotations {
// Ignore empty annotations to ensure to not overwrite potentially existing annotations with empty values.
if v == "" {
continue
}

mapiAnnotations[k] = v
}

// On the original MAPI object some label fields are nil rather than empty.
// So return nil instead to avoid unnecessary diff being picked up by the diff checker.
if len(mapiAnnotations) == 0 {
return nil
}

return mapiAnnotations
}
6 changes: 3 additions & 3 deletions pkg/conversion/capi2mapi/machine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const (
)

// fromCAPIMachineToMAPIMachine translates a core CAPI Machine to its MAPI Machine correspondent.
func fromCAPIMachineToMAPIMachine(capiMachine *clusterv1.Machine) (*mapiv1beta1.Machine, field.ErrorList) {
func fromCAPIMachineToMAPIMachine(capiMachine *clusterv1.Machine, additionalMachineAPILabels, additionalMachineAPIAnnotations map[string]string) (*mapiv1beta1.Machine, field.ErrorList) {
errs := field.ErrorList{}

lifecycleHooks, capiMachineNonHookAnnotations := convertCAPILifecycleHookAnnotationsToMAPILifecycleHooksAndAnnotations(capiMachine.Annotations)
Expand All @@ -48,8 +48,8 @@ func fromCAPIMachineToMAPIMachine(capiMachine *clusterv1.Machine) (*mapiv1beta1.
ObjectMeta: metav1.ObjectMeta{
Name: capiMachine.Name,
Namespace: mapiNamespace,
Labels: convertCAPILabelsToMAPILabels(capiMachine.Labels),
Annotations: convertCAPIAnnotationsToMAPIAnnotations(capiMachineNonHookAnnotations),
Labels: convertCAPILabelsToMAPILabels(capiMachine.Labels, additionalMachineAPILabels),
Annotations: convertCAPIAnnotationsToMAPIAnnotations(capiMachineNonHookAnnotations, additionalMachineAPIAnnotations),
Finalizers: []string{mapiv1beta1.MachineFinalizer},
OwnerReferences: nil, // OwnerReferences not populated here. They are added later by the machineSync controller.
},
Expand Down
8 changes: 4 additions & 4 deletions pkg/conversion/capi2mapi/machineset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ func fromCAPIMachineSetToMAPIMachineSet(capiMachineSet *clusterv1.MachineSet) (*
ObjectMeta: metav1.ObjectMeta{
Name: capiMachineSet.Name,
Namespace: capiMachineSet.Namespace,
Labels: convertCAPILabelsToMAPILabels(capiMachineSet.Labels),
Annotations: convertCAPIAnnotationsToMAPIAnnotations(capiMachineSet.Annotations),
Labels: convertCAPILabelsToMAPILabels(capiMachineSet.Labels, nil),
Annotations: convertCAPIAnnotationsToMAPIAnnotations(capiMachineSet.Annotations, nil),
Finalizers: nil, // MAPI MachineSet does not have finalizers.
OwnerReferences: nil, // OwnerReferences not populated here. They are added later by the machineSetSync controller.
},
Expand All @@ -44,8 +44,8 @@ func fromCAPIMachineSetToMAPIMachineSet(capiMachineSet *clusterv1.MachineSet) (*
DeletePolicy: capiMachineSet.Spec.DeletePolicy,
Template: mapiv1beta1.MachineTemplateSpec{
ObjectMeta: mapiv1beta1.ObjectMeta{
Labels: convertCAPILabelsToMAPILabels(capiMachineSet.Spec.Template.Labels),
Annotations: convertCAPIAnnotationsToMAPIAnnotations(capiMachineSet.Spec.Template.Annotations),
Labels: convertCAPILabelsToMAPILabels(capiMachineSet.Spec.Template.Labels, nil),
Annotations: convertCAPIAnnotationsToMAPIAnnotations(capiMachineSet.Spec.Template.Annotations, nil),
},
},
},
Expand Down
28 changes: 23 additions & 5 deletions pkg/conversion/capi2mapi/openstack.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/utils/ptr"
openstackv1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
)
Expand All @@ -39,9 +40,10 @@ var (

// machineAndOpenStackMachineAndOpenStackCluster stores the details of a Cluster API Machine and OpenStackMachine and OpenStackCluster.
type machineAndOpenStackMachineAndOpenStackCluster struct {
machine *clusterv1.Machine
openstackMachine *openstackv1.OpenStackMachine
openstackCluster *openstackv1.OpenStackCluster
machine *clusterv1.Machine
openstackMachine *openstackv1.OpenStackMachine
openstackCluster *openstackv1.OpenStackCluster
excludeMachineAPILabelsAndAnnotations bool
}

// machineSetAndOpenStackMachineTemplateAndOpenStackCluster stores the details of a Cluster API MachineSet and OpenStackMachineTemplate and OpenStackCluster.
Expand Down Expand Up @@ -74,7 +76,8 @@ func FromMachineSetAndOpenStackMachineTemplateAndOpenStackCluster(ms *clusterv1.
openstackMachine: &openstackv1.OpenStackMachine{
Spec: mts.Spec.Template.Spec,
},
openstackCluster: ac,
openstackCluster: ac,
excludeMachineAPILabelsAndAnnotations: true,
},
}
}
Expand Down Expand Up @@ -201,7 +204,22 @@ func (m machineAndOpenStackMachineAndOpenStackCluster) ToMachine() (*mapiv1beta1

warnings = append(warnings, warns...)

mapiMachine, errs := fromCAPIMachineToMAPIMachine(m.machine)
var additionalMachineAPILabels, additionalMachineAPIAnnotations map[string]string
if !m.excludeMachineAPILabelsAndAnnotations {
additionalMachineAPILabels = map[string]string{
// Unable to determine the region without fetching the identity resources as done at:
// https://github.com/openshift/machine-api-provider-openstack/blob/2defb131bd0836beba0a9790de7c9a63137a5cec/pkg/machine/actuator.go#L85-L89
// "machine.openshift.io/region":
"machine.openshift.io/instance-type": ptr.Deref(m.openstackMachine.Spec.Flavor, ""),
"machine.openshift.io/zone": ptr.Deref(m.machine.Spec.FailureDomain, ""),
}

additionalMachineAPIAnnotations = map[string]string{
"machine.openshift.io/instance-state": string(ptr.Deref(m.openstackMachine.Status.InstanceState, "")),
}
}

mapiMachine, errs := fromCAPIMachineToMAPIMachine(m.machine, additionalMachineAPILabels, additionalMachineAPIAnnotations)
if errs != nil {
errors = append(errors, errs...)
}
Expand Down
25 changes: 20 additions & 5 deletions pkg/conversion/capi2mapi/powervs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@ var (

// machineAndPowerVSMachineAndPowerVSCluster stores the details of a Cluster API Machine and PowerVSMachine and PowerVSCluster.
type machineAndPowerVSMachineAndPowerVSCluster struct {
machine *clusterv1.Machine
powerVSMachine *ibmpowervsv1.IBMPowerVSMachine
powerVSCluster *ibmpowervsv1.IBMPowerVSCluster
machine *clusterv1.Machine
powerVSMachine *ibmpowervsv1.IBMPowerVSMachine
powerVSCluster *ibmpowervsv1.IBMPowerVSCluster
excludeMachineAPILabelsAndAnnotations bool
}

// machineSetAndPowerVSMachineTemplateAndPowerVSCluster stores the details of a Cluster API MachineSet and PowerVSMachineTemplate and AWSCluster.
Expand Down Expand Up @@ -74,7 +75,8 @@ func FromMachineSetAndPowerVSMachineTemplateAndPowerVSCluster(ms *clusterv1.Mach
powerVSMachine: &ibmpowervsv1.IBMPowerVSMachine{
Spec: mts.Spec.Template.Spec,
},
powerVSCluster: pc,
powerVSCluster: pc,
excludeMachineAPILabelsAndAnnotations: true,
},
}
}
Expand All @@ -100,7 +102,20 @@ func (m machineAndPowerVSMachineAndPowerVSCluster) ToMachine() (*mapiv1beta1.Mac
return nil, nil, fmt.Errorf("unable to convert PowerVS providerSpec to raw extension: %w", errRaw)
}

mapiMachine, err := fromCAPIMachineToMAPIMachine(m.machine)
var additionalMachineAPILabels, additionalMachineAPIAnnotations map[string]string
if !m.excludeMachineAPILabelsAndAnnotations {
additionalMachineAPILabels = map[string]string{
"machine.openshift.io/instance-type": m.powerVSMachine.Spec.SystemType,
"machine.openshift.io/region": ptr.Deref(m.powerVSMachine.Status.Region, ""),
"machine.openshift.io/zone": ptr.Deref(m.machine.Spec.FailureDomain, ""),
}

additionalMachineAPIAnnotations = map[string]string{
"machine.openshift.io/instance-state": string(m.powerVSMachine.Status.InstanceState),
}
}

mapiMachine, err := fromCAPIMachineToMAPIMachine(m.machine, additionalMachineAPILabels, additionalMachineAPIAnnotations)
if err != nil {
errors = append(errors, err...)
}
Expand Down
Loading