Skip to content

Improve topology testing coverage #536

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
openshift-merge-bot merged 1 commit into from
Mar 31, 2025
Merged

Improve topology testing coverage #536

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
41 changes: 33 additions & 8 deletions tests/functional/base_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,18 @@ import (

. "github.com/onsi/gomega" //revive:disable:dot-imports

topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1"
ironicv1 "github.com/openstack-k8s-operators/ironic-operator/api/v1beta1"
ironic_pkg "github.com/openstack-k8s-operators/ironic-operator/pkg/ironic"
ironic_inspector_pkg "github.com/openstack-k8s-operators/ironic-operator/pkg/ironicinspector"
condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

ironicv1 "github.com/openstack-k8s-operators/ironic-operator/api/v1beta1"
condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition"
)

const (
Expand Down Expand Up @@ -655,9 +656,11 @@ func CreateUnstructured(rawObj map[string]interface{}) *unstructured.Unstructure
return unstructuredObj
}

// GetSampleTopologySpec - A sample (and opinionated) Topology Spec used to
// test Ironic components
func GetSampleTopologySpec(svcSelector string) map[string]interface{} {
// GetSampleTopologySpec - An opinionated Topology Spec sample used to
// test Service components. It returns both the user input representation
// in the form of map[string]string, and the Golang expected representation
// used in the test asserts.
func GetSampleTopologySpec(label string) (map[string]interface{}, []corev1.TopologySpreadConstraint) {
// Build the topology Spec
topologySpec := map[string]interface{}{
"topologySpreadConstraints": []map[string]interface{}{
Expand All @@ -667,13 +670,26 @@ func GetSampleTopologySpec(svcSelector string) map[string]interface{} {
"whenUnsatisfiable": "ScheduleAnyway",
"labelSelector": map[string]interface{}{
"matchLabels": map[string]interface{}{
"service": svcSelector,
"component": label,
},
},
},
},
}
return topologySpec
// Build the topologyObj representation
topologySpecObj := []corev1.TopologySpreadConstraint{
{
MaxSkew: 1,
TopologyKey: corev1.LabelHostname,
WhenUnsatisfiable: corev1.ScheduleAnyway,
LabelSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"component": label,
},
},
},
}
return topologySpec, topologySpecObj
}

// CreateTopology - Creates a Topology CR based on the spec passed as input
Expand All @@ -689,3 +705,12 @@ func CreateTopology(topology types.NamespacedName, spec map[string]interface{})
}
return th.CreateUnstructured(raw)
}

