Skip to content

Commit 8252610

Browse files
authored
Merge pull request kubernetes#72514 from fabriziopandini/cleanup-etcd-client
kubeadm: cleanup etcd client
2 parents 716b253 + 684b80f commit 8252610

File tree

3 files changed

+2
-325
lines changed

3 files changed

+2
-325
lines changed

cmd/kubeadm/app/util/etcd/BUILD

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ go_library(
99
"//cmd/kubeadm/app/apis/kubeadm:go_default_library",
1010
"//cmd/kubeadm/app/constants:go_default_library",
1111
"//cmd/kubeadm/app/util/config:go_default_library",
12-
"//cmd/kubeadm/app/util/staticpod:go_default_library",
1312
"//staging/src/k8s.io/client-go/kubernetes:go_default_library",
1413
"//vendor/github.com/coreos/etcd/clientv3:go_default_library",
1514
"//vendor/github.com/coreos/etcd/pkg/transport:go_default_library",
@@ -25,7 +24,6 @@ go_test(
2524
deps = [
2625
"//cmd/kubeadm/app/apis/kubeadm:go_default_library",
2726
"//cmd/kubeadm/app/constants:go_default_library",
28-
"//cmd/kubeadm/test:go_default_library",
2927
],
3028
)
3129

cmd/kubeadm/app/util/etcd/etcd.go

Lines changed: 2 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ import (
3434
kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
3535
"k8s.io/kubernetes/cmd/kubeadm/app/constants"
3636
"k8s.io/kubernetes/cmd/kubeadm/app/util/config"
37-
"k8s.io/kubernetes/cmd/kubeadm/app/util/staticpod"
3837
)
3938

4039
// ClusterInterrogator is an interface to get etcd cluster related information
@@ -54,41 +53,6 @@ type Client struct {
5453
TLS *tls.Config
5554
}
5655

57-
// PodManifestsHaveTLS reads the etcd staticpod manifest from disk and returns false if the TLS flags
58-
// are missing from the command list. If all the flags are present it returns true.
59-
func PodManifestsHaveTLS(ManifestDir string) (bool, error) {
60-
etcdPodPath := constants.GetStaticPodFilepath(constants.Etcd, ManifestDir)
61-
etcdPod, err := staticpod.ReadStaticPodFromDisk(etcdPodPath)
62-
if err != nil {
63-
return false, errors.Wrap(err, "failed to check if etcd pod implements TLS")
64-
}
65-
66-
tlsFlags := []string{
67-
"--cert-file=",
68-
"--key-file=",
69-
"--trusted-ca-file=",
70-
"--client-cert-auth=",
71-
"--peer-cert-file=",
72-
"--peer-key-file=",
73-
"--peer-trusted-ca-file=",
74-
"--peer-client-cert-auth=",
75-
}
76-
FlagLoop:
77-
for _, flag := range tlsFlags {
78-
for _, container := range etcdPod.Spec.Containers {
79-
for _, arg := range container.Command {
80-
if strings.Contains(arg, flag) {
81-
continue FlagLoop
82-
}
83-
}
84-
}
85-
// flag not found in any container
86-
return false, nil
87-
}
88-
// all flags were found in container args; pod fully implements TLS
89-
return true, nil
90-
}
91-
9256
// New creates a new EtcdCluster client
9357
func New(endpoints []string, ca, cert, key string) (*Client, error) {
9458
client := Client{Endpoints: endpoints}
@@ -113,53 +77,8 @@ func New(endpoints []string, ca, cert, key string) (*Client, error) {
11377
// the kubeadm-config ConfigMap in kube-system namespace.
11478
// Once created, the client synchronizes client's endpoints with the known endpoints from the etcd membership API (reality check).
11579
func NewFromCluster(client clientset.Interface, certificatesDir string) (*Client, error) {
116-
117-
// Kubeadm v1.13 should manage v1.12 clusters and v1.13 clusters
118-
// v1.12 clusters can be have etcd listening on localhost only (if the cluster was created with kubeadm v1.12)
119-
// or etcd listening on localhost and API server advertise address (if the cluster was created with kubeadm v1.13).
120-
// The first case should be dropped in v1.14 when support for v1.12 clusters can be removed from the codebase.
121-
122-
// Detect which type of etcd we are dealing with
123-
// Please note that this test can be executed only on master nodes during upgrades;
124-
// For nodes where we are joining a new control plane node instead we should tolerate that the etcd manifest does not
125-
// exists and try to connect to etcd using API server advertise address; as described above this will lead to a know isse
126-
// for cluster created with v1.12, but a documented workaround will be provided
127-
oldManifest := false
128-
klog.V(1).Infoln("checking etcd manifest")
129-
130-
etcdManifestFile := constants.GetStaticPodFilepath(constants.Etcd, constants.GetStaticPodDirectory())
131-
etcdPod, err := staticpod.ReadStaticPodFromDisk(etcdManifestFile)
132-
if err == nil {
133-
etcdContainer := etcdPod.Spec.Containers[0]
134-
for _, arg := range etcdContainer.Command {
135-
if arg == "--listen-client-urls=https://127.0.0.1:2379" {
136-
klog.V(1).Infoln("etcd manifest created by kubeadm v1.12")
137-
oldManifest = true
138-
}
139-
}
140-
141-
// if etcd is listening on localhost only
142-
if oldManifest == true {
143-
// etcd cluster has a single member "by design"
144-
endpoints := []string{fmt.Sprintf("localhost:%d", constants.EtcdListenClientPort)}
145-
146-
etcdClient, err := New(
147-
endpoints,
148-
filepath.Join(certificatesDir, constants.EtcdCACertName),
149-
filepath.Join(certificatesDir, constants.EtcdHealthcheckClientCertName),
150-
filepath.Join(certificatesDir, constants.EtcdHealthcheckClientKeyName),
151-
)
152-
if err != nil {
153-
return nil, errors.Wrapf(err, "error creating etcd client for %v endpoint", endpoints)
154-
}
155-
156-
return etcdClient, nil
157-
}
158-
}
159-
160-
// etcd is listening on localhost and API server advertise address, and
161-
// the etcd cluster can have more than one etcd members, so it is necessary to get the
162-
// list of endpoints from kubeadm cluster status before connecting
80+
// etcd is listening the API server advertise address on each control-plane node
81+
// so it is necessary to get the list of endpoints from kubeadm cluster status before connecting
16382

16483
// Gets the cluster status
16584
clusterStatus, err := config.GetClusterStatus(client)

cmd/kubeadm/app/util/etcd/etcd_test.go

Lines changed: 0 additions & 240 deletions
Original file line numberDiff line numberDiff line change
@@ -18,253 +18,13 @@ package etcd
1818

1919
import (
2020
"fmt"
21-
"io/ioutil"
22-
"os"
23-
"path/filepath"
2421
"strconv"
2522
"testing"
2623

2724
kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
2825
"k8s.io/kubernetes/cmd/kubeadm/app/constants"
29-
testutil "k8s.io/kubernetes/cmd/kubeadm/test"
3026
)
3127

32-
const (
33-
secureEtcdPod = `# generated by kubeadm v1.10.0
34-
apiVersion: v1
35-
kind: Pod
36-
metadata:
37-
annotations:
38-
scheduler.alpha.kubernetes.io/critical-pod: ""
39-
creationTimestamp: null
40-
labels:
41-
component: etcd
42-
tier: control-plane
43-
name: etcd
44-
namespace: kube-system
45-
spec:
46-
containers:
47-
- command:
48-
- etcd
49-
- --advertise-client-urls=https://127.0.0.1:2379
50-
- --data-dir=/var/lib/etcd
51-
- --peer-key-file=/etc/kubernetes/pki/etcd/peer.key
52-
- --peer-trusted-ca-file=/etc/kubernetes/pki/etcd/ca.crt
53-
- --listen-client-urls=https://127.0.0.1:2379
54-
- --peer-client-cert-auth=true
55-
- --cert-file=/etc/kubernetes/pki/etcd/server.crt
56-
- --key-file=/etc/kubernetes/pki/etcd/server.key
57-
- --trusted-ca-file=/etc/kubernetes/pki/etcd/ca.crt
58-
- --peer-cert-file=/etc/kubernetes/pki/etcd/peer.crt
59-
- --client-cert-auth=true
60-
image: k8s.gcr.io/etcd-amd64:3.1.12
61-
livenessProbe:
62-
exec:
63-
command:
64-
- /bin/sh
65-
- -ec
66-
- ETCDCTL_API=3 etcdctl --endpoints=127.0.0.1:2379 --cacert=/etc/kubernetes/pki/etcd/ca.crt
67-
--cert=/etc/kubernetes/pki/etcd/healthcheck-client.crt --key=/etc/kubernetes/pki/etcd/healthcheck-client.key
68-
get foo
69-
failureThreshold: 8
70-
initialDelaySeconds: 15
71-
timeoutSeconds: 15
72-
name: etcd
73-
resources: {}
74-
volumeMounts:
75-
- mountPath: /var/lib/etcd
76-
name: etcd-data
77-
- mountPath: /etc/kubernetes/pki/etcd
78-
name: etcd-certs
79-
hostNetwork: true
80-
volumes:
81-
- hostPath:
82-
path: /var/lib/etcd
83-
type: DirectoryOrCreate
84-
name: etcd-data
85-
- hostPath:
86-
path: /etc/kubernetes/pki/etcd
87-
type: DirectoryOrCreate
88-
name: etcd-certs
89-
status: {}
90-
`
91-
secureExposedEtcdPod = `
92-
apiVersion: v1
93-
kind: Pod
94-
metadata:
95-
annotations:
96-
scheduler.alpha.kubernetes.io/critical-pod: ""
97-
creationTimestamp: null
98-
labels:
99-
component: etcd
100-
tier: control-plane
101-
name: etcd
102-
namespace: kube-system
103-
spec:
104-
containers:
105-
- command:
106-
- etcd
107-
- --advertise-client-urls=https://10.0.5.5:2379
108-
- --data-dir=/var/lib/etcd
109-
- --peer-key-file=/etc/kubernetes/pki/etcd/peer.key
110-
- --peer-trusted-ca-file=/etc/kubernetes/pki/etcd/ca.crt
111-
- --listen-client-urls=https://[::0:0]:2379
112-
- --peer-client-cert-auth=true
113-
- --cert-file=/etc/kubernetes/pki/etcd/server.crt
114-
- --key-file=/etc/kubernetes/pki/etcd/server.key
115-
- --trusted-ca-file=/etc/kubernetes/pki/etcd/ca.crt
116-
- --peer-cert-file=/etc/kubernetes/pki/etcd/peer.crt
117-
- --client-cert-auth=true
118-
image: k8s.gcr.io/etcd-amd64:3.1.12
119-
livenessProbe:
120-
exec:
121-
command:
122-
- /bin/sh
123-
- -ec
124-
- ETCDCTL_API=3 etcdctl --endpoints=https://[::1]:2379 --cacert=/etc/kubernetes/pki/etcd/ca.crt
125-
--cert=/etc/kubernetes/pki/etcd/healthcheck-client.crt --key=/etc/kubernetes/pki/etcd/healthcheck-client.key
126-
get foo
127-
failureThreshold: 8
128-
initialDelaySeconds: 15
129-
timeoutSeconds: 15
130-
name: etcd
131-
resources: {}
132-
volumeMounts:
133-
- mountPath: /var/lib/etcd
134-
name: etcd-data
135-
- mountPath: /etc/kubernetes/pki/etcd
136-
name: etcd-certs
137-
hostNetwork: true
138-
volumes:
139-
- hostPath:
140-
path: /var/lib/etcd
141-
type: DirectoryOrCreate
142-
name: etcd-data
143-
- hostPath:
144-
path: /etc/kubernetes/pki/etcd
145-
type: DirectoryOrCreate
146-
name: etcd-certs
147-
status: {}
148-
`
149-
insecureEtcdPod = `# generated by kubeadm v1.9.6
150-
apiVersion: v1
151-
kind: Pod
152-
metadata:
153-
annotations:
154-
scheduler.alpha.kubernetes.io/critical-pod: ""
155-
creationTimestamp: null
156-
labels:
157-
component: etcd
158-
tier: control-plane
159-
name: etcd
160-
namespace: kube-system
161-
spec:
162-
containers:
163-
- command:
164-
- etcd
165-
- --listen-client-urls=http://127.0.0.1:2379
166-
- --advertise-client-urls=http://127.0.0.1:2379
167-
- --data-dir=/var/lib/etcd
168-
image: gcr.io/google_containers/etcd-amd64:3.1.11
169-
livenessProbe:
170-
failureThreshold: 8
171-
httpGet:
172-
host: 127.0.0.1
173-
path: /health
174-
port: 2379
175-
scheme: HTTP
176-
initialDelaySeconds: 15
177-
timeoutSeconds: 15
178-
name: etcd
179-
resources: {}
180-
volumeMounts:
181-
- mountPath: /var/lib/etcd
182-
name: etcd
183-
hostNetwork: true
184-
volumes:
185-
- hostPath:
186-
path: /var/lib/etcd
187-
type: DirectoryOrCreate
188-
name: etcd
189-
status: {}
190-
`
191-
invalidPod = `---{ broken yaml @@@`
192-
)
193-
194-
func TestPodManifestHasTLS(t *testing.T) {
195-
tests := []struct {
196-
description string
197-
podYaml string
198-
hasTLS bool
199-
expectErr bool
200-
writeManifest bool
201-
}{
202-
{
203-
description: "secure etcd returns true",
204-
podYaml: secureEtcdPod,
205-
hasTLS: true,
206-
writeManifest: true,
207-
expectErr: false,
208-
},
209-
{
210-
description: "secure exposed etcd returns true",
211-
podYaml: secureExposedEtcdPod,
212-
hasTLS: true,
213-
writeManifest: true,
214-
expectErr: false,
215-
},
216-
{
217-
description: "insecure etcd returns false",
218-
podYaml: insecureEtcdPod,
219-
hasTLS: false,
220-
writeManifest: true,
221-
expectErr: false,
222-
},
223-
{
224-
description: "invalid pod fails to unmarshal",
225-
podYaml: invalidPod,
226-
hasTLS: false,
227-
writeManifest: true,
228-
expectErr: true,
229-
},
230-
{
231-
description: "non-existent file returns error",
232-
podYaml: ``,
233-
hasTLS: false,
234-
writeManifest: false,
235-
expectErr: true,
236-
},
237-
}
238-
239-
for _, rt := range tests {
240-
tmpdir := testutil.SetupTempDir(t)
241-
defer os.RemoveAll(tmpdir)
242-
243-
manifestPath := filepath.Join(tmpdir, "etcd.yaml")
244-
if rt.writeManifest {
245-
err := ioutil.WriteFile(manifestPath, []byte(rt.podYaml), 0644)
246-
if err != nil {
247-
t.Fatalf("Failed to write pod manifest\n%s\n\tfatal error: %v", rt.description, err)
248-
}
249-
}
250-
251-
hasTLS, actualErr := PodManifestsHaveTLS(tmpdir)
252-
if (actualErr != nil) != rt.expectErr {
253-
t.Errorf(
254-
"PodManifestHasTLS failed\n%s\n\texpected error: %t\n\tgot: %t\n\tactual error: %v",
255-
rt.description,
256-
rt.expectErr,
257-
(actualErr != nil),
258-
actualErr,
259-
)
260-
}
261-
262-
if hasTLS != rt.hasTLS {
263-
t.Errorf("PodManifestHasTLS failed\n%s\n\texpected hasTLS: %t\n\tgot: %t", rt.description, rt.hasTLS, hasTLS)
264-
}
265-
}
266-
}
267-
26828
func TestCheckConfigurationIsHA(t *testing.T) {
26929
var tests = []struct {
27030
name string

0 commit comments

Comments
 (0)