Skip to content

Commit 8672956

Browse files
committed
etcd: use Unix Domain socket for testserver
Choosing a port in advance is racy. A better solution is to use a Unix Domain socket in the per-etcd-instance data directory. Then the name can be determined in advance and there's no risk of conflicts with other etcd instances. With unix:// for the endpoint, we have to be a bit more careful about passing a TLS config to the etcd client library because for unix://, in contrast to http://, it tries to use an incomplete config which then fails to establish the connection.
1 parent 047e4c8 commit 8672956

File tree

4 files changed

+67
-73
lines changed

4 files changed

+67
-73
lines changed

cmd/kube-apiserver/app/testing/testserver.go

Lines changed: 42 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"crypto/elliptic"
2323
"crypto/rand"
2424
"crypto/rsa"
25+
"crypto/tls"
2526
"crypto/x509"
2627
"crypto/x509/pkix"
2728
"encoding/pem"
@@ -501,26 +502,9 @@ func StartTestServer(t ktesting.TB, instanceOptions *TestServerInstanceOptions,
501502
return result, fmt.Errorf("failed to wait for default namespace to be created: %v", err)
502503
}
503504

504-
tlsInfo := transport.TLSInfo{
505-
CertFile: storageConfig.Transport.CertFile,
506-
KeyFile: storageConfig.Transport.KeyFile,
507-
TrustedCAFile: storageConfig.Transport.TrustedCAFile,
508-
}
509-
tlsConfig, err := tlsInfo.ClientConfig()
505+
etcdClient, _, err := GetEtcdClients(storageConfig.Transport)
510506
if err != nil {
511-
return result, err
512-
}
513-
etcdConfig := clientv3.Config{
514-
Endpoints: storageConfig.Transport.ServerList,
515-
DialTimeout: 20 * time.Second,
516-
DialOptions: []grpc.DialOption{
517-
grpc.WithBlock(), // block until the underlying connection is up
518-
},
519-
TLS: tlsConfig,
520-
}
521-
etcdClient, err := clientv3.New(etcdConfig)
522-
if err != nil {
523-
return result, err
507+
return result, fmt.Errorf("create etcd client: %w", err)
524508
}
525509

526510
// from here the caller must call tearDown
@@ -538,6 +522,45 @@ func StartTestServer(t ktesting.TB, instanceOptions *TestServerInstanceOptions,
538522
return result, nil
539523
}
540524

