Skip to content

Aggregates: Only reconcile after onboarding and before decomissioning #185

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 1 commit into
base: main
Choose a base branch
from
Open

Aggregates: Only reconcile after onboarding and before decomissioning #185

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
8 changes: 6 additions & 2 deletions internal/controller/aggregates_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/builder"
k8sclient "sigs.k8s.io/controller-runtime/pkg/client"
logger "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/predicate"

"github.com/gophercloud/gophercloud/v2"
"github.com/gophercloud/gophercloud/v2/openstack/compute/v2/aggregates"
Expand Down Expand Up @@ -63,6 +62,12 @@ func (ac *AggregatesController) Reconcile(ctx context.Context, req ctrl.Request)
return ctrl.Result{}, k8sclient.IgnoreNotFound(err)
}

/// On- and off-boarding need to mess with the aggregates, so let's get out of their way
if !meta.IsStatusConditionFalse(hv.Status.Conditions, ConditionTypeOnboarding) ||
meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeTerminating) {
return ctrl.Result{}, nil
}

if slices.Equal(hv.Spec.Aggregates, hv.Status.Aggregates) {
// Nothing to be done
return ctrl.Result{}, nil
Expand Down Expand Up @@ -154,7 +159,6 @@ func (ac *AggregatesController) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
Named(AggregatesControllerName).
For(&kvmv1.Hypervisor{}, builder.WithPredicates(utils.LifecycleEnabledPredicate)).
WithEventFilter(predicate.GenerationChangedPredicate{}).
Complete(ac)
}

Expand Down
142 changes: 88 additions & 54 deletions internal/controller/aggregates_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,18 +35,15 @@ import (
)

