Skip to content

Commit 03b8ab9

Browse files
fwieselfwiesel
committed
HypervisorSpec: Add maintenance reason and require it when manual
Setting simply a hypervisor into maintenance manually is always leaving the question why someone did that. So we add a reason field, and require it to be set.
1 parent c8c9db4 commit 03b8ab9

File tree

7 files changed

+227
-0
lines changed

7 files changed

+227
-0
lines changed

‎api/v1/hypervisor_types.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ const (
6969

7070
// HypervisorSpec defines the desired state of Hypervisor
7171
// +kubebuilder:validation:XValidation:rule="!has(oldSelf.maintenance) || oldSelf.maintenance != 'termination' || self.maintenance == 'ha' || self == oldSelf",message="spec is immutable when maintenance is 'termination'; can only change maintenance to 'ha'"
72+
// +kubebuilder:validation:XValidation:rule="!has(self.maintenance) || self.maintenance != 'manual' || (has(self.maintenanceReason) && self.maintenanceReason.size() > 0)",message="maintenanceReason must be non-empty when maintenance is 'manual'"
7273
type HypervisorSpec struct {
7374
// +kubebuilder:validation:Optional
7475
// OperatingSystemVersion represents the desired operating system version.
@@ -121,6 +122,10 @@ type HypervisorSpec struct {
121122
// +kubebuilder:validation:Enum:="";manual;auto;ha;termination
122123
// Maintenance indicates whether the hypervisor is in maintenance mode.
123124
Maintenance string `json:"maintenance,omitempty"`
125+
126+
// +kubebuilder:optional
127+
// MaintenanceReason provides the reason for manual maintenance mode.
128+
MaintenanceReason string `json:"maintenanceReason,omitempty"`
124129
}
125130

126131
const (

‎api/v1/hypervisor_validation_test.go‎

Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ var _ = Describe("Hypervisor Spec CEL Validation", func() {
5151
EvacuateOnReboot: true,
5252
InstallCertificate: true,
5353
Maintenance: MaintenanceManual,
54+
MaintenanceReason: "Test maintenance reason",
5455
},
5556
}
5657

@@ -204,3 +205,194 @@ var _ = Describe("Maintenance Constants", func() {
204205
Expect(MaintenanceTermination).To(Equal("termination"))
205206
})
206207
})
208+
209+
// TestMaintenanceReasonValidation tests the CEL validation rule for MaintenanceReason
210+
// The rule: !has(self.maintenance) || self.maintenance != 'manual' || (has(self.maintenanceReason) && self.maintenanceReason != ”)
211+
// This ensures that when maintenance is set to 'manual', maintenanceReason must be non-empty
212+
var _ = Describe("MaintenanceReason CEL Validation", func() {
213+
var (
214+
hypervisor *Hypervisor
215+
hypervisorName types.NamespacedName
216+
)
217+
218+
Context("When creating a new Hypervisor", func() {
219+
AfterEach(func(ctx SpecContext) {
220+
if hypervisor != nil {
221+
Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, hypervisor))).To(Succeed())
222+
}
223+
})
224+
225+
It("should allow creation with maintenance='manual' and a non-empty maintenanceReason", func(ctx SpecContext) {
226+
hypervisor = &Hypervisor{
227+
ObjectMeta: metav1.ObjectMeta{
228+
Name: "test-hypervisor-manual-with-reason",
229+
},
230+
Spec: HypervisorSpec{
231+
OperatingSystemVersion: "1.0",
232+
LifecycleEnabled: true,
233+
Maintenance: MaintenanceManual,
234+
MaintenanceReason: "Hardware upgrade required",
235+
},
236+
}
237+
238+
Expect(k8sClient.Create(ctx, hypervisor)).To(Succeed())
239+
240+
created := &Hypervisor{}
241+
Expect(k8sClient.Get(ctx, types.NamespacedName{Name: hypervisor.Name}, created)).To(Succeed())
242+
Expect(created.Spec.Maintenance).To(Equal(MaintenanceManual))
243+
Expect(created.Spec.MaintenanceReason).To(Equal("Hardware upgrade required"))
244+
})
245+
246+
It("should reject creation with maintenance='manual' but empty maintenanceReason", func(ctx SpecContext) {
247+
hypervisor = &Hypervisor{
248+
ObjectMeta: metav1.ObjectMeta{
249+
Name: "test-hypervisor-manual-empty-reason",
250+
},
251+
Spec: HypervisorSpec{
252+
Maintenance: MaintenanceManual,
253+
MaintenanceReason: "",
254+
},
255+
}
256+
257+
err := k8sClient.Create(ctx, hypervisor)
258+
Expect(err).To(HaveOccurred())
259+
Expect(err.Error()).To(ContainSubstring("maintenanceReason must be non-empty when maintenance is 'manual'"))
260+
})
261+
262+
It("should reject creation with maintenance='manual' but missing maintenanceReason", func(ctx SpecContext) {
263+
hypervisor = &Hypervisor{
264+
ObjectMeta: metav1.ObjectMeta{
265+
Name: "test-hypervisor-manual-no-reason",
266+
},
267+
Spec: HypervisorSpec{
268+
Maintenance: MaintenanceManual,
269+
},
270+
}
271+
272+
err := k8sClient.Create(ctx, hypervisor)
273+
Expect(err).To(HaveOccurred())
274+
Expect(err.Error()).To(ContainSubstring("maintenanceReason must be non-empty when maintenance is 'manual'"))
275+
})
276+
277+
It("should allow creation with non-manual maintenance modes without maintenanceReason", func(ctx SpecContext) {
278+
hypervisor = &Hypervisor{
279+
ObjectMeta: metav1.ObjectMeta{
280+
Name: "test-hypervisor-auto-no-reason",
281+
},
282+
Spec: HypervisorSpec{
283+
Maintenance: MaintenanceAuto,
284+
},
285+
}
286+
287+
Expect(k8sClient.Create(ctx, hypervisor)).To(Succeed())
288+
289+
created := &Hypervisor{}
290+
Expect(k8sClient.Get(ctx, types.NamespacedName{Name: hypervisor.Name}, created)).To(Succeed())
291+
Expect(created.Spec.Maintenance).To(Equal(MaintenanceAuto))
292+
})
293+
294+
It("should allow creation with empty maintenance and no maintenanceReason", func(ctx SpecContext) {
295+
hypervisor = &Hypervisor{
296+
ObjectMeta: metav1.ObjectMeta{
297+
Name: "test-hypervisor-no-maintenance",
298+
},
299+
Spec: HypervisorSpec{
300+
LifecycleEnabled: true,
301+
},
302+
}
303+
304+
Expect(k8sClient.Create(ctx, hypervisor)).To(Succeed())
305+
306+
created := &Hypervisor{}
307+
Expect(k8sClient.Get(ctx, types.NamespacedName{Name: hypervisor.Name}, created)).To(Succeed())
308+
Expect(created.Spec.Maintenance).To(Equal(""))
309+
})
310+
})
311+
312+
Context("When updating an existing Hypervisor", func() {
313+
BeforeEach(func(ctx SpecContext) {
314+
hypervisorName = types.NamespacedName{
315+
Name: "test-hypervisor-update",
316+
}
317+
318+
hypervisor = &Hypervisor{
319+
ObjectMeta: metav1.ObjectMeta{
320+
Name: hypervisorName.Name,
321+
},
322+
Spec: HypervisorSpec{
323+
LifecycleEnabled: true,
324+
Maintenance: MaintenanceAuto,
325+
},
326+
}
327+
328+
Expect(k8sClient.Create(ctx, hypervisor)).To(Succeed())
329+
})
330+
331+
AfterEach(func(ctx SpecContext) {
332+
Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, hypervisor))).To(Succeed())
333+
})
334+
335+
It("should allow updating to maintenance='manual' with a non-empty maintenanceReason", func(ctx SpecContext) {
336+
hypervisor.Spec.Maintenance = MaintenanceManual
337+
hypervisor.Spec.MaintenanceReason = "Planned maintenance window"
338+
Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed())
339+
340+
updated := &Hypervisor{}
341+
Expect(k8sClient.Get(ctx, hypervisorName, updated)).To(Succeed())
342+
Expect(updated.Spec.Maintenance).To(Equal(MaintenanceManual))
343+
Expect(updated.Spec.MaintenanceReason).To(Equal("Planned maintenance window"))
344+
})
345+
346+
It("should reject updating to maintenance='manual' without maintenanceReason", func(ctx SpecContext) {
347+
hypervisor.Spec.Maintenance = MaintenanceManual
348+
err := k8sClient.Update(ctx, hypervisor)
349+
Expect(err).To(HaveOccurred())
350+
Expect(err.Error()).To(ContainSubstring("maintenanceReason must be non-empty when maintenance is 'manual'"))
351+
})
352+
353+
It("should reject updating to maintenance='manual' with empty maintenanceReason", func(ctx SpecContext) {
354+
hypervisor.Spec.Maintenance = MaintenanceManual
355+
hypervisor.Spec.MaintenanceReason = ""
356+
err := k8sClient.Update(ctx, hypervisor)
357+
Expect(err).To(HaveOccurred())
358+
Expect(err.Error()).To(ContainSubstring("maintenanceReason must be non-empty when maintenance is 'manual'"))
359+
})
360+
361+
It("should allow updating from manual to another maintenance mode", func(ctx SpecContext) {
362+
// First set to manual with reason
363+
hypervisor.Spec.Maintenance = MaintenanceManual
364+
hypervisor.Spec.MaintenanceReason = "Initial reason"
365+
Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed())
366+
367+
// Refresh hypervisor
368+
Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
369+
370+
// Update to auto (reason becomes optional and can be cleared)
371+
hypervisor.Spec.Maintenance = MaintenanceAuto
372+
hypervisor.Spec.MaintenanceReason = ""
373+
Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed())
374+
375+
updated := &Hypervisor{}
376+
Expect(k8sClient.Get(ctx, hypervisorName, updated)).To(Succeed())
377+
Expect(updated.Spec.Maintenance).To(Equal(MaintenanceAuto))
378+
})
379+
380+
It("should allow updating maintenanceReason when maintenance is already 'manual'", func(ctx SpecContext) {
381+
// First set to manual with reason
382+
hypervisor.Spec.Maintenance = MaintenanceManual
383+
hypervisor.Spec.MaintenanceReason = "Initial reason"
384+
Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed())
385+
386+
// Refresh hypervisor
387+
Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
388+
389+
// Update the reason
390+
hypervisor.Spec.MaintenanceReason = "Updated reason"
391+
Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed())
392+
393+
updated := &Hypervisor{}
394+
Expect(k8sClient.Get(ctx, hypervisorName, updated)).To(Succeed())
395+
Expect(updated.Spec.MaintenanceReason).To(Equal("Updated reason"))
396+
})
397+
})
398+
})

