Skip to content

Commit f098914

Browse files
dtantsurdtantsur
committed
Introduce InspectionMode field
This change introduces the InspectionMode field from the BMH v1beta1 design proposal. It partly replaces the inspection annotation (namely, its "disabled" value) by something more user-friendly and consistent with AutomatedCleaningMode. The "disabled" value of the inspection annotation is now deprecated. Other values are not affected by this change. Assisted-By: Claude Code Signed-off-by: Dmitry Tantsur <[email protected]>
1 parent ea5e9f0 commit f098914

File tree

10 files changed

+282
-36
lines changed

10 files changed

+282
-36
lines changed

‎apis/metal3.io/v1alpha1/baremetalhost_types.go‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,22 @@ const (
6767
// InspectAnnotationValueDisabled is a constant string="disabled"
6868
// This is particularly useful to check if inspect annotation is disabled
6969
// inspect.metal3.io=disabled.
70+
//
71+
// Deprecated: use InspectionMode instead.
7072
InspectAnnotationValueDisabled = "disabled"
7173
)
7274

75+
// InspectionMode represents the mode of host inspection.
76+
type InspectionMode string
77+
78+
const (
79+
// InspectionModeDisabled disables host inspection.
80+
InspectionModeDisabled InspectionMode = "disabled"
81+
82+
// InspectionModeAgent runs standard agent-based inspection.
83+
InspectionModeAgent InspectionMode = "agent"
84+
)
85+
7386
// RootDeviceHints holds the hints for specifying the storage location
7487
// for the root filesystem for the image.
7588
type RootDeviceHints struct {
@@ -485,6 +498,13 @@ type BareMetalHostSpec struct {
485498
// instead, a reboot will be used in place of power on/off
486499
// +optional
487500
DisablePowerOff bool `json:"disablePowerOff,omitempty"`
501+
502+
// Specifies the mode for host inspection.
503+
// "disabled" - no inspection will be performed
504+
// "agent" - normal agent-based inspection will run
505+
// +optional
506+
// +kubebuilder:validation:Enum=disabled;agent
507+
InspectionMode InspectionMode `json:"inspectionMode,omitempty"`
488508
}
489509

490510
// AutomatedCleaningMode is the interface to enable/disable automated cleaning
@@ -974,6 +994,19 @@ func (host *BareMetalHost) CredentialsKey() types.NamespacedName {
974994
}
975995
}
976996

997+
// InspectionDisabled returns true if inspection is disabled via either
998+
// the inspect.metal3.io annotation or the inspectionMode field.
999+
func (host *BareMetalHost) InspectionDisabled() bool {
1000+
// Check the new InspectionMode field first
1001+
if host.Spec.InspectionMode == InspectionModeDisabled {
1002+
return true
1003+
}
1004+
1005+
// Fall back to the legacy annotation for backward compatibility
1006+
annotations := host.GetAnnotations()
1007+
return annotations[InspectAnnotationPrefix] == InspectAnnotationValueDisabled
1008+
}
1009+
9771010
// NeedsHardwareInspection looks at the state of the host to determine
9781011
// if hardware inspection should be run.
9791012
func (host *BareMetalHost) NeedsHardwareInspection() bool {
@@ -987,6 +1020,11 @@ func (host *BareMetalHost) NeedsHardwareInspection() bool {
9871020
// this host, because we don't want to reboot it.
9881021
return false
9891022
}
1023+
if host.InspectionDisabled() {
1024+
// Never perform inspection if it's explicitly disabled
1025+
return false
1026+
}
1027+
// FIXME(dtantsur): the HardwareDetails field is deprecated.
9901028
return host.Status.HardwareDetails == nil
9911029
}
9921030

