Skip to content

OCPBUGS-9925: Fix tests for OpenStack platform #306

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

Merged
merged 2 commits into from
Jan 18, 2024
Merged

OCPBUGS-9925: Fix tests for OpenStack platform #306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7047cdf
Move cleanup to the DeferCleanup function
mandre Dec 20, 2023
d009a74
Skip webhook test on unsupported platforms
mandre Dec 21, 2023
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
35 changes: 18 additions & 17 deletions pkg/autoscaler/autoscaler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,27 +148,28 @@ var _ = Describe("Autoscaler should", framework.LabelAutoscaler, Serial, func()

// Anything we create we must cleanup
cleanupObjects = make(map[string]runtimeclient.Object)
})

AfterEach(func() {
var machineSets []*machinev1.MachineSet
// Make sure to clean up the resources we created
DeferCleanup(func() {
var machineSets []*machinev1.MachineSet

for name, obj := range cleanupObjects {
if machineSet, ok := obj.(*machinev1.MachineSet); ok {
// Once we delete a MachineSet we should make sure that the
// all of its machines are deleted as well.
// Collect MachineSets to wait for.
machineSets = append(machineSets, machineSet)
}
for name, obj := range cleanupObjects {
if machineSet, ok := obj.(*machinev1.MachineSet); ok {
// Once we delete a MachineSet we should make sure that the
// all of its machines are deleted as well.
// Collect MachineSets to wait for.
machineSets = append(machineSets, machineSet)
}

Expect(deleteObject(name, obj)).To(Succeed(), "Failed to delete object %v", name)
}
Expect(deleteObject(name, obj)).To(Succeed(), "Failed to delete object %v", name)
}

if len(machineSets) > 0 {
// Wait for all MachineSets and their Machines to be deleted.
By("Waiting for MachineSets to be deleted...")
framework.WaitForMachineSetsDeleted(ctx, client, machineSets...)
}
if len(machineSets) > 0 {
// Wait for all MachineSets and their Machines to be deleted.
By("Waiting for MachineSets to be deleted...")
framework.WaitForMachineSetsDeleted(ctx, client, machineSets...)
}
})
})

