Skip to content

Commit 3381d60

Browse files
committed
HypervisorOffboarding: Decomission & Node are incorrect
The reconciler was already reconciling on a hypervisor custom-resource, so reflect that in the name. "Decomission" is misspelled, so lets go with offboarding as it matches onboarding on the other side, and also we may offboard a hypervisor for various reasons, decommissioning being one of them.
1 parent e97eaa7 commit 3381d60

3 files changed

Lines changed: 59 additions & 59 deletions

File tree

cmd/main.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,11 +260,11 @@ func main() {
260260
os.Exit(1)
261261
}
262262

263-
if err = (&controller.NodeDecommissionReconciler{
263+
if err = (&controller.HypervisorOffboardingReconciler{
264264
Client: mgr.GetClient(),
265265
Scheme: mgr.GetScheme(),
266266
}).SetupWithManager(mgr); err != nil {
267-
setupLog.Error(err, "unable to create controller", "controller", "NodeDecommission")
267+
setupLog.Error(err, "unable to create controller", "controller", "HypervisorOffboarding")
268268
os.Exit(1)
269269
}
270270

internal/controller/decomission_controller.go renamed to internal/controller/offboarding_controller.go

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
SPDX-FileCopyrightText: Copyright 2024 SAP SE or an SAP affiliate company and cobaltcore-dev contributors
2+
SPDX-FileCopyrightText: Copyright 2026 SAP SE or an SAP affiliate company and cobaltcore-dev contributors
33
SPDX-License-Identifier: Apache-2.0
44
55
Licensed under the Apache License, Version 2.0 (the "License");
@@ -38,10 +38,10 @@ import (
3838
)
3939

4040
const (
41-
DecommissionControllerName = "offboarding"
41+
OffboardingControllerName = "offboarding"
4242
)
4343

44-
type NodeDecommissionReconciler struct {
44+
type HypervisorOffboardingReconciler struct {
4545
k8sclient.Client
4646
Scheme *runtime.Scheme
4747
computeClient *gophercloud.ServiceClient
@@ -53,7 +53,7 @@ type NodeDecommissionReconciler struct {
5353

5454
// +kubebuilder:rbac:groups=kvm.cloud.sap,resources=hypervisors,verbs=get;list;watch
5555
// +kubebuilder:rbac:groups=kvm.cloud.sap,resources=hypervisors/status,verbs=get;list;watch;update;patch
56-
func (r *NodeDecommissionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
56+
func (r *HypervisorOffboardingReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
5757
hostname := req.Name
5858
log := logger.FromContext(ctx).WithName(req.Name).WithValues("hostname", hostname)
5959
ctx = logger.IntoContext(ctx, log)
@@ -69,7 +69,7 @@ func (r *NodeDecommissionReconciler) Reconcile(ctx context.Context, req ctrl.Req
6969
}
7070

7171
if meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeReady) {
72-
return r.setDecommissioningCondition(ctx, hv, "Node is being decommissioned, removing host from nova")
72+
return r.setOffboardingCondition(ctx, hv, "Hypervisor is being offboarded, removing host from nova")
7373
}
7474

7575
if meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeOffboarded) {
@@ -95,22 +95,22 @@ func (r *NodeDecommissionReconciler) Reconcile(ctx context.Context, req ctrl.Req
9595
// We are (hopefully) done
9696
return ctrl.Result{}, r.markOffboarded(ctx, hv)
9797
}
98-
return r.setDecommissioningCondition(ctx, hv, fmt.Sprintf("cannot get hypervisor by name %s due to %s", hostname, err))
98+
return r.setOffboardingCondition(ctx, hv, fmt.Sprintf("cannot get hypervisor by name %s due to %s", hostname, err))
9999
}
100100

101101
// TODO: remove since RunningVMs is only available until micro-version 2.87, and also is updated asynchronously
102102
// so it might be not accurate
103103
if hypervisor.RunningVMs > 0 {
104104
// Still running VMs, cannot delete the service
105-
msg := fmt.Sprintf("Node is being decommissioned, but still has %d running VMs", hypervisor.RunningVMs)
106-
return r.setDecommissioningCondition(ctx, hv, msg)
105+
msg := fmt.Sprintf("Hypervisor is being offboarded, but still has %d running VMs", hypervisor.RunningVMs)
106+
return r.setOffboardingCondition(ctx, hv, msg)
107107
}
108108

109109
if hypervisor.Servers != nil && len(*hypervisor.Servers) > 0 {
110110
// Still VMs assigned to the host, cannot delete the service
111-
msg := fmt.Sprintf("Node is being decommissioned, but still has %d assigned VMs, "+
111+
msg := fmt.Sprintf("Hypervisor is being offboarded, but still has %d assigned VMs, "+
112112
"check with `openstack server list --all-projects --host %s`", len(*hypervisor.Servers), hostname)
113-
return r.setDecommissioningCondition(ctx, hv, msg)
113+
return r.setOffboardingCondition(ctx, hv, msg)
114114
}
115115

116116
// Wait for aggregates controller to remove from all aggregates
@@ -120,44 +120,44 @@ func (r *NodeDecommissionReconciler) Reconcile(ctx context.Context, req ctrl.Req
120120
aggregateNames[i] = agg.Name
121121
}
122122
msg := fmt.Sprintf("Waiting for aggregates to be removed, current: %v", aggregateNames)
123-
return r.setDecommissioningCondition(ctx, hv, msg)
123+
return r.setOffboardingCondition(ctx, hv, msg)
124124
}
125125

126126
// Deleting and evicted, so better delete the service
127127
err = services.Delete(ctx, r.computeClient, hypervisor.Service.ID).ExtractErr()
128128
if err != nil && !gophercloud.ResponseCodeIs(err, http.StatusNotFound) {
129129
msg := fmt.Sprintf("cannot delete service %s due to %v", hypervisor.Service.ID, err)
130-
return r.setDecommissioningCondition(ctx, hv, msg)
130+
return r.setOffboardingCondition(ctx, hv, msg)
131131
}
132132

133133
rp, err := resourceproviders.Get(ctx, r.placementClient, hypervisor.ID).Extract()
134134
if err != nil && !gophercloud.ResponseCodeIs(err, http.StatusNotFound) {
135-
return r.setDecommissioningCondition(ctx, hv, fmt.Sprintf("cannot get resource provider: %v", err))
135+
return r.setOffboardingCondition(ctx, hv, fmt.Sprintf("cannot get resource provider: %v", err))
136136
}
137137

138138
if err = openstack.CleanupResourceProvider(ctx, r.placementClient, rp); err != nil {
139-
return r.setDecommissioningCondition(ctx, hv, fmt.Sprintf("cannot clean up resource provider: %v", err))
139+
return r.setOffboardingCondition(ctx, hv, fmt.Sprintf("cannot clean up resource provider: %v", err))
140140
}
141141

142142
return ctrl.Result{}, r.markOffboarded(ctx, hv)
143143
}
144144

145-
func (r *NodeDecommissionReconciler) setDecommissioningCondition(ctx context.Context, hv *kvmv1.Hypervisor, message string) (ctrl.Result, error) {
145+
func (r *HypervisorOffboardingReconciler) setOffboardingCondition(ctx context.Context, hv *kvmv1.Hypervisor, message string) (ctrl.Result, error) {
146146
base := hv.DeepCopy()
147147
meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
148148
Type: kvmv1.ConditionTypeReady,
149149
Status: metav1.ConditionFalse,
150-
Reason: "Decommissioning",
150+
Reason: "Offboarding",
151151
Message: message,
152152
})
153153
if err := r.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base,
154-
k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(DecommissionControllerName)); err != nil {
154+
k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(OffboardingControllerName)); err != nil {
155155
return ctrl.Result{}, fmt.Errorf("cannot update hypervisor status due to %w", err)
156156
}
157157
return ctrl.Result{}, nil
158158
}
159159

160-
func (r *NodeDecommissionReconciler) markOffboarded(ctx context.Context, hv *kvmv1.Hypervisor) error {
160+
func (r *HypervisorOffboardingReconciler) markOffboarded(ctx context.Context, hv *kvmv1.Hypervisor) error {
161161
base := hv.DeepCopy()
162162
meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
163163
Type: kvmv1.ConditionTypeOffboarded,
@@ -172,14 +172,14 @@ func (r *NodeDecommissionReconciler) markOffboarded(ctx context.Context, hv *kvm
172172
Message: "Offboarding successful",
173173
})
174174
if err := r.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base,
175-
k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(DecommissionControllerName)); err != nil {
175+
k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(OffboardingControllerName)); err != nil {
176176
return fmt.Errorf("cannot update hypervisor status due to %w", err)
177177
}
178178
return nil
179179
}
180180

181181
// SetupWithManager sets up the controller with the Manager.
182-
func (r *NodeDecommissionReconciler) SetupWithManager(mgr ctrl.Manager) error {
182+
func (r *HypervisorOffboardingReconciler) SetupWithManager(mgr ctrl.Manager) error {
183183
ctx := context.Background()
184184

185185
var err error
@@ -196,7 +196,7 @@ func (r *NodeDecommissionReconciler) SetupWithManager(mgr ctrl.Manager) error {
196196
r.placementClient.Microversion = "1.39" // yoga, or later
197197

198198
return ctrl.NewControllerManagedBy(mgr).
199-
Named(DecommissionControllerName).
199+
Named(OffboardingControllerName).
200200
For(&kvmv1.Hypervisor{}).
201201
Complete(r)
202202
}

internal/controller/decomission_controller_test.go renamed to internal/controller/offboarding_controller_test.go

Lines changed: 36 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
SPDX-FileCopyrightText: Copyright 2024 SAP SE or an SAP affiliate company and cobaltcore-dev contributors
2+
SPDX-FileCopyrightText: Copyright 2026 SAP SE or an SAP affiliate company and cobaltcore-dev contributors
33
SPDX-License-Identifier: Apache-2.0
44
55
Licensed under the Apache License, Version 2.0 (the "License");
@@ -36,7 +36,7 @@ import (
3636
kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1"
3737
)
3838

39-
var _ = Describe("Decommission Controller", func() {
39+
var _ = Describe("Offboarding Controller", func() {
4040
const (
4141
EOF = "EOF"
4242
serviceId = "service-1234"
@@ -45,10 +45,10 @@ var _ = Describe("Decommission Controller", func() {
4545
)
4646

4747
var (
48-
decommissionReconciler *NodeDecommissionReconciler
49-
resourceName = types.NamespacedName{Name: hypervisorName}
50-
reconcileReq = ctrl.Request{NamespacedName: resourceName}
51-
fakeServer testhelper.FakeServer
48+
offboardingReconciler *HypervisorOffboardingReconciler
49+
resourceName = types.NamespacedName{Name: hypervisorName}
50+
reconcileReq = ctrl.Request{NamespacedName: resourceName}
51+
fakeServer testhelper.FakeServer
5252
)
5353

5454
BeforeEach(func(ctx SpecContext) {
@@ -67,8 +67,8 @@ var _ = Describe("Decommission Controller", func() {
6767
os.Unsetenv("KVM_HA_SERVICE_URL")
6868
})
6969

70-
By("Creating the NodeDecommissionReconciler")
71-
decommissionReconciler = &NodeDecommissionReconciler{
70+
By("Creating the HypervisorOffboardingReconciler")
71+
offboardingReconciler = &HypervisorOffboardingReconciler{
7272
Client: k8sClient,
7373
Scheme: k8sClient.Scheme(),
7474
computeClient: client.ServiceClient(fakeServer),
@@ -101,7 +101,7 @@ var _ = Describe("Decommission Controller", func() {
101101
Context("When marking the hypervisor terminating", func() {
102102
JustBeforeEach(func(ctx SpecContext) {
103103
By("Reconciling first to add the finalizer")
104-
_, err := decommissionReconciler.Reconcile(ctx, reconcileReq)
104+
_, err := offboardingReconciler.Reconcile(ctx, reconcileReq)
105105
Expect(err).NotTo(HaveOccurred())
106106

107107
By("Marking the hypervisor for termination")
@@ -176,7 +176,7 @@ var _ = Describe("Decommission Controller", func() {
176176
})
177177

178178
It("should set the hypervisor ready condition", func(ctx SpecContext) {
179-
_, err := decommissionReconciler.Reconcile(ctx, reconcileReq)
179+
_, err := offboardingReconciler.Reconcile(ctx, reconcileReq)
180180
Expect(err).NotTo(HaveOccurred())
181181

182182
hypervisor := &kvmv1.Hypervisor{}
@@ -185,14 +185,14 @@ var _ = Describe("Decommission Controller", func() {
185185
SatisfyAll(
186186
HaveField("Type", kvmv1.ConditionTypeReady),
187187
HaveField("Status", metav1.ConditionFalse),
188-
HaveField("Reason", "Decommissioning"),
188+
HaveField("Reason", "Offboarding"),
189189
),
190190
))
191191
})
192192

193193
It("should set the hypervisor offboarded condition", func(ctx SpecContext) {
194194
for range 3 {
195-
_, err := decommissionReconciler.Reconcile(ctx, reconcileReq)
195+
_, err := offboardingReconciler.Reconcile(ctx, reconcileReq)
196196
Expect(err).NotTo(HaveOccurred())
197197
}
198198
Expect(getHypervisorsCalled).To(BeNumerically(">", 0))
@@ -227,8 +227,8 @@ var _ = Describe("Decommission Controller", func() {
227227
hypervisor.Status.HypervisorID = "c48f6247-abe4-4a24-824e-ea39e108874f"
228228
Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
229229

230-
By("Reconciling - decommission controller will wait for aggregates to be cleared")
231-
_, err := decommissionReconciler.Reconcile(ctx, reconcileReq)
230+
By("Reconciling - offboarding controller will wait for aggregates to be cleared")
231+
_, err := offboardingReconciler.Reconcile(ctx, reconcileReq)
232232
Expect(err).NotTo(HaveOccurred())
233233

234234
By("Simulating aggregates controller clearing aggregates")
@@ -238,21 +238,21 @@ var _ = Describe("Decommission Controller", func() {
238238

239239
By("Reconciling again after aggregates are cleared")
240240
for range 3 {
241-
_, err := decommissionReconciler.Reconcile(ctx, reconcileReq)
241+
_, err := offboardingReconciler.Reconcile(ctx, reconcileReq)
242242
Expect(err).NotTo(HaveOccurred())
243243
}
244244

245245
By("Verifying Status.Aggregates is empty")
246246
Expect(k8sClient.Get(ctx, resourceName, hypervisor)).To(Succeed())
247-
Expect(hypervisor.Status.Aggregates).To(BeEmpty(), "Status.Aggregates should be cleared after decommissioning")
247+
Expect(hypervisor.Status.Aggregates).To(BeEmpty(), "Status.Aggregates should be cleared after offboarding")
248248
})
249249
})
250250
})
251251
})
252252

253253
Context("Guard Conditions", func() {
254254
JustBeforeEach(func(ctx SpecContext) {
255-
result, err := decommissionReconciler.Reconcile(ctx, reconcileReq)
255+
result, err := offboardingReconciler.Reconcile(ctx, reconcileReq)
256256
Expect(err).NotTo(HaveOccurred())
257257
Expect(result).To(Equal(ctrl.Result{}))
258258
})
@@ -291,14 +291,14 @@ var _ = Describe("Decommission Controller", func() {
291291
})
292292

293293
Context("Failure Modes", func() {
294-
var sharedDecommissioningErrorCheck = func(ctx SpecContext, expectedMessageSubstring string) {
294+
var sharedOffboardingErrorCheck = func(ctx SpecContext, expectedMessageSubstring string) {
295295
hypervisor := &kvmv1.Hypervisor{}
296296
Expect(k8sClient.Get(ctx, resourceName, hypervisor)).To(Succeed())
297297
Expect(hypervisor.Status.Conditions).To(ContainElement(
298298
SatisfyAll(
299299
HaveField("Type", kvmv1.ConditionTypeReady),
300300
HaveField("Status", metav1.ConditionFalse),
301-
HaveField("Reason", "Decommissioning"),
301+
HaveField("Reason", "Offboarding"),
302302
HaveField("Message", ContainSubstring(expectedMessageSubstring)),
303303
),
304304
))
@@ -320,10 +320,10 @@ var _ = Describe("Decommission Controller", func() {
320320
})
321321
})
322322

323-
It("should set decommissioning condition with error", func(ctx SpecContext) {
324-
_, err := decommissionReconciler.Reconcile(ctx, reconcileReq)
323+
It("should set offboarding condition with error", func(ctx SpecContext) {
324+
_, err := offboardingReconciler.Reconcile(ctx, reconcileReq)
325325
Expect(err).NotTo(HaveOccurred())
326-
sharedDecommissioningErrorCheck(ctx, "")
326+
sharedOffboardingErrorCheck(ctx, "")
327327
})
328328
})
329329

@@ -352,10 +352,10 @@ var _ = Describe("Decommission Controller", func() {
352352
})
353353
})
354354

355-
It("should set decommissioning condition about running VMs", func(ctx SpecContext) {
356-
_, err := decommissionReconciler.Reconcile(ctx, reconcileReq)
355+
It("should set offboarding condition about running VMs", func(ctx SpecContext) {
356+
_, err := offboardingReconciler.Reconcile(ctx, reconcileReq)
357357
Expect(err).NotTo(HaveOccurred())
358-
sharedDecommissioningErrorCheck(ctx, "still has 2 running VMs")
358+
sharedOffboardingErrorCheck(ctx, "still has 2 running VMs")
359359
})
360360
})
361361

@@ -391,9 +391,9 @@ var _ = Describe("Decommission Controller", func() {
391391
})
392392

393393
It("should wait for aggregates to be cleared", func(ctx SpecContext) {
394-
_, err := decommissionReconciler.Reconcile(ctx, reconcileReq)
394+
_, err := offboardingReconciler.Reconcile(ctx, reconcileReq)
395395
Expect(err).NotTo(HaveOccurred())
396-
sharedDecommissioningErrorCheck(ctx, "Waiting for aggregates to be removed")
396+
sharedOffboardingErrorCheck(ctx, "Waiting for aggregates to be removed")
397397
})
398398
})
399399

@@ -424,10 +424,10 @@ var _ = Describe("Decommission Controller", func() {
424424
})
425425
})
426426

427-
It("should set decommissioning condition with error", func(ctx SpecContext) {
428-
_, err := decommissionReconciler.Reconcile(ctx, reconcileReq)
427+
It("should set offboarding condition with error", func(ctx SpecContext) {
428+
_, err := offboardingReconciler.Reconcile(ctx, reconcileReq)
429429
Expect(err).NotTo(HaveOccurred())
430-
sharedDecommissioningErrorCheck(ctx, "cannot delete service")
430+
sharedOffboardingErrorCheck(ctx, "cannot delete service")
431431
})
432432
})
433433

@@ -462,10 +462,10 @@ var _ = Describe("Decommission Controller", func() {
462462
})
463463
})
464464

465-
It("should set decommissioning condition with error", func(ctx SpecContext) {
466-
_, err := decommissionReconciler.Reconcile(ctx, reconcileReq)
465+
It("should set offboarding condition with error", func(ctx SpecContext) {
466+
_, err := offboardingReconciler.Reconcile(ctx, reconcileReq)
467467
Expect(err).NotTo(HaveOccurred())
468-
sharedDecommissioningErrorCheck(ctx, "cannot get resource provider")
468+
sharedOffboardingErrorCheck(ctx, "cannot get resource provider")
469469
})
470470
})
471471

@@ -512,10 +512,10 @@ var _ = Describe("Decommission Controller", func() {
512512
})
513513
})
514514

515-
It("should set decommissioning condition with error", func(ctx SpecContext) {
516-
_, err := decommissionReconciler.Reconcile(ctx, reconcileReq)
515+
It("should set offboarding condition with error", func(ctx SpecContext) {
516+
_, err := offboardingReconciler.Reconcile(ctx, reconcileReq)
517517
Expect(err).NotTo(HaveOccurred())
518-
sharedDecommissioningErrorCheck(ctx, "cannot clean up resource provider")
518+
sharedOffboardingErrorCheck(ctx, "cannot clean up resource provider")
519519
})
520520
})
521521
})

0 commit comments

Comments
 (0)