‎apis/metal3.io/v1alpha1/baremetalhost_types_test.go‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -665,3 +665,55 @@ func TestHasBMCDetails(t *testing.T) {
665665
})
666666
}
667667
}
668+
669+
func TestInspectionDisabled(t *testing.T) {
670+
for _, tc := range []struct {
671+
Scenario string
672+
InspectionMode InspectionMode
673+
Annotations map[string]string
674+
Expected bool
675+
}{
676+
{
677+
Scenario: "default",
678+
},
679+
{
680+
Scenario: "annotation",
681+
Annotations: map[string]string{
682+
InspectAnnotationPrefix: "disabled",
683+
},
684+
Expected: true,
685+
},
686+
{
687+
Scenario: "empty annotation value",
688+
Annotations: map[string]string{
689+
InspectAnnotationPrefix: "",
690+
},
691+
},
692+
{
693+
Scenario: "InspectionMode",
694+
InspectionMode: InspectionModeDisabled,
695+
Expected: true,
696+
},
697+
{
698+
Scenario: "both",
699+
Annotations: map[string]string{
700+
InspectAnnotationPrefix: "disabled",
701+
},
702+
InspectionMode: InspectionModeDisabled,
703+
Expected: true,
704+
},
705+
} {
706+
t.Run(tc.Scenario, func(t *testing.T) {
707+
host := BareMetalHost{
708+
ObjectMeta: metav1.ObjectMeta{
709+
Annotations: tc.Annotations,
710+
},
711+
Spec: BareMetalHostSpec{
712+
InspectionMode: tc.InspectionMode,
713+
},
714+
}
715+
actual := host.InspectionDisabled()
716+
assert.Equal(t, tc.Expected, actual)
717+
})
718+
}
719+
}

‎config/base/crds/bases/metal3.io_baremetalhosts.yaml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,15 @@ spec:
288288
required:
289289
- url
290290
type: object
291+
inspectionMode:
292+
description: |-
293+
Specifies the mode for host inspection.
294+
"disabled" - no inspection will be performed
295+
"agent" - normal agent-based inspection will run
296+
enum:
297+
- disabled
298+
- agent
299+
type: string
291300
metaData:
292301
description: |-
293302
MetaData holds the reference to the Secret containing host metadata

‎config/render/capm3.yaml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,15 @@ spec:
288288
required:
289289
- url
290290
type: object
291+
inspectionMode:
292+
description: |-
293+
Specifies the mode for host inspection.
294+
"disabled" - no inspection will be performed
295+
"agent" - normal agent-based inspection will run
296+
enum:
297+
- disabled
298+
- agent
299+
type: string
291300
metaData:
292301
description: |-
293302
MetaData holds the reference to the Secret containing host metadata

