Skip to content

Commit 4373790

Browse files
DanNiEShclaude
andcommitted
Add automatic reconnect on Ironic authentication failure
When gophercloud's built-in token refresh (ReauthFunc) fails — e.g. due to a network interruption or Keystone being temporarily unavailable — the Ironic client was left permanently stuck with a stale token until the pod restarted. This adds a fallback that detects 401/reauth errors, recreates the service client from scratch, and retries the operation once. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 3ad9e1a commit 4373790

4 files changed

Lines changed: 267 additions & 11 deletions

File tree

cmd/main.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,14 @@ limitations under the License.
1717
package main
1818

1919
import (
20+
"context"
2021
"crypto/tls"
2122
"flag"
2223
"fmt"
2324
"os"
2425
"path/filepath"
2526
"strconv"
27+
"time"
2628

2729
// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
2830
// to ensure that exec-entrypoint and run can make use of them.
@@ -233,7 +235,9 @@ func main() {
233235

234236
// Ironic client for bare metal management
235237
var ironicClient *ironic.Client
236-
if ironicClient, err = ironic.NewClient(); err != nil {
238+
ironicCtx, ironicCancel := context.WithTimeout(context.Background(), 30*time.Second)
239+
defer ironicCancel()
240+
if ironicClient, err = ironic.NewClient(ironicCtx); err != nil {
237241
setupLog.Error(err, "failed to create Ironic client")
238242
os.Exit(1)
239243
}

internal/ironic/client.go

Lines changed: 75 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,14 @@ package ironic
1919

2020
import (
2121
"context"
22+
"errors"
2223
"fmt"
24+
"net/http"
2325

2426
"github.com/gophercloud/gophercloud/v2"
2527
"github.com/gophercloud/gophercloud/v2/openstack/baremetal/v1/nodes"
2628
"github.com/gophercloud/utils/v2/openstack/clientconfig"
29+
ctrllog "sigs.k8s.io/controller-runtime/pkg/log"
2730
)
2831

2932
// NodeClient is the interface for interacting with Ironic nodes.
@@ -36,7 +39,8 @@ type NodeClient interface {
3639
// Client talks to Ironic over REST via gophercloud.
3740
type (
3841
Client struct {
39-
serviceClient *gophercloud.ServiceClient
42+
serviceClient *gophercloud.ServiceClient
43+
newServiceClient func(ctx context.Context) (*gophercloud.ServiceClient, error)
4044
}
4145

4246
TargetPowerState struct {
@@ -56,24 +60,77 @@ var (
5660
SoftRebooting = TargetPowerState{nodes.SoftRebooting}
5761
)
5862

59-
// NewClient creates an Ironic client with no-auth (typical for in-cluster Ironic with no Keystone).
60-
func NewClient() (*Client, error) {
61-
client, err := clientconfig.NewServiceClient(context.TODO(), "baremetal", nil)
63+
// NewClient creates an Ironic client configured via clouds.yaml or OS_* environment variables.
64+
func NewClient(ctx context.Context) (*Client, error) {
65+
factory := func(ctx context.Context) (*gophercloud.ServiceClient, error) {
66+
return clientconfig.NewServiceClient(ctx, "baremetal", nil)
67+
}
68+
sc, err := factory(ctx)
6269
if err != nil {
6370
return nil, fmt.Errorf("failed to create baremetal client: %w", err)
6471
}
65-
if client.Endpoint == "" {
72+
if sc == nil {
73+
return nil, fmt.Errorf("failed to create baremetal client: nil service client")
74+
}
75+
if sc.Endpoint == "" {
6676
return nil, fmt.Errorf("baremetal client has no endpoint configured")
6777
}
6878
return &Client{
69-
serviceClient: client,
79+
serviceClient: sc,
80+
newServiceClient: factory,
7081
}, nil
7182
}
7283

84+
func isAuthError(err error) bool {
85+
if err == nil {
86+
return false
87+
}
88+
if gophercloud.ResponseCodeIs(err, http.StatusUnauthorized) {
89+
return true
90+
}
91+
var errReauth *gophercloud.ErrUnableToReauthenticate
92+
if errors.As(err, &errReauth) {
93+
return true
94+
}
95+
var errAfterReauth *gophercloud.ErrErrorAfterReauthentication
96+
return errors.As(err, &errAfterReauth)
97+
}
98+
99+
func (c *Client) reconnect(ctx context.Context) error {
100+
log := ctrllog.FromContext(ctx)
101+
log.Info("recreating ironic service client after authentication failure")
102+
sc, err := c.newServiceClient(ctx)
103+
if err != nil {
104+
log.Error(err, "failed to recreate ironic service client")
105+
return fmt.Errorf("failed to recreate baremetal client: %w", err)
106+
}
107+
if sc == nil {
108+
return fmt.Errorf("failed to recreate baremetal client: nil service client")
109+
}
110+
if sc.Endpoint == "" {
111+
return fmt.Errorf("recreated baremetal client has no endpoint configured")
112+
}
113+
c.serviceClient = sc
114+
log.Info("ironic service client reconnected successfully", "endpoint", sc.Endpoint)
115+
return nil
116+
}
117+
73118
// GetNode fetches a node by UUID or name from Ironic.
74119
func (c *Client) GetNode(ctx context.Context, nodeID string) (*nodes.Node, error) {
120+
log := ctrllog.FromContext(ctx)
75121
node, err := nodes.Get(ctx, c.serviceClient, nodeID).Extract()
76122
if err != nil {
123+
if isAuthError(err) {
124+
log.Info("auth error on GetNode, attempting reconnect", "nodeID", nodeID, "error", err)
125+
if reconnErr := c.reconnect(ctx); reconnErr != nil {
126+
return nil, fmt.Errorf("get node %s: reconnect failed: %w", nodeID, reconnErr)
127+
}
128+
node, err = nodes.Get(ctx, c.serviceClient, nodeID).Extract()
129+
if err != nil {
130+
return nil, fmt.Errorf("get node %s after reconnect: %w", nodeID, err)
131+
}
132+
return node, nil
133+
}
77134
return nil, fmt.Errorf("get node %s: %w", nodeID, err)
78135
}
79136
return node, nil
@@ -91,8 +148,20 @@ func (c *Client) IsNodePowerTransitioning(node *nodes.Node) bool {
91148

92149
// SetPowerState requests power on or off for the node via Ironic.
93150
func (c *Client) SetPowerState(ctx context.Context, nodeID string, target TargetPowerState) error {
151+
log := ctrllog.FromContext(ctx)
94152
res := nodes.ChangePowerState(ctx, c.serviceClient, nodeID, nodes.PowerStateOpts{Target: target.powerstate})
95153
if err := res.ExtractErr(); err != nil {
154+
if isAuthError(err) {
155+
log.Info("auth error on SetPowerState, attempting reconnect", "nodeID", nodeID, "target", target.String(), "error", err)
156+
if reconnErr := c.reconnect(ctx); reconnErr != nil {
157+
return fmt.Errorf("failed to set power state on node %s: reconnect failed: %w", nodeID, reconnErr)
158+
}
159+
res = nodes.ChangePowerState(ctx, c.serviceClient, nodeID, nodes.PowerStateOpts{Target: target.powerstate})
160+
if err := res.ExtractErr(); err != nil {
161+
return fmt.Errorf("failed to set power state on node %s after reconnect: %w", nodeID, err)
162+
}
163+
return nil
164+
}
96165
return fmt.Errorf("failed to set power state on node %s: %w", nodeID, err)
97166
}
98167
return nil
Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
/*
2+
Copyright 2026.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package ironic
18+
19+
import (
20+
"context"
21+
"fmt"
22+
"net/http"
23+
24+
. "github.com/onsi/ginkgo/v2"
25+
. "github.com/onsi/gomega"
26+
27+
"github.com/gophercloud/gophercloud/v2"
28+
)
29+
30+
const (
31+
oldEndpoint = "http://old:6385/v1/"
32+
newEndpoint = "http://new:6385/v1/"
33+
)
34+
35+
var _ = Describe("isAuthError", func() {
36+
It("should return false for nil error", func() {
37+
Expect(isAuthError(nil)).To(BeFalse())
38+
})
39+
40+
It("should return false for generic errors", func() {
41+
Expect(isAuthError(fmt.Errorf("some random error"))).To(BeFalse())
42+
})
43+
44+
It("should return true for 401 ErrUnexpectedResponseCode", func() {
45+
err := gophercloud.ErrUnexpectedResponseCode{
46+
Actual: http.StatusUnauthorized,
47+
Expected: []int{http.StatusOK},
48+
}
49+
Expect(isAuthError(err)).To(BeTrue())
50+
})
51+
52+
It("should return false for 404 ErrUnexpectedResponseCode", func() {
53+
err := gophercloud.ErrUnexpectedResponseCode{
54+
Actual: http.StatusNotFound,
55+
Expected: []int{http.StatusOK},
56+
}
57+
Expect(isAuthError(err)).To(BeFalse())
58+
})
59+
60+
It("should return false for 500 ErrUnexpectedResponseCode", func() {
61+
err := gophercloud.ErrUnexpectedResponseCode{
62+
Actual: http.StatusInternalServerError,
63+
Expected: []int{http.StatusOK},
64+
}
65+
Expect(isAuthError(err)).To(BeFalse())
66+
})
67+
68+
It("should return true for *ErrUnableToReauthenticate (pointer, as gophercloud returns)", func() {
69+
err := &gophercloud.ErrUnableToReauthenticate{
70+
ErrOriginal: fmt.Errorf("original"),
71+
ErrReauth: fmt.Errorf("reauth failed"),
72+
}
73+
Expect(isAuthError(err)).To(BeTrue())
74+
})
75+
76+
It("should return true for *ErrErrorAfterReauthentication (pointer, as gophercloud returns)", func() {
77+
err := &gophercloud.ErrErrorAfterReauthentication{
78+
ErrOriginal: fmt.Errorf("still failing"),
79+
}
80+
Expect(isAuthError(err)).To(BeTrue())
81+
})
82+
83+
It("should return true for wrapped *ErrUnableToReauthenticate", func() {
84+
inner := &gophercloud.ErrUnableToReauthenticate{
85+
ErrOriginal: fmt.Errorf("original"),
86+
ErrReauth: fmt.Errorf("reauth failed"),
87+
}
88+
wrapped := fmt.Errorf("operation failed: %w", inner)
89+
Expect(isAuthError(wrapped)).To(BeTrue())
90+
})
91+
92+
It("should return true for wrapped *ErrErrorAfterReauthentication", func() {
93+
inner := &gophercloud.ErrErrorAfterReauthentication{
94+
ErrOriginal: fmt.Errorf("still failing"),
95+
}
96+
wrapped := fmt.Errorf("operation failed: %w", inner)
97+
Expect(isAuthError(wrapped)).To(BeTrue())
98+
})
99+
100+
It("should return true for wrapped 401 error", func() {
101+
inner := gophercloud.ErrUnexpectedResponseCode{
102+
Actual: http.StatusUnauthorized,
103+
Expected: []int{http.StatusOK},
104+
}
105+
wrapped := fmt.Errorf("operation failed: %w", inner)
106+
Expect(isAuthError(wrapped)).To(BeTrue())
107+
})
108+
109+
It("should return false for wrapped non-auth error", func() {
110+
inner := gophercloud.ErrUnexpectedResponseCode{
111+
Actual: http.StatusNotFound,
112+
Expected: []int{http.StatusOK},
113+
}
114+
wrapped := fmt.Errorf("operation failed: %w", inner)
115+
Expect(isAuthError(wrapped)).To(BeFalse())
116+
})
117+
})
118+
119+
var _ = Describe("reconnect", func() {
120+
It("should swap the service client on success", func() {
121+
oldSC := &gophercloud.ServiceClient{Endpoint: oldEndpoint}
122+
newSC := &gophercloud.ServiceClient{Endpoint: newEndpoint}
123+
124+
c := &Client{
125+
serviceClient: oldSC,
126+
newServiceClient: func(context.Context) (*gophercloud.ServiceClient, error) {
127+
return newSC, nil
128+
},
129+
}
130+
131+
Expect(c.reconnect(context.Background())).To(Succeed())
132+
Expect(c.serviceClient).To(Equal(newSC))
133+
Expect(c.GetEndpoint()).To(Equal(newEndpoint))
134+
})
135+
136+
It("should return error when factory fails", func() {
137+
oldSC := &gophercloud.ServiceClient{Endpoint: oldEndpoint}
138+
139+
c := &Client{
140+
serviceClient: oldSC,
141+
newServiceClient: func(context.Context) (*gophercloud.ServiceClient, error) {
142+
return nil, fmt.Errorf("keystone is down")
143+
},
144+
}
145+
146+
err := c.reconnect(context.Background())
147+
Expect(err).To(HaveOccurred())
148+
Expect(err.Error()).To(ContainSubstring("keystone is down"))
149+
Expect(c.serviceClient).To(Equal(oldSC), "should keep old client on failure")
150+
})
151+
152+
It("should return error when factory returns nil client without error", func() {
153+
oldSC := &gophercloud.ServiceClient{Endpoint: oldEndpoint}
154+
155+
c := &Client{
156+
serviceClient: oldSC,
157+
newServiceClient: func(context.Context) (*gophercloud.ServiceClient, error) {
158+
return nil, nil
159+
},
160+
}
161+
162+
err := c.reconnect(context.Background())
163+
Expect(err).To(HaveOccurred())
164+
Expect(err.Error()).To(ContainSubstring("nil service client"))
165+
Expect(c.serviceClient).To(Equal(oldSC), "should keep old client on nil return")
166+
})
167+
168+
It("should return error when new client has empty endpoint", func() {
169+
oldSC := &gophercloud.ServiceClient{Endpoint: oldEndpoint}
170+
171+
c := &Client{
172+
serviceClient: oldSC,
173+
newServiceClient: func(context.Context) (*gophercloud.ServiceClient, error) {
174+
return &gophercloud.ServiceClient{Endpoint: ""}, nil
175+
},
176+
}
177+
178+
err := c.reconnect(context.Background())
179+
Expect(err).To(HaveOccurred())
180+
Expect(err.Error()).To(ContainSubstring("no endpoint configured"))
181+
})
182+
})

internal/ironic/client_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package ironic_test
1818

1919
import (
20+
"context"
2021
"os"
2122
"testing"
2223

@@ -106,7 +107,7 @@ var _ = Describe("Client", func() {
106107
})
107108

108109
It("should return an error when no credentials are available", func() {
109-
client, err := ironic.NewClient()
110+
client, err := ironic.NewClient(context.Background())
110111
Expect(err).To(HaveOccurred())
111112
Expect(err.Error()).To(ContainSubstring("failed to create baremetal client"))
112113
Expect(client).To(BeNil())
@@ -121,14 +122,14 @@ var _ = Describe("Client", func() {
121122
})
122123

123124
It("should create a client successfully", func() {
124-
client, err := ironic.NewClient()
125+
client, err := ironic.NewClient(context.Background())
125126
Expect(err).NotTo(HaveOccurred())
126127
Expect(client).NotTo(BeNil())
127128
Expect(client.GetEndpoint()).NotTo(BeEmpty())
128129
})
129130

130131
It("should have a non-empty endpoint", func() {
131-
client, err := ironic.NewClient()
132+
client, err := ironic.NewClient(context.Background())
132133
Expect(err).NotTo(HaveOccurred())
133134

134135
endpoint := client.GetEndpoint()
@@ -145,7 +146,7 @@ var _ = Describe("Client", func() {
145146
})
146147

147148
It("should return a valid HTTP(S) endpoint", func() {
148-
client, err := ironic.NewClient()
149+
client, err := ironic.NewClient(context.Background())
149150
Expect(err).NotTo(HaveOccurred())
150151

151152
endpoint := client.GetEndpoint()

0 commit comments

Comments
 (0)