Skip to content

OCPCLOUD-2993: support network interface and no MAPI ebs volumesize conversion #431

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

Closed
huali9 wants to merge 1 commit into from
Closed

OCPCLOUD-2993: support network interface and no MAPI ebs volumesize conversion #431

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
50 changes: 50 additions & 0 deletions e2e/machineset_migration_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,3 +365,53 @@ func cleanupMachineSetTestResources(ctx context.Context, cl client.Client, capiM
capiframework.DeleteAWSMachineTemplates(ctx, cl, template)
}
}

// createMAPIMachineSetWithProviderSpecUpdates creates a MAPI MachineSet with custom AWS provider spec updates.
// The updateFunc allows callers to customize any field in the AWS provider spec.
func createMAPIMachineSetWithProviderSpecUpdates(
ctx context.Context,
cl client.Client,
replicas int,
machineSetName string,
updateFunc func(*mapiv1beta1.AWSMachineProviderConfig),
) *mapiv1beta1.MachineSet {
By(fmt.Sprintf("Creating MAPI MachineSet %s with custom provider spec", machineSetName))

// Build base MachineSet params
machineSetParams := mapiframework.BuildMachineSetParams(ctx, cl, replicas)
machineSetParams.Name = machineSetName
machineSetParams.Labels[mapiframework.MachineSetKey] = machineSetName
machineSetParams.MachinesetAuthoritativeAPI = mapiv1beta1.MachineAuthorityMachineAPI
machineSetParams.MachineAuthoritativeAPI = mapiv1beta1.MachineAuthorityMachineAPI
machineSetParams.Taints = []corev1.Taint{}

// Get and update the provider spec
providerSpec := &mapiv1beta1.AWSMachineProviderConfig{}
Expect(yaml.Unmarshal(machineSetParams.ProviderSpec.Value.Raw, providerSpec)).To(Succeed())

// Apply custom updates to the provider spec
if updateFunc != nil {
updateFunc(providerSpec)
}

// Marshal the updated provider spec back
rawProviderSpec, err := json.Marshal(providerSpec)
Expect(err).ToNot(HaveOccurred(), "failed to marshal updated provider spec")
machineSetParams.ProviderSpec.Value.Raw = rawProviderSpec

// Create the MachineSet
mapiMachineSet, err := mapiframework.CreateMachineSet(cl, machineSetParams)
Expect(err).ToNot(HaveOccurred(), "MAPI MachineSet %s creation should succeed", machineSetName)

// Wait for CAPI mirror to be created
capiMachineSet := &clusterv1beta1.MachineSet{
ObjectMeta: metav1.ObjectMeta{
Name: machineSetName,
Namespace: capiframework.CAPINamespace,
},
}
Eventually(komega.Get(capiMachineSet), capiframework.WaitShort, capiframework.RetryShort).Should(
Succeed(), "Should have mirror CAPI MachineSet created")

return mapiMachineSet
}
134 changes: 134 additions & 0 deletions e2e/machineset_migration_mapi_authoritative_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"k8s.io/utils/ptr"

configv1 "github.com/openshift/api/config/v1"
mapiv1beta1 "github.com/openshift/api/machine/v1beta1"
Expand Down Expand Up @@ -377,4 +378,137 @@ var _ = Describe("[sig-cluster-lifecycle][OCPFeatureGate:MachineAPIMigration] Ma
})
})
})