525+
// GetEtcdClients returns an initialized etcd clientv3.Client and clientv3.KV.
526+
func GetEtcdClients(config storagebackend.TransportConfig) (*clientv3.Client, clientv3.KV, error) {
527+
// clientv3.New ignores an invalid TLS config for http://, but not for unix:// (https://github.com/etcd-io/etcd/blob/5a8fba466087686fc15815f5bc041fb7eb1f23ea/client/v3/internal/endpoint/endpoint.go#L61-L66).
528+
// To support unix://, we must not set Config.TLS unless we really have
529+
// transport security.
530+
var tlsConfig *tls.Config
531+
if config.CertFile != "" ||
532+
config.KeyFile != "" ||
533+
config.TrustedCAFile != "" {
534+
tlsInfo := transport.TLSInfo{
535+
CertFile: config.CertFile,
536+
KeyFile: config.KeyFile,
537+
TrustedCAFile: config.TrustedCAFile,
538+
}
539+
540+
var err error
541+
tlsConfig, err = tlsInfo.ClientConfig()
542+
if err != nil {
543+
return nil, nil, err
544+
}
545+
}
546+
547+
cfg := clientv3.Config{
548+
Endpoints: config.ServerList,
549+
DialTimeout: 20 * time.Second,
550+
DialOptions: []grpc.DialOption{
551+
grpc.WithBlock(), // block until the underlying connection is up
552+
},
553+
TLS: tlsConfig,
554+
}
555+
556+
c, err := clientv3.New(cfg)
557+
if err != nil {
558+
return nil, nil, err
559+
}
560+
561+
return c, clientv3.NewKV(c), nil
562+
}
563+
541564
// StartTestServerOrDie calls StartTestServer t.Fatal if it does not succeed.
542565
func StartTestServerOrDie(t testing.TB, instanceOptions *TestServerInstanceOptions, flags []string, storageConfig *storagebackend.Config) *TestServer {
543566
result, err := StartTestServer(t, instanceOptions, flags, storageConfig)

test/integration/framework/controlplane_utils.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,12 @@ func DefaultEtcdOptions() *options.EtcdOptions {
8686
}
8787

8888
// SharedEtcd creates a storage config for a shared etcd instance, with a unique prefix.
89+
//
90+
// The transport CertFile/KeyFile/TrustedCAFile will be empty for insecure connections.
91+
// In that case, *no* TLS config should be used because etcd would try to use
92+
// it for Unix Domain sockets (https://github.com/etcd-io/etcd/blob/5a8fba466087686fc15815f5bc041fb7eb1f23ea/client/v3/internal/endpoint/endpoint.go#L61-L66)
93+
// and fail to connect because the TLS config is insufficient. It works
94+
// for TCP because http disables using TLS.
8995
func SharedEtcd() *storagebackend.Config {
9096
cfg := storagebackend.NewDefaultConfig(path.Join(uuid.New().String(), "registry"), nil)
9197
cfg.Transport.ServerList = []string{GetEtcdURL()}

test/integration/framework/etcd.go

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"net"
2525
"os"
2626
"os/exec"
27+
"path"
2728
"strconv"
2829
"strings"
2930
"sync"
@@ -51,18 +52,6 @@ func getEtcdPath() (string, error) {
5152
return exec.LookPath("etcd")
5253
}
5354

54-
// getAvailablePort returns a TCP port that is available for binding.
55-
func getAvailablePort() (int, error) {
56-
l, err := net.Listen("tcp", ":0")
57-
if err != nil {
58-
return 0, fmt.Errorf("could not bind to a port: %v", err)
59-
}
60-
// It is possible but unlikely that someone else will bind this port before we
61-
// get a chance to use it.
62-
defer l.Close()
63-
return l.Addr().(*net.TCPAddr).Port, nil
64-
}
65-
6655
// startEtcd executes an etcd instance. The returned function will signal the
6756
// etcd process and wait for it to exit.
6857
func startEtcd(output io.Writer, forceCreate bool) (func(), error) {
@@ -105,28 +94,30 @@ func RunCustomEtcd(dataDir string, customFlags []string, output io.Writer) (url
10594
fmt.Fprint(os.Stderr, installEtcd)
10695
return "", nil, fmt.Errorf("could not find etcd in PATH: %v", err)
10796
}
108-
etcdPort, err := getAvailablePort()
109-
if err != nil {
110-
return "", nil, fmt.Errorf("could not get a port: %v", err)
111-
}
112-
customURL := fmt.Sprintf("http://127.0.0.1:%d", etcdPort)
113-
114-
klog.Infof("starting etcd on %s", customURL)
115-
11697
etcdDataDir, err := os.MkdirTemp(os.TempDir(), dataDir)
11798
if err != nil {
11899
return "", nil, fmt.Errorf("unable to make temp etcd data dir %s: %v", dataDir, err)
119100
}
120-
klog.Infof("storing etcd data in: %v", etcdDataDir)
101+
etcdSocketPath := path.Join(etcdDataDir, "etcd.sock")
102+
customURL := "unix://" + etcdSocketPath
121103

104+
klog.V(2).InfoS("starting etcd", "url", customURL, "dataDir", etcdDataDir)
122105
ctx, cancel := context.WithCancel(context.Background())
123106
args := []string{
124107
"--data-dir",
125108
etcdDataDir,
126109
"--listen-client-urls",
127110
customURL,
111+
// This should be how clients connect to etcd, but https://github.com/etcd-io/etcd/pull/12469
112+
// apparently was incomplete: trying to pass a Unix Domain URL here is rejected by ectd 3.15.13 with
113+
// --advertise-client-urls "unix:///tmp/etcd.sock" must be "host:port" (missing port in address)
114+
//
115+
// We don't need to advertise the correct address. To prevent connecting to the default URL
116+
// in the unlikely case that something does use this URL after all, an invalid URL is set here.
128117
"--advertise-client-urls",
129-
customURL,
118+
"http://127.0.0.111:0",
119+
// With :0 we let the kernel pick a unique port. We don't care which port this will be,
120+
// no other peer is going to connect.
130121
"--listen-peer-urls",
131122
"http://127.0.0.1:0",
132123
"-log-level",
@@ -176,7 +167,7 @@ func RunCustomEtcd(dataDir string, customFlags []string, output io.Writer) (url
176167
const pollCount = int32(300)
177168

178169
for i <= pollCount {
179-
conn, err := net.DialTimeout("tcp", strings.TrimPrefix(customURL, "http://"), 1*time.Second)
170+
conn, err := net.DialTimeout("unix", etcdSocketPath, 1*time.Second)
180171
if err == nil {
181172
conn.Close()
182173
break

test/integration/utils.go

Lines changed: 5 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,15 @@ import (
2121
"testing"
2222
"time"
2323

24-
"google.golang.org/grpc"
24+
clientv3 "go.etcd.io/etcd/client/v3"
25+
2526
apierrors "k8s.io/apimachinery/pkg/api/errors"
2627
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2728
"k8s.io/apimachinery/pkg/util/wait"
2829
"k8s.io/apiserver/pkg/storage/storagebackend"
2930
clientset "k8s.io/client-go/kubernetes"
3031
coreclient "k8s.io/client-go/kubernetes/typed/core/v1"
31-
32-
"go.etcd.io/etcd/client/pkg/v3/transport"
33-
clientv3 "go.etcd.io/etcd/client/v3"
32+
kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing"
3433
)
3534

3635
// DeletePodOrErrorf deletes a pod or fails with a call to t.Errorf.
@@ -67,32 +66,7 @@ func WaitForPodToDisappear(podClient coreclient.PodInterface, podName string, in
6766
})
6867
}
6968

70-
// GetEtcdClients returns an initialized clientv3.Client and clientv3.KV.
69+
// GetEtcdClients returns an initialized etcd clientv3.Client and clientv3.KV.
7170
func GetEtcdClients(config storagebackend.TransportConfig) (*clientv3.Client, clientv3.KV, error) {
72-
tlsInfo := transport.TLSInfo{
73-
CertFile: config.CertFile,
74-
KeyFile: config.KeyFile,
75-
TrustedCAFile: config.TrustedCAFile,
76-
}
77-
78-
tlsConfig, err := tlsInfo.ClientConfig()
79-
if err != nil {
80-
return nil, nil, err
81-
}
82-
83-
cfg := clientv3.Config{
84-
Endpoints: config.ServerList,
85-
DialTimeout: 20 * time.Second,
86-
DialOptions: []grpc.DialOption{
87-
grpc.WithBlock(), // block until the underlying connection is up
88-
},
89-
TLS: tlsConfig,
90-
}
91-
92-
c, err := clientv3.New(cfg)
93-
if err != nil {
94-
return nil, nil, err
95-
}
96-
97-
return c, clientv3.NewKV(c), nil
71+
return kubeapiservertesting.GetEtcdClients(config)
9872
}

0 commit comments

Comments
 (0)