‎internal/controller/metal3.io/baremetalhost_controller.go‎

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ func (r *BareMetalHostReconciler) Reconcile(ctx context.Context, request ctrl.Re
269269
// inspect.metal3.io=disabled or there are no existing HardwareDetails.
270270
func (r *BareMetalHostReconciler) updateHardwareDetails(ctx context.Context, request ctrl.Request, host *metal3api.BareMetalHost) (bool, error) {
271271
updated := false
272-
if host.Status.HardwareDetails == nil || inspectionDisabled(host) {
272+
if host.Status.HardwareDetails == nil || host.InspectionDisabled() {
273273
objHardwareDetails, err := r.getHardwareDetailsFromAnnotation(host)
274274
if err != nil {
275275
return updated, fmt.Errorf("error parsing HardwareDetails from annotation: %w", err)
@@ -446,16 +446,9 @@ func clearRebootAnnotations(host *metal3api.BareMetalHost) (dirty bool) {
446446
return
447447
}
448448

449-
// inspectionDisabled checks for existence of inspect.metal3.io=disabled
450-
// which means we don't inspect even in Inspecting state.
451-
func inspectionDisabled(host *metal3api.BareMetalHost) bool {
452-
annotations := host.GetAnnotations()
453-
return annotations[metal3api.InspectAnnotationPrefix] == metal3api.InspectAnnotationValueDisabled
454-
}
455-
456-
// hasInspectAnnotation checks for existence of inspect.metal3.io annotation
457-
// and returns true if it exist.
458-
func hasInspectAnnotation(host *metal3api.BareMetalHost) bool {
449+
// inspectionRefreshRequested checks for existence of inspect.metal3.io
450+
// annotation and returns true if it exist.
451+
func inspectionRefreshRequested(host *metal3api.BareMetalHost) bool {
459452
annotations := host.GetAnnotations()
460453
if annotations != nil {
461454
if expect, ok := annotations[metal3api.InspectAnnotationPrefix]; ok && expect != metal3api.InspectAnnotationValueDisabled {
@@ -974,15 +967,15 @@ func updateRootDeviceHints(host *metal3api.BareMetalHost, info *reconcileInfo) (
974967
func (r *BareMetalHostReconciler) actionInspecting(prov provisioner.Provisioner, info *reconcileInfo) actionResult {
975968
info.log.Info("inspecting hardware")
976969

977-
if inspectionDisabled(info.host) {
978-
info.log.Info("inspection disabled by annotation")
979-
info.publishEvent("InspectionSkipped", "disabled by annotation")
970+
if info.host.InspectionDisabled() {
971+
info.log.Info("inspection disabled by user")
972+
info.publishEvent("InspectionSkipped", "disabled by user")
980973
return actionComplete{}
981974
}
982975

983976
info.log.Info("inspecting hardware")
984977

985-
refresh := hasInspectAnnotation(info.host)
978+
refresh := inspectionRefreshRequested(info.host)
986979
forceReboot, _ := hasRebootAnnotation(info, true)
987980

988981
provResult, started, details, err := prov.InspectHardware(
@@ -1004,7 +997,7 @@ func (r *BareMetalHostReconciler) actionInspecting(prov provisioner.Provisioner,
1004997
dirty := false
1005998

1006999
// Delete inspect annotation if exists
1007-
if hasInspectAnnotation(info.host) {
1000+
if inspectionRefreshRequested(info.host) {
10081001
delete(info.host.Annotations, metal3api.InspectAnnotationPrefix)
10091002
dirty = true
10101003
}

‎internal/controller/metal3.io/baremetalhost_controller_test.go‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -592,16 +592,6 @@ func TestSetLastUpdated(t *testing.T) {
592592
)
593593
}
594594

595-
func TestInspectionDisabledAnnotation(t *testing.T) {
596-
host := newDefaultHost(t)
597-
host.Annotations = make(map[string]string)
598-
599-
assert.False(t, inspectionDisabled(host))
600-
601-
host.Annotations[metal3api.InspectAnnotationPrefix] = "disabled"
602-
assert.True(t, inspectionDisabled(host))
603-
}
604-
605595
func makeReconcileInfo(host *metal3api.BareMetalHost) *reconcileInfo {
606596
return &reconcileInfo{
607597
log: logf.Log.WithName("controllers").WithName("BareMetalHost").WithName("baremetal_controller"),

‎internal/controller/metal3.io/host_state_machine.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ func (hsm *hostStateMachine) handleRegistering(_ *reconcileInfo) actionResult {
408408
// if the credentials change and the Host must be re-registered.
409409
if hsm.Host.Spec.ExternallyProvisioned {
410410
hsm.NextState = metal3api.StateExternallyProvisioned
411-
} else if inspectionDisabled(hsm.Host) {
411+
} else if hsm.Host.InspectionDisabled() {
412412
hsm.NextState = metal3api.StatePreparing
413413
} else {
414414
hsm.NextState = metal3api.StateInspecting
@@ -439,8 +439,7 @@ func (hsm *hostStateMachine) handleExternallyProvisioned(info *reconcileInfo) ac
439439
return hsm.Reconciler.actionManageSteadyState(hsm.Provisioner, info)
440440
}
441441

442-
// TODO(dtantsur): move this logic inside NeedsHardwareInspection?
443-
if hsm.Host.NeedsHardwareInspection() && !inspectionDisabled(hsm.Host) {
442+
if hsm.Host.NeedsHardwareInspection() {
444443
hsm.NextState = metal3api.StateInspecting
445444
} else {
446445
hsm.NextState = metal3api.StatePreparing
@@ -464,7 +463,7 @@ func (hsm *hostStateMachine) handleAvailable(info *reconcileInfo) actionResult {
464463
return actionComplete{}
465464
}
466465

467-
if hasInspectAnnotation(hsm.Host) {
466+
if inspectionRefreshRequested(hsm.Host) {
468467
hsm.NextState = metal3api.StateInspecting
469468
return actionComplete{}
470469
}

‎internal/webhooks/metal3.io/v1alpha1/baremetalhost_validation.go‎

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,10 @@ func (webhook *BareMetalHost) validateHost(host *metal3api.BareMetalHost) []erro
8282
errs = append(errs, annotationErrors...)
8383
}
8484

85+
if err := validateInspectionMode(host); err != nil {
86+
errs = append(errs, err)
87+
}
88+
8589
if err := validatePowerStatus(host); err != nil {
8690
errs = append(errs, err)
8791
}
@@ -219,8 +223,7 @@ func validateAnnotations(host *metal3api.BareMetalHost) []error {
219223
case annotation == metal3api.InspectAnnotationPrefix:
220224
err = validateInspectAnnotation(value)
221225
case annotation == metal3api.HardwareDetailsAnnotation:
222-
inspect := host.Annotations[metal3api.InspectAnnotationPrefix]
223-
err = validateHwdDetailsAnnotation(value, inspect)
226+
err = validateHwdDetailsAnnotation(value, host.InspectionDisabled())
224227
default:
225228
err = nil
226229
}
@@ -298,13 +301,14 @@ func checkStatusAnnotation(bmhStatus *metal3api.BareMetalHostStatus) error {
298301
return nil
299302
}
300303

301-
func validateHwdDetailsAnnotation(hwdDetAnnotation string, inspect string) error {
304+
func validateHwdDetailsAnnotation(hwdDetAnnotation string, inspectionDisabled bool) error {
302305
if hwdDetAnnotation == "" {
303306
return nil
304307
}
305308

306-
if inspect != "disabled" {
307-
return errors.New("when hardware details are provided, the inspect.metal3.io annotation must be set to disabled")
309+
// Check both the annotation and the new field for disabled inspection
310+
if !inspectionDisabled {
311+
return errors.New("when hardware details are provided, inspection must be disabled (either via inspect.metal3.io annotation or inspectionMode field)")
308312
}
309313

310314
objHwdDet := &metal3api.HardwareDetails{}
@@ -380,6 +384,31 @@ func (webhook *BareMetalHost) validateCrossNamespaceSecretReferences(host *metal
380384
return errs
381385
}
382386

387+
func validateInspectionMode(host *metal3api.BareMetalHost) error {
388+
inspectAnnotation := host.Annotations[metal3api.InspectAnnotationPrefix]
389+
390+
// Check for contradicting values when both are set
391+
if inspectAnnotation != "" && host.Spec.InspectionMode != "" {
392+
// Annotation logic: "disabled" means disabled, anything else means enabled
393+
annotationDisabled := inspectAnnotation == metal3api.InspectAnnotationValueDisabled
394+
fieldDisabled := host.Spec.InspectionMode == metal3api.InspectionModeDisabled
395+
396+
if annotationDisabled != fieldDisabled {
397+
return errors.New("inspect.metal3.io annotation and inspectionMode field have contradicting values")
398+
}
399+
}
400+
401+
// If InspectionMode is set, it must be a valid value (kubebuilder validation handles this for the API,
402+
// but we validate here for safety)
403+
if host.Spec.InspectionMode != "" &&
404+
host.Spec.InspectionMode != metal3api.InspectionModeDisabled &&
405+
host.Spec.InspectionMode != metal3api.InspectionModeAgent {
406+
return fmt.Errorf("invalid inspectionMode value: %s, allowed values are 'disabled' or 'agent'", host.Spec.InspectionMode)
407+
}
408+
409+
return nil
410+
}
411+
383412
func validatePowerStatus(host *metal3api.BareMetalHost) error {
384413
if host.Spec.DisablePowerOff && !host.Spec.Online {
385414
return errors.New("node can't simultaneously have online set to false and have power off disabled")

0 commit comments

Comments
 (0)