// GetTopology - Returns the referenced Topology
func GetTopology(name types.NamespacedName) *topologyv1.Topology {
instance := &topologyv1.Topology{}
Eventually(func(g Gomega) {
g.Expect(k8sClient.Get(ctx, name, instance)).Should(Succeed())
}, timeout, interval).Should(Succeed())
return instance
}
142 changes: 128 additions & 14 deletions tests/functional/ironic_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (

. "github.com/openstack-k8s-operators/lib-common/modules/common/test/helpers"

topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1"
ironicv1 "github.com/openstack-k8s-operators/ironic-operator/api/v1beta1"
"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
mariadb_test "github.com/openstack-k8s-operators/mariadb-operator/api/test/helpers"
Expand Down Expand Up @@ -227,11 +228,21 @@ var _ = Describe("Ironic controller", func() {
})

When("Ironic is created with topologyref", func() {
var topologyRef, topologyRefAlt *topologyv1.TopoRef
BeforeEach(func() {
// Build the topology Spec
topologySpec := GetSampleTopologySpec("ironic")
// Define the two topology references used in this test
topologyRef = &topologyv1.TopoRef{
Name: ironicNames.IronicTopologies[0].Name,
Namespace: ironicNames.IronicTopologies[0].Namespace,
}
topologyRefAlt = &topologyv1.TopoRef{
Name: ironicNames.IronicTopologies[1].Name,
Namespace: ironicNames.IronicTopologies[1].Namespace,
}
// Create Test Topologies
for _, t := range ironicNames.IronicTopologies {
// Build the topology Spec
topologySpec, _ := GetSampleTopologySpec(t.Name)
CreateTopology(t, topologySpec)
}
DeferCleanup(
Expand All @@ -254,7 +265,7 @@ var _ = Describe("Ironic controller", func() {
keystone.CreateKeystoneAPI(ironicNames.Namespace))
spec := GetDefaultIronicSpec()
spec["topologyRef"] = map[string]interface{}{
"name": ironicNames.IronicTopologies[0].Name,
"name": topologyRef.Name,
}
DeferCleanup(
th.DeleteInstance,
Expand Down Expand Up @@ -286,46 +297,96 @@ var _ = Describe("Ironic controller", func() {

It("sets topology in CR status", func() {
Eventually(func(g Gomega) {
tp := GetTopology(types.NamespacedName{
Name: topologyRef.Name,
Namespace: topologyRef.Namespace,
})
finalizers := tp.GetFinalizers()
g.Expect(finalizers).To(HaveLen(4))
ironicAPI := GetIronicAPI(ironicNames.APIName)
g.Expect(ironicAPI.Status.LastAppliedTopology).ToNot(BeNil())
g.Expect(ironicAPI.Status.LastAppliedTopology.Name).To(Equal(ironicNames.IronicTopologies[0].Name))
g.Expect(ironicAPI.Status.LastAppliedTopology).To(Equal(topologyRef))
g.Expect(finalizers).To(ContainElement(
fmt.Sprintf("openstack.org/ironicapi-%s", ironicAPI.Name)))

ironicConductor := GetIronicConductor(ironicNames.ConductorName)
g.Expect(ironicConductor.Status.LastAppliedTopology).ToNot(BeNil())
g.Expect(ironicConductor.Status.LastAppliedTopology.Name).To(Equal(ironicNames.IronicTopologies[0].Name))
g.Expect(ironicConductor.Status.LastAppliedTopology).To(Equal(topologyRef))
g.Expect(finalizers).To(ContainElement(
fmt.Sprintf("openstack.org/ironicconductor-%s", ironicConductor.Name)))

ironicInspector := GetIronicInspector(ironicNames.InspectorName)
g.Expect(ironicInspector.Status.LastAppliedTopology).ToNot(BeNil())
g.Expect(ironicInspector.Status.LastAppliedTopology.Name).To(Equal(ironicNames.IronicTopologies[0].Name))
g.Expect(ironicInspector.Status.LastAppliedTopology).To(Equal(topologyRef))
g.Expect(finalizers).To(ContainElement(
fmt.Sprintf("openstack.org/ironicinspector-%s", ironicInspector.Name)))
}, timeout, interval).Should(Succeed())
})
It("sets nodeSelector in resource specs", func() {
It("sets topology in resource specs", func() {
Eventually(func(g Gomega) {
_, expectedTopologySpecObj := GetSampleTopologySpec(topologyRef.Name)
g.Expect(th.GetDeployment(ironicNames.IronicName).Spec.Template.Spec.TopologySpreadConstraints).ToNot(BeNil())
g.Expect(th.GetDeployment(ironicNames.IronicName).Spec.Template.Spec.TopologySpreadConstraints).To(Equal(expectedTopologySpecObj))
g.Expect(th.GetDeployment(ironicNames.IronicName).Spec.Template.Spec.Affinity).To(BeNil())

g.Expect(th.GetStatefulSet(ironicNames.InspectorName).Spec.Template.Spec.TopologySpreadConstraints).ToNot(BeNil())
g.Expect(th.GetStatefulSet(ironicNames.InspectorName).Spec.Template.Spec.TopologySpreadConstraints).To(Equal(expectedTopologySpecObj))
g.Expect(th.GetStatefulSet(ironicNames.InspectorName).Spec.Template.Spec.Affinity).To(BeNil())

g.Expect(th.GetStatefulSet(ironicNames.ConductorName).Spec.Template.Spec.TopologySpreadConstraints).To(Equal(expectedTopologySpecObj))
g.Expect(th.GetStatefulSet(ironicNames.ConductorName).Spec.Template.Spec.TopologySpreadConstraints).ToNot(BeNil())
g.Expect(th.GetStatefulSet(ironicNames.ConductorName).Spec.Template.Spec.Affinity).To(BeNil())

g.Expect(th.GetDeployment(ironicNames.INAName).Spec.Template.Spec.TopologySpreadConstraints).ToNot(BeNil())
g.Expect(th.GetDeployment(ironicNames.INAName).Spec.Template.Spec.TopologySpreadConstraints).To(Equal(expectedTopologySpecObj))
g.Expect(th.GetDeployment(ironicNames.INAName).Spec.Template.Spec.Affinity).To(BeNil())
}, timeout, interval).Should(Succeed())
})
It("updates topology when the reference changes", func() {
expectedTopology := &topologyv1.TopoRef{
Name: ironicNames.IronicTopologies[1].Name,
Namespace: ironicNames.IronicTopologies[1].Namespace,
}
var finalizers []string
Eventually(func(g Gomega) {
ironic := GetIronic(ironicNames.IronicName)
ironic.Spec.TopologyRef.Name = ironicNames.IronicTopologies[1].Name
ironic.Spec.TopologyRef.Name = topologyRefAlt.Name
g.Expect(k8sClient.Update(ctx, ironic)).To(Succeed())
}, timeout, interval).Should(Succeed())

Eventually(func(g Gomega) {
tp := GetTopology(types.NamespacedName{
Name: expectedTopology.Name,
Namespace: expectedTopology.Namespace,
})
finalizers = tp.GetFinalizers()
g.Expect(finalizers).To(HaveLen(4))

ironicAPI := GetIronicAPI(ironicNames.APIName)
g.Expect(ironicAPI.Status.LastAppliedTopology).ToNot(BeNil())
g.Expect(ironicAPI.Status.LastAppliedTopology.Name).To(Equal(ironicNames.IronicTopologies[1].Name))
g.Expect(ironicAPI.Status.LastAppliedTopology).To(Equal(topologyRefAlt))
g.Expect(finalizers).To(ContainElement(
fmt.Sprintf("openstack.org/ironicapi-%s", ironicAPI.Name)))

ironicConductor := GetIronicConductor(ironicNames.ConductorName)
g.Expect(ironicConductor.Status.LastAppliedTopology).ToNot(BeNil())
g.Expect(ironicConductor.Status.LastAppliedTopology.Name).To(Equal(ironicNames.IronicTopologies[1].Name))
g.Expect(ironicConductor.Status.LastAppliedTopology).To(Equal(topologyRefAlt))
g.Expect(finalizers).To(ContainElement(
fmt.Sprintf("openstack.org/ironicconductor-%s", ironicConductor.Name)))

ironicInspector := GetIronicInspector(ironicNames.InspectorName)
g.Expect(ironicInspector.Status.LastAppliedTopology).ToNot(BeNil())
g.Expect(ironicInspector.Status.LastAppliedTopology.Name).To(Equal(ironicNames.IronicTopologies[1].Name))
g.Expect(ironicInspector.Status.LastAppliedTopology).To(Equal(topologyRefAlt))
g.Expect(finalizers).To(ContainElement(
fmt.Sprintf("openstack.org/ironicinspector-%s", ironicInspector.Name)))

// Get the previous topology and verify there are no finalizers
// anymore
tp = GetTopology(types.NamespacedName{
Name: topologyRef.Name,
Namespace: topologyRef.Namespace,
})
g.Expect(tp.GetFinalizers()).To(BeEmpty())
}, timeout, interval).Should(Succeed())
})
It("overrides topology when the reference changes", func() {
Expand All @@ -347,15 +408,57 @@ var _ = Describe("Ironic controller", func() {
}, timeout, interval).Should(Succeed())

Eventually(func(g Gomega) {
expectedTopology := &topologyv1.TopoRef{
Name: ironicNames.IronicTopologies[1].Name,
Namespace: ironicNames.IronicTopologies[1].Namespace,
}
tp := GetTopology(types.NamespacedName{
Name: expectedTopology.Name,
Namespace: expectedTopology.Namespace,
})
g.Expect(tp.GetFinalizers()).To(HaveLen(1))
finalizers := tp.GetFinalizers()
ironicAPI := GetIronicAPI(ironicNames.APIName)
g.Expect(ironicAPI.Status.LastAppliedTopology).ToNot(BeNil())
g.Expect(ironicAPI.Status.LastAppliedTopology.Name).To(Equal(ironicNames.IronicTopologies[1].Name))
g.Expect(ironicAPI.Status.LastAppliedTopology).To(Equal(expectedTopology))
g.Expect(finalizers).To(ContainElement(
fmt.Sprintf("openstack.org/ironicapi-%s", ironicAPI.Name)))
}, timeout, interval).Should(Succeed())

Eventually(func(g Gomega) {
expectedTopology := &topologyv1.TopoRef{
Name: ironicNames.IronicTopologies[2].Name,
Namespace: ironicNames.IronicTopologies[2].Namespace,
}
tp := GetTopology(types.NamespacedName{
Name: expectedTopology.Name,
Namespace: expectedTopology.Namespace,
})
g.Expect(tp.GetFinalizers()).To(HaveLen(1))
finalizers := tp.GetFinalizers()
ironicConductor := GetIronicConductor(ironicNames.ConductorName)
g.Expect(ironicConductor.Status.LastAppliedTopology).ToNot(BeNil())
g.Expect(ironicConductor.Status.LastAppliedTopology.Name).To(Equal(ironicNames.IronicTopologies[2].Name))
g.Expect(ironicConductor.Status.LastAppliedTopology).To(Equal(expectedTopology))
g.Expect(finalizers).To(ContainElement(
fmt.Sprintf("openstack.org/ironicconductor-%s", ironicConductor.Name)))
}, timeout, interval).Should(Succeed())

Eventually(func(g Gomega) {
expectedTopology := &topologyv1.TopoRef{
Name: ironicNames.IronicTopologies[3].Name,
Namespace: ironicNames.IronicTopologies[3].Namespace,
}
tp := GetTopology(types.NamespacedName{
Name: expectedTopology.Name,
Namespace: expectedTopology.Namespace,
})
g.Expect(tp.GetFinalizers()).To(HaveLen(1))
finalizers := tp.GetFinalizers()
ironicInspector := GetIronicInspector(ironicNames.InspectorName)
g.Expect(ironicInspector.Status.LastAppliedTopology).ToNot(BeNil())
g.Expect(ironicInspector.Status.LastAppliedTopology.Name).To(Equal(ironicNames.IronicTopologies[3].Name))
g.Expect(ironicInspector.Status.LastAppliedTopology).To(Equal(expectedTopology))
g.Expect(finalizers).To(ContainElement(
fmt.Sprintf("openstack.org/ironicinspector-%s", ironicInspector.Name)))
}, timeout, interval).Should(Succeed())
})
It("removes topologyRef from the spec", func() {
Expand Down Expand Up @@ -385,6 +488,17 @@ var _ = Describe("Ironic controller", func() {
g.Expect(th.GetDeployment(ironicNames.INAName).Spec.Template.Spec.TopologySpreadConstraints).To(BeNil())
g.Expect(th.GetDeployment(ironicNames.INAName).Spec.Template.Spec.Affinity).ToNot(BeNil())
}, timeout, interval).Should(Succeed())

Eventually(func(g Gomega) {
for _, topology := range ironicNames.IronicTopologies {
// Get the current topology and verify there are no finalizers
tp := GetTopology(types.NamespacedName{
Name: topology.Name,
Namespace: topology.Namespace,
})
g.Expect(tp.GetFinalizers()).To(BeEmpty())
}
}, timeout, interval).Should(Succeed())
})
})
When("Ironic is created with nodeSelector", func() {
Expand Down