Skip to content

Commit 49981e4

Browse files
committed
Request microversion 1.109 for health API support
The Health field on Ironic nodes requires API microversion 1.109 or later. Without requesting this version, Ironic omits the field from responses and health is permanently reported as Unknown. - Add HasHealthAPI() feature check for microversion >= 1.109 - Update ChooseMicroversion() to negotiate 1.109 when available - Guard GetHealth() to skip the getNode() call when Ironic is too old - Suppress ErrNeedsRegistration log noise in GetHealth() since it is expected before host registration Signed-off-by: Jacob Anders <jacob-anders-dev@proton.me> Assisted-by: Claude (claude-4.6-opus-high-thinking) (cherry picked from commit 751c99a58201f39ba937ff19cfe79b8d5f0718ac)
1 parent 183367a commit 49981e4

4 files changed

Lines changed: 109 additions & 7 deletions

File tree

pkg/provisioner/ironic/clients/features.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ func (af AvailableFeatures) Log(logger logr.Logger) {
4545
"maxVersion", fmt.Sprintf("1.%d", af.MaxVersion),
4646
"chosenVersion", af.ChooseMicroversion(),
4747
"virtualMediaGET", af.HasVirtualMediaGetAPI(),
48-
"disablePowerOff", af.HasDisablePowerOff())
48+
"disablePowerOff", af.HasDisablePowerOff(),
49+
"healthAPI", af.HasHealthAPI())
4950
}
5051

5152
func (af AvailableFeatures) HasVirtualMediaGetAPI() bool {
@@ -56,7 +57,15 @@ func (af AvailableFeatures) HasDisablePowerOff() bool {
5657
return af.MaxVersion >= 95 //nolint:mnd
5758
}
5859

60+
func (af AvailableFeatures) HasHealthAPI() bool {
61+
return af.MaxVersion >= 109 //nolint:mnd
62+
}
63+
5964
func (af AvailableFeatures) ChooseMicroversion() string {
65+
if af.HasHealthAPI() {
66+
return "1.109"
67+
}
68+
6069
if af.HasDisablePowerOff() {
6170
return "1.95"
6271
}

pkg/provisioner/ironic/clients/features_test.go

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
)
77

88
func TestAvailableFeatures_ChooseMicroversion(t *testing.T) {
9-
microVersion := "1.95"
109
type fields struct {
1110
MaxVersion int
1211
}
@@ -16,25 +15,39 @@ func TestAvailableFeatures_ChooseMicroversion(t *testing.T) {
1615
want string
1716
}{
1817
{
19-
name: fmt.Sprintf("MaxVersion < %d return microversion %s", 89, baselineVersionString),
18+
name: fmt.Sprintf("MaxVersion < %d return microversion %s", baseline, baselineVersionString),
2019
feature: fields{
2120
MaxVersion: 50,
2221
},
2322
want: baselineVersionString,
2423
},
2524
{
26-
name: fmt.Sprintf("MaxVersion = %d return %s", 89, microVersion),
25+
name: "MaxVersion = 95 return 1.95",
2726
feature: fields{
2827
MaxVersion: 95,
2928
},
30-
want: microVersion,
29+
want: "1.95",
3130
},
3231
{
33-
name: fmt.Sprintf("MaxVersion > %d return %s", 89, microVersion),
32+
name: "MaxVersion between 95 and 109 return 1.95",
3433
feature: fields{
3534
MaxVersion: 100,
3635
},
37-
want: microVersion,
36+
want: "1.95",
37+
},
38+
{
39+
name: "MaxVersion = 109 return 1.109",
40+
feature: fields{
41+
MaxVersion: 109,
42+
},
43+
want: "1.109",
44+
},
45+
{
46+
name: "MaxVersion > 109 return 1.109",
47+
feature: fields{
48+
MaxVersion: 120,
49+
},
50+
want: "1.109",
3851
},
3952
}
4053
for _, tt := range tests {

pkg/provisioner/ironic/ironic.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1990,3 +1990,20 @@ func (p *ironicProvisioner) HasPowerFailure(ctx context.Context) bool {
19901990
}
19911991
return node.Fault == "power failure"
19921992
}
1993+
1994+
// TODO(jacobanders): GetHealth calls getNode() which makes an additional HTTP
1995+
// request to Ironic on every reconcile. Consider passing the node object through
1996+
// computeConditions or caching it per reconcile cycle to avoid redundant calls.
1997+
func (p *ironicProvisioner) GetHealth(ctx context.Context) string {
1998+
if !p.availableFeatures.HasHealthAPI() {
1999+
return ""
2000+
}
2001+
node, err := p.getNode(ctx)
2002+
if err != nil {
2003+
if !errors.Is(err, provisioner.ErrNeedsRegistration) {
2004+
p.log.Error(err, "ignored error while checking health status")
2005+
}
2006+
return ""
2007+
}
2008+
return node.Health
2009+
}

pkg/provisioner/ironic/power_test.go

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,3 +462,66 @@ func TestSoftPowerOffFallback(t *testing.T) {
462462
require.Error(t, err)
463463
assert.ErrorAs(t, err, &softPowerOffUnsupportedError{})
464464
}
465+
466+
func TestGetHealth(t *testing.T) {
467+
nodeUUID := "33ce8659-7400-4c68-9535-d10766f07a58"
468+
cases := []struct {
469+
name string
470+
ironic *testserver.IronicMock
471+
expectedHealth string
472+
}{
473+
{
474+
name: "healthy node",
475+
ironic: testserver.NewIronic(t).Node(nodes.Node{
476+
UUID: nodeUUID,
477+
Health: provisioner.HealthOK,
478+
}),
479+
expectedHealth: provisioner.HealthOK,
480+
},
481+
{
482+
name: "warning health",
483+
ironic: testserver.NewIronic(t).Node(nodes.Node{
484+
UUID: nodeUUID,
485+
Health: provisioner.HealthWarning,
486+
}),
487+
expectedHealth: provisioner.HealthWarning,
488+
},
489+
{
490+
name: "critical health",
491+
ironic: testserver.NewIronic(t).Node(nodes.Node{
492+
UUID: nodeUUID,
493+
Health: provisioner.HealthCritical,
494+
}),
495+
expectedHealth: provisioner.HealthCritical,
496+
},
497+
{
498+
name: "empty health",
499+
ironic: testserver.NewIronic(t).Node(nodes.Node{
500+
UUID: nodeUUID,
501+
}),
502+
expectedHealth: "",
503+
},
504+
{
505+
name: "node not found returns empty",
506+
ironic: testserver.NewIronic(t).NoNode(nodeUUID),
507+
expectedHealth: "",
508+
},
509+
}
510+
511+
for _, tc := range cases {
512+
t.Run(tc.name, func(t *testing.T) {
513+
tc.ironic.Start()
514+
defer tc.ironic.Stop()
515+
516+
host := makeHost()
517+
host.Status.Provisioning.ID = nodeUUID
518+
auth := clients.AuthConfig{Type: clients.NoAuth}
519+
prov, err := newProvisionerWithSettings(host, bmc.Credentials{}, nullEventPublisher, tc.ironic.Endpoint(), auth)
520+
require.NoError(t, err)
521+
prov.availableFeatures = clients.AvailableFeatures{MaxVersion: 109}
522+
523+
health := prov.GetHealth(t.Context())
524+
assert.Equal(t, tc.expectedHealth, health)
525+
})
526+
}
527+
}

0 commit comments

Comments
 (0)