Context("use a ClusterAutoscaler that has 100 maximum total nodes count", framework.LabelPeriodic, func() {
Expand Down
32 changes: 25 additions & 7 deletions pkg/infra/infra.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

configv1 "github.com/openshift/api/config/v1"
machinev1 "github.com/openshift/api/machine/v1beta1"
corev1 "k8s.io/api/core/v1"
policyv1 "k8s.io/api/policy/v1"
Expand Down Expand Up @@ -165,20 +166,24 @@ var _ = Describe("Managed cluster should", framework.LabelMachines, func() {
client, err = framework.LoadClient()
Expect(err).ToNot(HaveOccurred(), "Controller-runtime client should be able to be created")

// Reset the machineSet between each test
machineSet = nil

// Make sure to clean up the resources we created
DeferCleanup(func() {
if machineSet != nil {
By("Deleting the new MachineSet")
Expect(client.Delete(ctx, machineSet)).To(Succeed(), "MachineSet should be able to be deleted")
framework.WaitForMachineSetsDeleted(ctx, client, machineSet)
}
})
})

AfterEach(func() {
specReport := CurrentSpecReport()
if specReport.Failed() {
Expect(gatherer.WithSpecReport(specReport).GatherAll()).To(Succeed(), "StateGatherer should be able to gather resources")
}

if machineSet != nil {
By("Deleting the new MachineSet")
Expect(client.Delete(ctx, machineSet)).To(Succeed(), "MachineSet should be able to be deleted")
framework.WaitForMachineSetsDeleted(ctx, client, machineSet)
}

})

When("machineset has one replica", func() {
Expand Down Expand Up @@ -382,6 +387,19 @@ var _ = Describe("Managed cluster should", framework.LabelMachines, func() {
// Machines required for test: 0
// Reason: The machineSet creation is rejected by the webhook.
It("reject invalid machinesets", func() {
client, err := framework.LoadClient()
Expect(err).ToNot(HaveOccurred(), "Controller-runtime client should be able to be created")
// Only run on platforms that have webhooks
clusterInfra, err := framework.GetInfrastructure(ctx, client)
Expect(err).NotTo(HaveOccurred(), "Should be able to get Infrastructure")
platform := clusterInfra.Status.PlatformStatus.Type
switch platform {
case configv1.AWSPlatformType, configv1.AzurePlatformType, configv1.GCPPlatformType, configv1.VSpherePlatformType, configv1.PowerVSPlatformType, configv1.NutanixPlatformType:
// Do Nothing
default:
Skip(fmt.Sprintf("Platform %s does not have webhooks, skipping.", platform))
}

By("Creating invalid machineset")
invalidMachineSet := invalidMachinesetWithEmptyProviderConfig()
expectedAdmissionWebhookErr := "admission webhook \"default.machineset.machine.openshift.io\" denied the request: providerSpec.value: Required value: a value must be provided"
Expand Down
36 changes: 22 additions & 14 deletions pkg/infra/lifecyclehooks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,19 @@ var _ = Describe("Lifecycle Hooks should", framework.LabelMachines, func() {
// Create machine set
machineSet, err = framework.CreateMachineSet(client, machineSetParams)
Expect(err).ToNot(HaveOccurred(), "MachineSet should be able to be created")

// Make sure to clean up the machineSet, if we create one.
DeferCleanup(func() {
By("Deleting the machineset")
cascadeDelete := metav1.DeletePropagationForeground
Expect(client.Delete(context.Background(), machineSet, &runtimeclient.DeleteOptions{
PropagationPolicy: &cascadeDelete,
})).To(Succeed(), "MachineSet should be able to be deleted")

By("Waiting for the MachineSet to be deleted...")
framework.WaitForMachineSetsDeleted(ctx, client, machineSet)
})

// Wait for machine to be running
framework.WaitForMachineSet(ctx, client, machineSet.GetName())

Expand All @@ -71,6 +84,15 @@ var _ = Describe("Lifecycle Hooks should", framework.LabelMachines, func() {
})
Expect(client.Create(context.Background(), workload)).To(Succeed(), "Could not create workload job")

// Make sure to clean up the workload job, if we create one.
DeferCleanup(func() {
cascadeDelete := metav1.DeletePropagationForeground
By("Deleting workload job")
Expect(client.Delete(context.Background(), workload, &runtimeclient.DeleteOptions{
PropagationPolicy: &cascadeDelete,
})).To(Succeed(), "Workload job should be able to be deleted")
})

By("Waiting for job pod to start running on machine.")
Eventually(func() (bool, error) {
jobPodList, err := framework.GetPods(client, map[string]string{lifecycleHooksPodLabel: ""})
Expand All @@ -91,20 +113,6 @@ var _ = Describe("Lifecycle Hooks should", framework.LabelMachines, func() {
if specReport.Failed() {
Expect(gatherer.WithSpecReport(specReport).GatherAll()).To(Succeed(), "StateGatherer should be able to gather resources")
}

By("Deleting the machineset")
cascadeDelete := metav1.DeletePropagationForeground
Expect(client.Delete(context.Background(), machineSet, &runtimeclient.DeleteOptions{
PropagationPolicy: &cascadeDelete,
})).To(Succeed(), "MachineSet should be able to be deleted")

By("Waiting for the MachineSet to be deleted...")
framework.WaitForMachineSetsDeleted(ctx, client, machineSet)

By("Deleting workload job")
Expect(client.Delete(context.Background(), workload, &runtimeclient.DeleteOptions{
PropagationPolicy: &cascadeDelete,
})).To(Succeed(), "Workload job should be able to be deleted")
})

// Machines required for test: 1
Expand Down
41 changes: 22 additions & 19 deletions pkg/infra/spot.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,28 @@ var _ = Describe("Running on Spot", framework.LabelMachines, framework.LabelSpot
BeforeEach(func() {
delObjects = make(map[string]runtimeclient.Object)

// Make sure to clean up the resources we created
DeferCleanup(func() {
var machineSets []*machinev1.MachineSet

for _, obj := range delObjects {
if machineSet, ok := obj.(*machinev1.MachineSet); ok {
// Once we delete a MachineSet we should make sure that the
// all of its machines are deleted as well.
// Collect MachineSets to wait for.
machineSets = append(machineSets, machineSet)
}

Expect(deleteObject(client, obj)).To(Succeed(), "Should be able to cleanup test objects")
}

if len(machineSets) > 0 {
// Wait for all MachineSets and their Machines to be deleted.
By("Waiting for MachineSets to be deleted...")
framework.WaitForMachineSetsDeleted(ctx, client, machineSets...)
}
})

var err error

gatherer, err = framework.NewGatherer()
Expand Down Expand Up @@ -117,25 +139,6 @@ var _ = Describe("Running on Spot", framework.LabelMachines, framework.LabelSpot
if specReport.Failed() {
Expect(gatherer.WithSpecReport(specReport).GatherAll()).To(Succeed(), "StateGatherer should be able to gather resources")
}

var machineSets []*machinev1.MachineSet

for _, obj := range delObjects {
if machineSet, ok := obj.(*machinev1.MachineSet); ok {
// Once we delete a MachineSet we should make sure that the
// all of its machines are deleted as well.
// Collect MachineSets to wait for.
machineSets = append(machineSets, machineSet)
}

Expect(deleteObject(client, obj)).To(Succeed(), "Should be able to cleanup test objects")
}

if len(machineSets) > 0 {
// Wait for all MachineSets and their Machines to be deleted.
By("Waiting for MachineSets to be deleted...")
framework.WaitForMachineSetsDeleted(ctx, client, machineSets...)
}
})

// Machines required for test: 1
Expand Down
23 changes: 13 additions & 10 deletions pkg/infra/webhooks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,23 +70,26 @@ var _ = Describe("Webhooks", framework.LabelMachines, func() {
return framework.IsValidatingWebhookConfigurationSynced(ctx, client)
}, framework.WaitShort).Should(BeTrue(), "ValidingWebhookConfiguration must be synced before running these tests")
})

// Make sure to clean up the resources we created
DeferCleanup(func() {
machineSets, err := framework.GetMachineSets(client, testSelector)
Expect(err).ToNot(HaveOccurred(), "Should be able to list test MachineSets")
Expect(framework.DeleteMachineSets(client, machineSets...)).To(Succeed(), "Should be able to delete test MachineSets")
framework.WaitForMachineSetsDeleted(ctx, client, machineSets...)

machines, err := framework.GetMachines(ctx, client, testSelector)
Expect(err).ToNot(HaveOccurred(), "Should be able to get test Machines")
Expect(framework.DeleteMachines(ctx, client, machines...)).To(Succeed(), "Should be able to delete test Machines")
framework.WaitForMachinesDeleted(client, machines...)
})
})

AfterEach(func() {
specReport := CurrentSpecReport()
if specReport.Failed() {
Expect(gatherer.WithSpecReport(specReport).GatherAll()).To(Succeed(), "StateGatherer should be able to gather resources")
}

machineSets, err := framework.GetMachineSets(client, testSelector)
Expect(err).ToNot(HaveOccurred(), "Should be able to list test MachineSets")
Expect(framework.DeleteMachineSets(client, machineSets...)).To(Succeed(), "Should be able to delete test MachineSets")
framework.WaitForMachineSetsDeleted(ctx, client, machineSets...)

machines, err := framework.GetMachines(ctx, client, testSelector)
Expect(err).ToNot(HaveOccurred(), "Should be able to get test Machines")
Expect(framework.DeleteMachines(ctx, client, machines...)).To(Succeed(), "Should be able to delete test Machines")
framework.WaitForMachinesDeleted(client, machines...)
})

// Machines required for test: 1
Expand Down
19 changes: 11 additions & 8 deletions pkg/machinehealthcheck/machinehealthcheck.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,17 @@ var _ = Describe("MachineHealthCheck", framework.LabelMachineHealthChecks, func(
machineSet, err = framework.CreateMachineSet(client, machineSetParams)
Expect(err).ToNot(HaveOccurred(), "failed to create a new machineSet resource")

// Make sure to clean up the resources we created
DeferCleanup(func() {
By("Deleting the MachineHealthCheck resource")
Expect(client.Delete(context.Background(), machinehealthcheck)).To(Succeed(), "failed to delete MHC")

By("Deleting the new MachineSet")
Expect(client.Delete(context.Background(), machineSet)).To(Succeed(), "failed to delete machineSet")

framework.WaitForMachineSetsDeleted(ctx, client, machineSet)
})
Comment on lines +59 to +67
Copy link
Contributor

Choose a reason for hiding this comment

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

This should come before the WaitForMachineSet else it won't run if the wait fails


framework.WaitForMachineSet(ctx, client, machineSet.GetName())
})

Expand All @@ -63,14 +74,6 @@ var _ = Describe("MachineHealthCheck", framework.LabelMachineHealthChecks, func(
if specReport.Failed() {
Expect(gatherer.WithSpecReport(specReport).GatherAll()).To(Succeed(), "failed to gather spec report")
}

By("Deleting the MachineHealthCheck resource")
Expect(client.Delete(context.Background(), machinehealthcheck)).To(Succeed(), "failed to delete MHC")

By("Deleting the new MachineSet")
Expect(client.Delete(context.Background(), machineSet)).To(Succeed(), "failed to delete machineSet")

framework.WaitForMachineSetsDeleted(ctx, client, machineSet)
})

// Machines required for test: 3
Expand Down
13 changes: 8 additions & 5 deletions pkg/providers/aws.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,21 @@ var _ = Describe("MetadataServiceOptions", framework.LabelCloudProviderSpecific,
if platform != configv1.AWSPlatformType {
Skip(fmt.Sprintf("skipping AWS specific tests on %s", platform))
}

// Make sure to clean up the resources we created
DeferCleanup(func() {
Expect(framework.DeleteMachineSets(client, toDelete...)).To(Succeed())
toDelete = make([]*machinev1.MachineSet, 0, 3)

framework.WaitForMachineSetsDeleted(ctx, client, toDelete...)
})
})

AfterEach(func() {
specReport := CurrentSpecReport()
if specReport.Failed() {
Expect(gatherer.WithSpecReport(specReport).GatherAll()).To(Succeed())
}

Expect(framework.DeleteMachineSets(client, toDelete...)).To(Succeed())
toDelete = make([]*machinev1.MachineSet, 0, 3)

framework.WaitForMachineSetsDeleted(ctx, client, toDelete...)
})

createMachineSet := func(metadataAuth string) (*machinev1.MachineSet, error) {
Expand Down