‎applyconfigurations/api/v1/hypervisorspec.go‎

Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎charts/openstack-hypervisor-operator/crds/hypervisor-crd.yaml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,10 @@ spec:
160160
- ha
161161
- termination
162162
type: string
163+
maintenanceReason:
164+
description: MaintenanceReason provides the reason for manual maintenance
165+
mode.
166+
type: string
163167
reboot:
164168
default: false
165169
description: Reboot request an reboot after successful installation
@@ -190,6 +194,9 @@ spec:
190194
change maintenance to 'ha'
191195
rule: '!has(oldSelf.maintenance) || oldSelf.maintenance != ''termination''
192196
|| self.maintenance == ''ha'' || self == oldSelf'
197+
- message: maintenanceReason must be non-empty when maintenance is 'manual'
198+
rule: '!has(self.maintenance) || self.maintenance != ''manual'' || (has(self.maintenanceReason)
199+
&& self.maintenanceReason.size() > 0)'
193200
status:
194201
description: HypervisorStatus defines the observed state of Hypervisor
195202
properties:

‎config/crd/bases/kvm.cloud.sap_hypervisors.yaml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,10 @@ spec:
161161
- ha
162162
- termination
163163
type: string
164+
maintenanceReason:
165+
description: MaintenanceReason provides the reason for manual maintenance
166+
mode.
167+
type: string
164168
reboot:
165169
default: false
166170
description: Reboot request an reboot after successful installation
@@ -191,6 +195,9 @@ spec:
191195
change maintenance to 'ha'
192196
rule: '!has(oldSelf.maintenance) || oldSelf.maintenance != ''termination''
193197
|| self.maintenance == ''ha'' || self == oldSelf'
198+
- message: maintenanceReason must be non-empty when maintenance is 'manual'
199+
rule: '!has(self.maintenance) || self.maintenance != ''manual'' || (has(self.maintenanceReason)
200+
&& self.maintenanceReason.size() > 0)'
194201
status:
195202
description: HypervisorStatus defines the observed state of Hypervisor
196203
properties:

‎internal/controller/decomission_controller_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,7 @@ var _ = Describe("Decommission Controller", func() {
281281
hypervisor := &kvmv1.Hypervisor{}
282282
Expect(k8sClient.Get(ctx, resourceName, hypervisor)).To(Succeed())
283283
hypervisor.Spec.Maintenance = kvmv1.MaintenanceManual
284+
hypervisor.Spec.MaintenanceReason = "Test maintenance reason"
284285
Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed())
285286
})
286287

‎internal/controller/hypervisor_maintenance_controller_test.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,9 @@ var _ = Describe("HypervisorMaintenanceController", func() {
176176
hypervisor := &kvmv1.Hypervisor{}
177177
Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
178178
hypervisor.Spec.Maintenance = mode
179+
if mode == "manual" {
180+
hypervisor.Spec.MaintenanceReason = "Test maintenance reason"
181+
}
179182
Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed())
180183
expectedBody := fmt.Sprintf(`{"disabled_reason": "Hypervisor CRD: spec.maintenance=%v", "status": "disabled"}`, mode)
181184
mockServiceUpdate(expectedBody)
@@ -261,6 +264,9 @@ var _ = Describe("HypervisorMaintenanceController", func() {
261264
hypervisor := &kvmv1.Hypervisor{}
262265
Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
263266
hypervisor.Spec.Maintenance = mode
267+
if mode == "manual" {
268+
hypervisor.Spec.MaintenanceReason = "Test maintenance reason"
269+
}
264270
Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed())
265271
expectedBody := fmt.Sprintf(`{"disabled_reason": "Hypervisor CRD: spec.maintenance=%v", "status": "disabled"}`, mode)
266272
mockServiceUpdate(expectedBody)

0 commit comments

Comments
 (0)