var _ = Describe("AggregatesController", func() {
var (
tc *AggregatesController
fakeServer testhelper.FakeServer
)
const EOF = "EOF"
const AggregateListBodyEmpty = `
const (
EOF = "EOF"
AggregateListBodyEmpty = `
{
"aggregates": []
}
`

const AggregateListBodyFull = `
AggregateListBodyFull = `
{
"aggregates": [
{
Expand All @@ -67,7 +64,7 @@ var _ = Describe("AggregatesController", func() {
}
`

const AggregatesPostBody = `
AggregatesPostBody = `
{
"aggregate": {
"name": "test-aggregate1",
Expand All @@ -77,7 +74,7 @@ var _ = Describe("AggregatesController", func() {
}
}`

const AggregateRemoveHostBody = `
AggregateRemoveHostBody = `
{
"aggregate": {
"name": "test-aggregate3",
Expand All @@ -87,7 +84,7 @@ var _ = Describe("AggregatesController", func() {
}
}`

const AggregateAddHostBody = `
AggregateAddHostBody = `
{
"aggregate": {
"name": "test-aggregate1",
Expand All @@ -99,47 +96,63 @@ var _ = Describe("AggregatesController", func() {
"id": 42
}
}`
)

var (
tc *AggregatesController
fakeServer testhelper.FakeServer
hypervisorName = types.NamespacedName{Name: "hv-test"}
)
// Setup and teardown

BeforeEach(func(ctx context.Context) {
By("Setting up the OpenStack http mock server")
fakeServer = testhelper.SetupHTTP()
DeferCleanup(fakeServer.Teardown)

hypervisor := &kvmv1.Hypervisor{
ObjectMeta: v1.ObjectMeta{
Name: hypervisorName.Name,
},
Spec: kvmv1.HypervisorSpec{
LifecycleEnabled: true,
},
}
Expect(k8sClient.Create(ctx, hypervisor)).To(Succeed())
Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
meta.SetStatusCondition(&hypervisor.Status.Conditions, v1.Condition{
Type: ConditionTypeOnboarding,
Status: v1.ConditionFalse,
Reason: "dontcare",
Message: "dontcare",
})
Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())

By("Creating the AggregatesController")
tc = &AggregatesController{
Client: k8sClient,
Scheme: k8sClient.Scheme(),
computeClient: client.ServiceClient(fakeServer),
}
})

AfterEach(func() {
By("Tearing down the OpenStack http mock server")
fakeServer.Teardown()
DeferCleanup(func(ctx context.Context) {
Expect(tc.Client.Delete(ctx, hypervisor)).To(Succeed())
})
})

By("Deleting the Hypervisor resource")
hypervisor := &kvmv1.Hypervisor{}
Expect(tc.Client.Get(ctx, types.NamespacedName{Name: "hv-test", Namespace: "default"}, hypervisor)).To(Succeed())
Expect(tc.Client.Delete(ctx, hypervisor)).To(Succeed())
JustBeforeEach(func(ctx context.Context) {
_, err := tc.Reconcile(ctx, ctrl.Request{NamespacedName: hypervisorName})
Expect(err).NotTo(HaveOccurred())
})

// Tests

Context("Adding new Aggregate", func() {
BeforeEach(func() {
By("Creating a Hypervisor resource")
hypervisor := &kvmv1.Hypervisor{
ObjectMeta: v1.ObjectMeta{
Name: "hv-test",
Namespace: "default",
},
Spec: kvmv1.HypervisorSpec{
LifecycleEnabled: true,
Aggregates: []string{"test-aggregate1"},
},
}
Expect(k8sClient.Create(ctx, hypervisor)).To(Succeed())
By("Setting a missing aggregate")
hypervisor := &kvmv1.Hypervisor{}
Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
hypervisor.Spec.Aggregates = []string{"test-aggregate1"}
Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed())

// Mock resourceproviders.GetAggregates
fakeServer.Mux.HandleFunc("GET /os-aggregates", func(w http.ResponseWriter, r *http.Request) {
Expand Down Expand Up @@ -176,32 +189,18 @@ var _ = Describe("AggregatesController", func() {
})
})

It("should update Aggregates and set status condition when Aggregates differ", func() {
req := ctrl.Request{NamespacedName: types.NamespacedName{Name: "hv-test", Namespace: "default"}}
_, err := tc.Reconcile(ctx, req)
Expect(err).NotTo(HaveOccurred())

It("should update Aggregates and set status condition as Aggregates differ", func() {
updated := &kvmv1.Hypervisor{}
Expect(tc.Client.Get(ctx, req.NamespacedName, updated)).To(Succeed())
Expect(tc.Client.Get(ctx, hypervisorName, updated)).To(Succeed())
Expect(updated.Status.Aggregates).To(ContainElements("test-aggregate1"))
Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, ConditionTypeAggregatesUpdated)).To(BeTrue())
})
})

Context("Removing Aggregate", func() {
BeforeEach(func() {
By("Creating a Hypervisor resource")
hypervisor := &kvmv1.Hypervisor{
ObjectMeta: v1.ObjectMeta{
Name: "hv-test",
Namespace: "default",
},
Spec: kvmv1.HypervisorSpec{
LifecycleEnabled: true,
},
}
Expect(k8sClient.Create(ctx, hypervisor)).To(Succeed())

hypervisor := &kvmv1.Hypervisor{}
Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
// update status to have existing aggregate
hypervisor.Status.Aggregates = []string{"test-aggregate2", "test-aggregate3"}
Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
Expand Down Expand Up @@ -235,14 +234,49 @@ var _ = Describe("AggregatesController", func() {
})

It("should update Aggregates and set status condition when Aggregates differ", func() {
req := ctrl.Request{NamespacedName: types.NamespacedName{Name: "hv-test", Namespace: "default"}}
_, err := tc.Reconcile(ctx, req)
Expect(err).NotTo(HaveOccurred())

updated := &kvmv1.Hypervisor{}
Expect(tc.Client.Get(ctx, req.NamespacedName, updated)).To(Succeed())
Expect(tc.Client.Get(ctx, hypervisorName, updated)).To(Succeed())
Expect(updated.Status.Aggregates).To(BeEmpty())
Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, ConditionTypeAggregatesUpdated)).To(BeTrue())
})
})

Context("before onboarding", func() {
BeforeEach(func() {
hypervisor := &kvmv1.Hypervisor{}
Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
// Remove the onboarding condition
hypervisor.Status.Conditions = []v1.Condition{}
Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
})

It("should neither update Aggregates and nor set status condition", func() {
updated := &kvmv1.Hypervisor{}
Expect(tc.Client.Get(ctx, hypervisorName, updated)).To(Succeed())
Expect(updated.Status.Aggregates).To(BeEmpty())
Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, ConditionTypeAggregatesUpdated)).To(BeFalse())
})
})

Context("when terminating", func() {
BeforeEach(func() {
hypervisor := &kvmv1.Hypervisor{}
Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
// Remove the onboarding condition
meta.SetStatusCondition(&hypervisor.Status.Conditions, v1.Condition{
Type: kvmv1.ConditionTypeTerminating,
Status: v1.ConditionTrue,
Reason: "dontcare",
Message: "dontcare",
})
Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
})

It("should neither update Aggregates and nor set status condition", func() {
updated := &kvmv1.Hypervisor{}
Expect(tc.Client.Get(ctx, hypervisorName, updated)).To(Succeed())
Expect(updated.Status.Aggregates).To(BeEmpty())
Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, ConditionTypeAggregatesUpdated)).To(BeFalse())
})
})
})
Loading