var _ = Describe("Convert MAPI MachineSets with AWS special fields", Ordered, func() {
var mapiMSSpecialFieldsName = "ms-with-aws-special-fields"
var mapiMachineSet *mapiv1beta1.MachineSet
var capiMachineSet *clusterv1beta1.MachineSet
var awsMachineTemplate *awsv1.AWSMachineTemplate

Context("with networkInterfaceType ENA and blockDevices without volumeSize", Ordered, func() {
BeforeAll(func() {
By("Creating MAPI MachineSet with networkInterfaceType: ENA and blockDevices without volumeSize")

// Use the extensible helper function with a custom update function
mapiMachineSet = createMAPIMachineSetWithProviderSpecUpdates(ctx, cl, 1, mapiMSSpecialFieldsName,
func(providerSpec *mapiv1beta1.AWSMachineProviderConfig) {
// Set networkInterfaceType to ENA
providerSpec.NetworkInterfaceType = "ENA"
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Dec 16, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the test file to see the context around the mentioned lines
cd /tmp && find . -name "machineset_migration_mapi_authoritative_test.go" 2>/dev/null | head -1

Repository: openshift/cluster-capi-operator

Length of output: 57

🏁 Script executed:

# List files to understand repo structure
git ls-files "**/*machineset_migration*" 2>/dev/null

Repository: openshift/cluster-capi-operator

Length of output: 361

🏁 Script executed:

# Search for the test file
find . -type f -name "*machineset_migration_mapi_authoritative_test.go" 2>/dev/null

Repository: openshift/cluster-capi-operator

Length of output: 128

🏁 Script executed:

# Get NetworkInterfaceType related constants and conversion logic
rg -n "NetworkInterfaceType" --type=go -A 3 -B 1 | head -150

Repository: openshift/cluster-capi-operator

Length of output: 8980

🏁 Script executed:

# Search for ENA, EFA, ENI definitions
rg -n "const.*NetworkInterfaceType" --type=go -A 5

Repository: openshift/cluster-capi-operator

Length of output: 57

🏁 Script executed:

# Look for conversion or mapping logic between network interface types
rg -n "(ENA|EFA|ENI).*NetworkInterface|NetworkInterface.*(ENA|EFA|ENI)" --type=go

Repository: openshift/cluster-capi-operator

Length of output: 2378

🏁 Script executed:

cat -n e2e/machineset_migration_mapi_authoritative_test.go | sed -n '460,495p'

Repository: openshift/cluster-capi-operator

Length of output: 2258

🏁 Script executed:

# Also check if there are any constants defined for the network interface types
rg -n "AWSENANetworkInterfaceType|AWSEFANetworkInterfaceType" --type=go -B 2 -A 1

Repository: openshift/cluster-capi-operator

Length of output: 2047

🏁 Script executed:

cat -n e2e/machineset_migration_mapi_authoritative_test.go | sed -n '385,400p'

Repository: openshift/cluster-capi-operator

Length of output: 998

🏁 Script executed:

# Check if there's a PR or issue reference for OCP-81294
rg -n "OCP-81294|81294" --type=go --type=md

Repository: openshift/cluster-capi-operator

Length of output: 57

Fix assertion message on line 489 to match the expected value.

The test correctly sets NetworkInterfaceType to "ENA" in the MAPI MachineSet (line 396), which is intentionally converted to NetworkInterfaceTypeENI in CAPI per the conversion logic in pkg/conversion/mapi2capi/aws.go (lines 695-696). The assertions on lines 463 and 488 correctly expect NetworkInterfaceTypeENI. However, the assertion message on line 489 says "AWSMachine should have networkInterfaceType set to ENA" when the actual check is for NetworkInterfaceTypeENI. Update the message to say "set to ENI" to match the assertion.

🤖 Prompt for AI Agents 
In e2e/machineset_migration_mapi_authoritative_test.go around lines 396–396 and
specifically fix the assertion message at line 489: the test sets
providerSpec.NetworkInterfaceType = "ENA" (line 396) which the conversion maps
to NetworkInterfaceTypeENI, and the assertion at line 488 correctly checks for
NetworkInterfaceTypeENI but the message on line 489 mistakenly says "set to
ENA"; update that assertion message to say "set to ENI" so the message matches
the expected value being asserted.


// Set blockDevices with encrypted=true, volumeType=gp3, but NO volumeSize
providerSpec.BlockDevices = []mapiv1beta1.BlockDeviceMappingSpec{
{
EBS: &mapiv1beta1.EBSBlockDeviceSpec{
Encrypted: ptr.To(true),
Iops: ptr.To[int64](0),
VolumeType: ptr.To("gp3"),
KMSKey: mapiv1beta1.AWSResourceReference{
ARN: ptr.To(""),
},
// Intentionally omit VolumeSize to test conversion
},
},
}
},
)

capiMachineSet, awsMachineTemplate = waitForMAPIMachineSetMirrors(cl, mapiMSSpecialFieldsName)

DeferCleanup(func() {
By("Cleaning up Context 'with networkInterfaceType ENA and blockDevices without volumeSize' resources")
cleanupMachineSetTestResources(
ctx,
cl,
[]*clusterv1beta1.MachineSet{capiMachineSet},
[]*awsv1.AWSMachineTemplate{awsMachineTemplate},
[]*mapiv1beta1.MachineSet{mapiMachineSet},
)
})
})

It("should find MAPI MachineSet .status.authoritativeAPI to equal MachineAPI", func() {
verifyMachineSetAuthoritative(mapiMachineSet, mapiv1beta1.MachineAuthorityMachineAPI)
})

It("should verify MAPI MachineSet Synchronized condition is True", func() {
verifyMAPIMachineSetSynchronizedCondition(mapiMachineSet, mapiv1beta1.MachineAuthorityMachineAPI)
})

It("should verify networkInterfaceType: ENA is preserved in MAPI MachineSet", func() {
freshMAPIMS, err := mapiframework.GetMachineSet(ctx, cl, mapiMSSpecialFieldsName)
Expect(err).ToNot(HaveOccurred())
providerSpec := getAWSProviderSpecFromMachineSet(freshMAPIMS)
Expect(string(providerSpec.NetworkInterfaceType)).To(Equal("ENA"))
})

It("should verify blockDevices without volumeSize are preserved in MAPI MachineSet", func() {
freshMAPIMS, err := mapiframework.GetMachineSet(ctx, cl, mapiMSSpecialFieldsName)
Expect(err).ToNot(HaveOccurred())
providerSpec := getAWSProviderSpecFromMachineSet(freshMAPIMS)

Expect(providerSpec.BlockDevices).To(HaveLen(1), "Should have one block device")
Expect(providerSpec.BlockDevices[0].EBS).NotTo(BeNil(), "Block device should have EBS config")
Expect(*providerSpec.BlockDevices[0].EBS.Encrypted).To(BeTrue(), "Should be encrypted")
Expect(*providerSpec.BlockDevices[0].EBS.VolumeType).To(Equal("gp3"), "Should have volumeType gp3")
Expect(providerSpec.BlockDevices[0].EBS.VolumeSize).To(BeNil(), "VolumeSize should be nil")
Expect(*providerSpec.BlockDevices[0].EBS.Iops).To(BeZero(), "IOPS should be zero")
Expect(*providerSpec.BlockDevices[0].EBS.KMSKey.ARN).To(BeEmpty(), "KMS ARN should be empty")
})

It("should verify AWS special fields are converted to CAPI AWSMachineTemplate", func() {
freshAWSMachineTemplate, err := capiframework.GetAWSMachineTemplateByName(cl, awsMachineTemplate.Name, capiframework.CAPINamespace)
Expect(err).ToNot(HaveOccurred())

By("Verifying networkInterfaceType: ENA is converted")
Expect(freshAWSMachineTemplate.Spec.Template.Spec.NetworkInterfaceType).To(Equal(awsv1.NetworkInterfaceTypeENI))

By("Verifying blockDevices volumeSize are defaulted to 120")
Expect(freshAWSMachineTemplate.Spec.Template.Spec.RootVolume).NotTo(BeNil())
Expect(*freshAWSMachineTemplate.Spec.Template.Spec.RootVolume.Encrypted).To(BeTrue())
Expect(freshAWSMachineTemplate.Spec.Template.Spec.RootVolume.Type).To(Equal(awsv1.VolumeTypeGP3))
Expect(freshAWSMachineTemplate.Spec.Template.Spec.RootVolume.Size).To(BeNumerically("==", 120))
})

It("should create a Machine with AWS special fields converted correctly", func() {
mapiframework.WaitForMachineSet(ctx, cl, mapiMSSpecialFieldsName)
verifyMachinesetReplicas(mapiMachineSet, 1)
verifyMachinesetReplicas(capiMachineSet, 1)

By("Getting the created MAPI Machine")
mapiMachine, err := mapiframework.GetLatestMachineFromMachineSet(ctx, cl, mapiMachineSet)
Expect(err).ToNot(HaveOccurred())
verifyMachineRunning(cl, mapiMachine)

By("Getting the CAPI Machine mirror and AWSMachine")
capiMachine := capiframework.GetNewestMachineFromMachineSet(cl, capiMachineSet)
awsMachine := capiframework.GetAWSMachine(cl, capiMachine.Name, capiframework.CAPINamespace)

By("Verifying AWSMachine has networkInterfaceType: ENA")
Eventually(k.Object(awsMachine), capiframework.WaitMedium, capiframework.RetryMedium).Should(
HaveField("Spec.NetworkInterfaceType", Equal(awsv1.NetworkInterfaceTypeENI)),
"AWSMachine should have networkInterfaceType set to ENA",
)

By("Verifying AWSMachine has correct blockDevice configuration")
Eventually(func() bool {
freshAWSMachine := capiframework.GetAWSMachine(cl, awsMachine.Name, capiframework.CAPINamespace)
if freshAWSMachine.Spec.RootVolume == nil {
return false
}
if freshAWSMachine.Spec.RootVolume.Encrypted == nil || !*freshAWSMachine.Spec.RootVolume.Encrypted {
return false
}
if freshAWSMachine.Spec.RootVolume.Type != awsv1.VolumeTypeGP3 {
return false
}
if freshAWSMachine.Spec.RootVolume.Size != int64(120) {
return false
}
return true
}, capiframework.WaitMedium, capiframework.RetryMedium).Should(BeTrue(),
"AWSMachine should have blockDevice configuration from MAPI MachineSet",
)
})
})
})
})