Skip to content

Commit

Permalink
Close etcd leaky connection if client is not initialized
Browse files Browse the repository at this point in the history
Currently, the `NewClient` method creates an Etcd client that is initialized and a go-routine is spawned off because it uses GPRC in the background. For the most part, CAPI handles closing this connection up in the stack.

However, if the `newEtcdClient` method fails, we will then return a `nil` client, the go-routine that was spawned off will leak and CAPI will not be able to end it.

This commit fixes that by forcing the `newEtcdClient` method to close connections if the client initialization fails.
  • Loading branch information
vpineda1996 committed May 25, 2022
1 parent 875d9e3 commit 958050c
Showing 1 changed file with 8 additions and 2 deletions.
10 changes: 8 additions & 2 deletions controlplane/kubeadm/internal/etcd/etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"go.etcd.io/etcd/api/v3/etcdserverpb"
clientv3 "go.etcd.io/etcd/client/v3"
"google.golang.org/grpc"
kerrors "k8s.io/apimachinery/pkg/util/errors"

"sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/proxy"
)
Expand Down Expand Up @@ -151,7 +152,12 @@ func NewClient(ctx context.Context, config ClientConfiguration) (*Client, error)
return nil, errors.Wrap(err, "unable to create etcd client")
}

return newEtcdClient(ctx, etcdClient)
client, err := newEtcdClient(ctx, etcdClient)
if err != nil {
closeErr := etcdClient.Close()
return nil, errors.Wrap(kerrors.NewAggregate([]error{err, closeErr}), "unable to create etcd client")
}
return client, err
}

func newEtcdClient(ctx context.Context, etcdClient etcd) (*Client, error) {
Expand All @@ -162,7 +168,7 @@ func newEtcdClient(ctx context.Context, etcdClient etcd) (*Client, error) {

status, err := etcdClient.Status(ctx, endpoints[0])
if err != nil {
return nil, err
return nil, errors.Wrap(err, "failed to get etcd status")
}

return &Client{
Expand Down

0 comments on commit 958050c

Please sign in to comment.