Skip to content

Commit 693a781

Browse files
DanNiEShclaude
DanNiESh
and
claude
committed
Add automatic reconnect on Ironic authentication failure
Port the reconnect logic from the old internal/ironic package (PR #7) into the new internal/management layer. When gophercloud's built-in token refresh fails, the client detects auth errors (401, reauth failures), recreates the service client from scratch, and retries the operation once. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent a126183 commit 693a781

2 files changed

Lines changed: 238 additions & 16 deletions

File tree

‎internal/management/openstack.go‎

Lines changed: 97 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@ package management
33
import (
44
"context"
55
"encoding/json"
6+
"errors"
67
"fmt"
78
"net/http"
89

910
"github.com/gophercloud/gophercloud/v2"
1011
"github.com/gophercloud/gophercloud/v2/openstack"
1112
"github.com/gophercloud/gophercloud/v2/openstack/baremetal/v1/nodes"
1213
"github.com/gophercloud/utils/v2/openstack/clientconfig"
14+
ctrllog "sigs.k8s.io/controller-runtime/pkg/log"
1315
)
1416

1517
var (
@@ -22,7 +24,8 @@ func init() {
2224
}
2325

2426
type OpenStackClient struct {
25-
client *gophercloud.ServiceClient
27+
client *gophercloud.ServiceClient
28+
newServiceClient func(ctx context.Context) (*gophercloud.ServiceClient, error)
2629
}
2730

2831
func NewOpenStackClient(ctx context.Context, cfg *Config) (Client, error) {
@@ -39,36 +42,97 @@ func NewOpenStackClient(ctx context.Context, cfg *Config) (Client, error) {
3942
}
4043
}
4144

42-
clientOpts := clientconfig.ClientOpts{
43-
Cloud: cloud.Cloud,
44-
AuthType: cloud.AuthType,
45-
AuthInfo: cloud.AuthInfo,
46-
RegionName: cloud.RegionName,
47-
EndpointType: cloud.EndpointType,
45+
factory := func(ctx context.Context) (*gophercloud.ServiceClient, error) {
46+
clientOpts := clientconfig.ClientOpts{
47+
Cloud: cloud.Cloud,
48+
AuthType: cloud.AuthType,
49+
AuthInfo: cloud.AuthInfo,
50+
RegionName: cloud.RegionName,
51+
EndpointType: cloud.EndpointType,
52+
}
53+
54+
providerClient, err := clientconfig.AuthenticatedClient(ctx, &clientOpts)
55+
if err != nil {
56+
return nil, fmt.Errorf("failed to create authenticated client (check cloud credentials and endpoint configuration)")
57+
}
58+
59+
ironicClient, err := openstack.NewBareMetalV1(providerClient, gophercloud.EndpointOpts{
60+
Region: cloud.RegionName,
61+
Availability: gophercloud.Availability(cloud.EndpointType),
62+
})
63+
if err != nil {
64+
return nil, fmt.Errorf("failed to create baremetal client (check endpoint configuration and region)")
65+
}
66+
67+
return ironicClient, nil
4868
}
4969

50-
providerClient, err := clientconfig.AuthenticatedClient(ctx, &clientOpts)
70+
sc, err := factory(ctx)
5171
if err != nil {
52-
return nil, fmt.Errorf("failed to create authenticated client (check cloud credentials and endpoint configuration)")
72+
return nil, err
5373
}
5474

55-
ironicClient, err := openstack.NewBareMetalV1(providerClient, gophercloud.EndpointOpts{
56-
Region: cloud.RegionName,
57-
Availability: gophercloud.Availability(cloud.EndpointType),
58-
})
59-
if err != nil {
60-
return nil, fmt.Errorf("failed to create baremetal client (check endpoint configuration and region)")
75+
return &OpenStackClient{
76+
client: sc,
77+
newServiceClient: factory,
78+
}, nil
79+
}
80+
81+
func isAuthError(err error) bool {
82+
if err == nil {
83+
return false
6184
}
85+
if gophercloud.ResponseCodeIs(err, http.StatusUnauthorized) {
86+
return true
87+
}
88+
var errReauth *gophercloud.ErrUnableToReauthenticate
89+
if errors.As(err, &errReauth) {
90+
return true
91+
}
92+
var errAfterReauth *gophercloud.ErrErrorAfterReauthentication
93+
return errors.As(err, &errAfterReauth)
94+
}
6295

63-
return &OpenStackClient{client: ironicClient}, nil
96+
func (c *OpenStackClient) reconnect(ctx context.Context) error {
97+
log := ctrllog.FromContext(ctx)
98+
log.Info("recreating ironic service client after authentication failure")
99+
sc, err := c.newServiceClient(ctx)
100+
if err != nil {
101+
return fmt.Errorf("failed to recreate baremetal client: %w", err)
102+
}
103+
if sc == nil {
104+
return fmt.Errorf("failed to recreate baremetal client: nil service client")
105+
}
106+
if sc.Endpoint == "" {
107+
return fmt.Errorf("recreated baremetal client has no endpoint configured")
108+
}
109+
c.client = sc
110+
log.Info("ironic service client reconnected successfully", "endpoint", sc.Endpoint)
111+
return nil
64112
}
65113

66114
func (c *OpenStackClient) GetPowerState(ctx context.Context, hostID string) (*PowerStatus, error) {
115+
log := ctrllog.FromContext(ctx)
67116
node, err := nodes.Get(ctx, c.client, hostID).Extract()
68117
if err != nil {
118+
if isAuthError(err) {
119+
log.Info("auth error on GetPowerState, attempting reconnect", "nodeID", hostID)
120+
if reconnErr := c.reconnect(ctx); reconnErr != nil {
121+
return nil, fmt.Errorf("get node %s: reconnect failed: %w", hostID, reconnErr)
122+
}
123+
node, err = nodes.Get(ctx, c.client, hostID).Extract()
124+
if err != nil {
125+
return nil, fmt.Errorf("get node %s after reconnect: %w", hostID, err)
126+
}
127+
return nodePowerStatus(node, hostID)
128+
}
69129
return nil, fmt.Errorf("get node %s: %w", hostID, err)
70130
}
71131

132+
return nodePowerStatus(node, hostID)
133+
}
134+
135+
func nodePowerStatus(node *nodes.Node, hostID string) (*PowerStatus, error) {
72136
state := PowerState(node.PowerState)
73137
switch state {
74138
case PowerOn, PowerOff:
@@ -83,6 +147,7 @@ func (c *OpenStackClient) GetPowerState(ctx context.Context, hostID string) (*Po
83147
}
84148

85149
func (c *OpenStackClient) SetPowerState(ctx context.Context, hostID string, target PowerState) error {
150+
log := ctrllog.FromContext(ctx)
86151
switch target {
87152
case PowerOn, PowerOff:
88153
default:
@@ -93,6 +158,22 @@ func (c *OpenStackClient) SetPowerState(ctx context.Context, hostID string, targ
93158
Target: nodes.TargetPowerState(target),
94159
})
95160
if err := res.ExtractErr(); err != nil {
161+
if isAuthError(err) {
162+
log.Info("auth error on SetPowerState, attempting reconnect", "nodeID", hostID, "target", target)
163+
if reconnErr := c.reconnect(ctx); reconnErr != nil {
164+
return fmt.Errorf("failed to set power state on node %s: reconnect failed: %w", hostID, reconnErr)
165+
}
166+
res = nodes.ChangePowerState(ctx, c.client, hostID, nodes.PowerStateOpts{
167+
Target: nodes.TargetPowerState(target),
168+
})
169+
if err := res.ExtractErr(); err != nil {
170+
if gophercloud.ResponseCodeIs(err, http.StatusConflict) {
171+
return fmt.Errorf("node %s: %w", hostID, ErrTransitioning)
172+
}
173+
return fmt.Errorf("failed to set power state on node %s after reconnect: %w", hostID, err)
174+
}
175+
return nil
176+
}
96177
if gophercloud.ResponseCodeIs(err, http.StatusConflict) {
97178
return fmt.Errorf("node %s: %w", hostID, ErrTransitioning)
98179
}

‎internal/management/openstack_internal_test.go‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
package management
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"net/http"
7+
8+
. "github.com/onsi/ginkgo/v2"
9+
. "github.com/onsi/gomega"
10+
11+
"github.com/gophercloud/gophercloud/v2"
12+
)
13+
14+
const (
15+
oldEndpoint = "http://old:6385/v1/"
16+
newEndpoint = "http://new:6385/v1/"
17+
)
18+
19+
var _ = Describe("isAuthError", func() {
20+
It("should return false for nil error", func() {
21+
Expect(isAuthError(nil)).To(BeFalse())
22+
})
23+
24+
It("should return false for generic errors", func() {
25+
Expect(isAuthError(fmt.Errorf("some random error"))).To(BeFalse())
26+
})
27+
28+
It("should return true for 401 ErrUnexpectedResponseCode", func() {
29+
err := gophercloud.ErrUnexpectedResponseCode{
30+
Actual: http.StatusUnauthorized,
31+
Expected: []int{http.StatusOK},
32+
}
33+
Expect(isAuthError(err)).To(BeTrue())
34+
})
35+
36+
It("should return false for 404 ErrUnexpectedResponseCode", func() {
37+
err := gophercloud.ErrUnexpectedResponseCode{
38+
Actual: http.StatusNotFound,
39+
Expected: []int{http.StatusOK},
40+
}
41+
Expect(isAuthError(err)).To(BeFalse())
42+
})
43+
44+
It("should return true for *ErrUnableToReauthenticate", func() {
45+
err := &gophercloud.ErrUnableToReauthenticate{
46+
ErrOriginal: fmt.Errorf("original"),
47+
ErrReauth: fmt.Errorf("reauth failed"),
48+
}
49+
Expect(isAuthError(err)).To(BeTrue())
50+
})
51+
52+
It("should return true for *ErrErrorAfterReauthentication", func() {
53+
err := &gophercloud.ErrErrorAfterReauthentication{
54+
ErrOriginal: fmt.Errorf("still failing"),
55+
}
56+
Expect(isAuthError(err)).To(BeTrue())
57+
})
58+
59+
It("should return true for wrapped 401 error", func() {
60+
inner := gophercloud.ErrUnexpectedResponseCode{
61+
Actual: http.StatusUnauthorized,
62+
Expected: []int{http.StatusOK},
63+
}
64+
wrapped := fmt.Errorf("operation failed: %w", inner)
65+
Expect(isAuthError(wrapped)).To(BeTrue())
66+
})
67+
68+
It("should return false for wrapped non-auth error", func() {
69+
inner := gophercloud.ErrUnexpectedResponseCode{
70+
Actual: http.StatusNotFound,
71+
Expected: []int{http.StatusOK},
72+
}
73+
wrapped := fmt.Errorf("operation failed: %w", inner)
74+
Expect(isAuthError(wrapped)).To(BeFalse())
75+
})
76+
})
77+
78+
var _ = Describe("reconnect", func() {
79+
It("should swap the service client on success", func() {
80+
oldSC := &gophercloud.ServiceClient{Endpoint: oldEndpoint}
81+
newSC := &gophercloud.ServiceClient{Endpoint: newEndpoint}
82+
83+
c := &OpenStackClient{
84+
client: oldSC,
85+
newServiceClient: func(context.Context) (*gophercloud.ServiceClient, error) {
86+
return newSC, nil
87+
},
88+
}
89+
90+
Expect(c.reconnect(context.Background())).To(Succeed())
91+
Expect(c.client).To(Equal(newSC))
92+
Expect(c.client.Endpoint).To(Equal(newEndpoint))
93+
})
94+
95+
It("should return error when factory fails", func() {
96+
oldSC := &gophercloud.ServiceClient{Endpoint: oldEndpoint}
97+
98+
c := &OpenStackClient{
99+
client: oldSC,
100+
newServiceClient: func(context.Context) (*gophercloud.ServiceClient, error) {
101+
return nil, fmt.Errorf("keystone is down")
102+
},
103+
}
104+
105+
err := c.reconnect(context.Background())
106+
Expect(err).To(HaveOccurred())
107+
Expect(err.Error()).To(ContainSubstring("keystone is down"))
108+
Expect(c.client).To(Equal(oldSC), "should keep old client on failure")
109+
})
110+
111+
It("should return error when factory returns nil client without error", func() {
112+
oldSC := &gophercloud.ServiceClient{Endpoint: oldEndpoint}
113+
114+
c := &OpenStackClient{
115+
client: oldSC,
116+
newServiceClient: func(context.Context) (*gophercloud.ServiceClient, error) {
117+
return nil, nil
118+
},
119+
}
120+
121+
err := c.reconnect(context.Background())
122+
Expect(err).To(HaveOccurred())
123+
Expect(err.Error()).To(ContainSubstring("nil service client"))
124+
Expect(c.client).To(Equal(oldSC), "should keep old client on nil return")
125+
})
126+
127+
It("should return error when new client has empty endpoint", func() {
128+
oldSC := &gophercloud.ServiceClient{Endpoint: oldEndpoint}
129+
130+
c := &OpenStackClient{
131+
client: oldSC,
132+
newServiceClient: func(context.Context) (*gophercloud.ServiceClient, error) {
133+
return &gophercloud.ServiceClient{Endpoint: ""}, nil
134+
},
135+
}
136+
137+
err := c.reconnect(context.Background())
138+
Expect(err).To(HaveOccurred())
139+
Expect(err.Error()).To(ContainSubstring("no endpoint configured"))
140+
})
141+
})

0 commit comments

Comments
 (0)