Skip to content

Commit 805e6fc

Browse files
committed
fix: propagate snapshot close error, improve endpoint tracing, rename endpoint to host
1 parent edc9f91 commit 805e6fc

7 files changed

Lines changed: 27 additions & 16 deletions

File tree

api/v1alpha1/hostedcontrolplane.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,12 @@ func (a *Audit) ModeOrDefault() string {
5252
return ptr.Deref(a.Mode, defaultMode)
5353
}
5454

55-
func (ebs *ETCDBackupSecret) EndpointKeyOrDefault() string {
56-
defaultKey := "endpoint"
55+
func (ebs *ETCDBackupSecret) HostKeyOrDefault() string {
56+
defaultKey := "host"
5757
if ebs == nil {
5858
return defaultKey
5959
}
60-
return ptr.Deref(ebs.EndpointKey, defaultKey)
60+
return ptr.Deref(ebs.HostKey, defaultKey)
6161
}
6262

6363
func (ebs *ETCDBackupSecret) RegionKeyOrDefault() string {

api/v1alpha1/hostedcontrolplane_types.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ type ETCDBackupSecret struct {
169169
//+kubebuilder:validation:Optional
170170
SecretAccessKeyKey *string `json:"secretAccessKeyKey,omitempty"`
171171
//+kubebuilder:validation:Optional
172-
EndpointKey *string `json:"endpointKey,omitempty"`
172+
HostKey *string `json:"hostKey,omitempty"`
173173
//+kubebuilder:validation:Optional
174174
RegionKey *string `json:"regionKey,omitempty"`
175175
// The bucket will be split by `/` and only the first part is the bucket,

api/v1alpha1/zz_generated.deepcopy.go

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

pkg/reconcilers/etcd_cluster/etcd_client/etcd_client.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ var (
3434

3535
type EtcdClient interface {
3636
GetStatuses(ctx context.Context) (map[string]*clientv3.StatusResponse, error)
37-
CreateSnapshot(ctx context.Context) (*clientv3.SnapshotResponse, func(), error)
37+
CreateSnapshot(ctx context.Context) (*clientv3.SnapshotResponse, func() error, error)
3838
ListAlarms(ctx context.Context) (*clientv3.AlarmResponse, error)
3939
DisarmAlarm(ctx context.Context, alarm *clientv3.AlarmMember) error
4040
}
@@ -108,19 +108,24 @@ func (e *etcdClient) GetStatuses(ctx context.Context) (map[string]*clientv3.Stat
108108
)
109109
}
110110

111-
func (e *etcdClient) CreateSnapshot(ctx context.Context) (*clientv3.SnapshotResponse, func(), error) {
111+
func (e *etcdClient) CreateSnapshot(ctx context.Context) (*clientv3.SnapshotResponse, func() error, error) {
112112
return tracing.WithSpan3(ctx, tracer, "EtcdClient.CreateSnapshot",
113-
func(ctx context.Context, span trace.Span) (_ *clientv3.SnapshotResponse, _ func(), retErr error) {
113+
func(ctx context.Context, span trace.Span) (_ *clientv3.SnapshotResponse, _ func() error, retErr error) {
114114
endpoint := e.anyEndpoint
115115
etcdClient, err := createEtcdClient(e, endpoint)
116-
closeFunc := func() { closeEtcdClient(etcdClient, &retErr, endpoint) }
116+
closeFunc := func() error {
117+
var closeErr error
118+
closeEtcdClient(etcdClient, &closeErr, endpoint)
119+
return closeErr
120+
}
117121
if err != nil {
118122
return nil, closeFunc, err
119123
}
120124

121125
snapshotResponse, err := callETCDFuncOnAnyMember(
122126
ctx,
123127
etcdClient,
128+
endpoint,
124129
clientv3.Client.SnapshotWithVersion,
125130
)
126131
return snapshotResponse, closeFunc, err
@@ -183,6 +188,7 @@ func (e *etcdClient) ListAlarms(ctx context.Context) (*clientv3.AlarmResponse, e
183188
return callETCDFuncOnAnyMember(
184189
ctx,
185190
etcdClient,
191+
endpoint,
186192
clientv3.Client.AlarmList,
187193
)
188194
},
@@ -203,6 +209,7 @@ func (e *etcdClient) DisarmAlarm(ctx context.Context, alarm *clientv3.AlarmMembe
203209
_, err = callETCDFuncOnAnyMember(
204210
ctx,
205211
etcdClient,
212+
endpoint,
206213
func(client clientv3.Client, ctx context.Context) (*clientv3.AlarmResponse, error) {
207214
response, err := client.AlarmDisarm(ctx, alarm)
208215
if err != nil {
@@ -313,11 +320,12 @@ func callETCDFuncOnMember[R any](
313320
func callETCDFuncOnAnyMember[R any](
314321
ctx context.Context,
315322
etcdClient *clientv3.Client,
323+
endpoint string,
316324
etcdFunc func(client clientv3.Client, ctx context.Context) (R, error),
317325
) (R, error) {
318326
return tracing.WithSpan(ctx, tracer, "CallETCDFuncOnAnyMember",
319327
func(ctx context.Context, span trace.Span) (R, error) {
320-
return callETCDFuncOnMember(ctx, etcdClient, "",
328+
return callETCDFuncOnMember(ctx, etcdClient, endpoint,
321329
func(client clientv3.Client, ctx context.Context, _ string) (R, error) {
322330
return etcdFunc(client, ctx)
323331
})

pkg/reconcilers/etcd_cluster/reconciler.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ func (er *etcdClusterReconciler) reconcileETCDBackup(
319319
"Starting etcd backup",
320320
)
321321
snapshotResponse, closeClientFunc, err := etcdClient.CreateSnapshot(ctx)
322-
defer closeClientFunc()
322+
defer func() { err = errors.Join(err, closeClientFunc()) }()
323323
if err != nil {
324324
return fmt.Errorf("failed to create etcd snapshot: %w", err)
325325
}

pkg/reconcilers/etcd_cluster/s3_client/s3_client.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func NewS3Client(
7676
secretName := etcdBackupSecretConfig.Name
7777
accessKeyIDKey := etcdBackupSecretConfig.AccessKeyIDKeyOrDefault()
7878
secretAccessKeyKey := etcdBackupSecretConfig.SecretAccessKeyKeyOrDefault()
79-
endpointKey := etcdBackupSecretConfig.EndpointKeyOrDefault()
79+
endpointKey := etcdBackupSecretConfig.HostKeyOrDefault()
8080
regionKey := etcdBackupSecretConfig.RegionKeyOrDefault()
8181
bucketKey := etcdBackupSecretConfig.BucketKeyOrDefault()
8282
spanAttributes := []attribute.KeyValue{
@@ -119,6 +119,9 @@ func NewS3Client(
119119

120120
parts := strings.SplitN(bucketString, "/", 2)
121121
bucket := parts[0]
122+
if bucket == "" {
123+
return nil, fmt.Errorf("bucket secret key %q resolved to an empty bucket name", bucketKey)
124+
}
122125
prefix := ""
123126
if len(parts) == 2 {
124127
prefix = parts[1]

test/etcd_stubs.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,14 @@ func (s *EtcdClientStub) GetStatuses(_ context.Context) (map[string]*clientv3.St
3838
return s.StatusResponses, nil
3939
}
4040

41-
func (s *EtcdClientStub) CreateSnapshot(_ context.Context) (*clientv3.SnapshotResponse, func(), error) {
41+
func (s *EtcdClientStub) CreateSnapshot(_ context.Context) (*clientv3.SnapshotResponse, func() error, error) {
4242
if s.SnapshotError != nil {
43-
return nil, func() {}, s.SnapshotError
43+
return nil, func() error { return nil }, s.SnapshotError
4444
}
4545
return &clientv3.SnapshotResponse{
4646
Header: &etcdserverpb.ResponseHeader{ClusterId: 1},
4747
Snapshot: io.NopCloser(bytes.NewReader(EtcdSnapshotData)),
48-
}, func() {}, nil
48+
}, func() error { return nil }, nil
4949
}
5050

5151
func (s *EtcdClientStub) ListAlarms(_ context.Context) (*clientv3.AlarmResponse, error) {

0 commit comments

Comments
 (0)