From aae1f2a85f40fcf0168582b27fb844999b4629ed Mon Sep 17 00:00:00 2001 From: Dave Chen Date: Thu, 3 Nov 2022 10:30:31 +0800 Subject: [PATCH 1/4] kubeadm: `cri-socket` is not allowed for mixed configuration Set the `cri-socket` both in flags and config file will hit errors, this should not be a valid case to validate in current testcases. Signed-off-by: Dave Chen --- cmd/kubeadm/app/cmd/init_test.go | 2 +- cmd/kubeadm/app/cmd/join_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/kubeadm/app/cmd/init_test.go b/cmd/kubeadm/app/cmd/init_test.go index b3f12e0e014..8fde9183a62 100644 --- a/cmd/kubeadm/app/cmd/init_test.go +++ b/cmd/kubeadm/app/cmd/init_test.go @@ -100,7 +100,7 @@ func TestNewInitData(t *testing.T) { }, }, { - name: "--cri-socket and --node-name flags override config from file", + name: "--node-name flags override config from file", flags: map[string]string{ options.CfgPath: configFilePath, options.NodeName: "anotherName", diff --git a/cmd/kubeadm/app/cmd/join_test.go b/cmd/kubeadm/app/cmd/join_test.go index 743e4d3aee4..d3b21d234f7 100644 --- a/cmd/kubeadm/app/cmd/join_test.go +++ b/cmd/kubeadm/app/cmd/join_test.go @@ -208,7 +208,7 @@ func TestNewJoinData(t *testing.T) { }, }, { - name: "--cri-socket and --node-name flags override config from file", + name: "--node-name flags override config from file", flags: map[string]string{ options.CfgPath: configFilePath, options.NodeName: "anotherName", From a7b610c3bb29c7d224ac23240a4946fd443e2645 Mon Sep 17 00:00:00 2001 From: Dave Chen Date: Thu, 3 Nov 2022 10:47:10 +0800 Subject: [PATCH 2/4] kubeadm: use the right methods for logging if no args are passing Signed-off-by: Dave Chen --- cmd/kubeadm/app/cmd/init_test.go | 4 ++-- cmd/kubeadm/app/cmd/join_test.go | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cmd/kubeadm/app/cmd/init_test.go b/cmd/kubeadm/app/cmd/init_test.go index 8fde9183a62..075c8c7304f 100644 --- a/cmd/kubeadm/app/cmd/init_test.go +++ b/cmd/kubeadm/app/cmd/init_test.go @@ -108,7 +108,7 @@ func TestNewInitData(t *testing.T) { validate: func(t *testing.T, data *initData) { // validate that node-name is overwritten if data.cfg.NodeRegistration.Name != "anotherName" { - t.Errorf("Invalid NodeRegistration.Name") + t.Error("Invalid NodeRegistration.Name") } }, }, @@ -162,7 +162,7 @@ func TestNewInitData(t *testing.T) { t.Fatalf("newInitData returned unexpected error: %v", err) } if err == nil && tc.expectError { - t.Fatalf("newInitData didn't return error when expected") + t.Fatal("newInitData didn't return error when expected") } // exec additional validation on the returned value diff --git a/cmd/kubeadm/app/cmd/join_test.go b/cmd/kubeadm/app/cmd/join_test.go index d3b21d234f7..37ecf27d30b 100644 --- a/cmd/kubeadm/app/cmd/join_test.go +++ b/cmd/kubeadm/app/cmd/join_test.go @@ -104,7 +104,7 @@ func TestNewJoinData(t *testing.T) { validate: func(t *testing.T, data *joinData) { // validate that file discovery settings are set into join data if data.cfg.Discovery.File == nil || data.cfg.Discovery.File.KubeConfigPath != "https://foo" { - t.Errorf("Invalid data.cfg.Discovery.File") + t.Error("Invalid data.cfg.Discovery.File") } }, }, @@ -121,7 +121,7 @@ func TestNewJoinData(t *testing.T) { data.cfg.Discovery.BootstrapToken.APIServerEndpoint != "1.2.3.4:6443" || //only first arg should be kept as APIServerEndpoint data.cfg.Discovery.BootstrapToken.Token != "abcdef.0123456789abcdef" || data.cfg.Discovery.BootstrapToken.UnsafeSkipCAVerification != true { - t.Errorf("Invalid data.cfg.Discovery.BootstrapToken") + t.Error("Invalid data.cfg.Discovery.BootstrapToken") } }, }, @@ -137,7 +137,7 @@ func TestNewJoinData(t *testing.T) { if data.cfg.Discovery.TLSBootstrapToken != "abcdef.0123456789abcdef" || data.cfg.Discovery.BootstrapToken == nil || data.cfg.Discovery.BootstrapToken.Token != "abcdef.0123456789abcdef" { - t.Errorf("Invalid TLSBootstrapToken or BootstrapToken.Token") + t.Error("Invalid TLSBootstrapToken or BootstrapToken.Token") } }, }, @@ -155,7 +155,7 @@ func TestNewJoinData(t *testing.T) { if data.cfg.Discovery.TLSBootstrapToken != "abcdef.0123456789abcdef" || data.cfg.Discovery.BootstrapToken == nil || data.cfg.Discovery.BootstrapToken.Token != "defghi.0123456789defghi" { - t.Errorf("Invalid TLSBootstrapToken or BootstrapToken.Token") + t.Error("Invalid TLSBootstrapToken or BootstrapToken.Token") } }, }, @@ -172,7 +172,7 @@ func TestNewJoinData(t *testing.T) { if data.cfg.ControlPlane == nil || data.cfg.ControlPlane.LocalAPIEndpoint.AdvertiseAddress != "1.2.3.4" || data.cfg.ControlPlane.LocalAPIEndpoint.BindPort != 1234 { - t.Errorf("Invalid ControlPlane") + t.Error("Invalid ControlPlane") } }, }, @@ -187,7 +187,7 @@ func TestNewJoinData(t *testing.T) { validate: func(t *testing.T, data *joinData) { // validate that control plane attributes are unset in join data if data.cfg.ControlPlane != nil { - t.Errorf("Invalid ControlPlane") + t.Error("Invalid ControlPlane") } }, expectWarn: true, @@ -216,7 +216,7 @@ func TestNewJoinData(t *testing.T) { validate: func(t *testing.T, data *joinData) { // validate that node-name is overwritten if data.cfg.NodeRegistration.Name != "anotherName" { - t.Errorf("Invalid NodeRegistration.Name") + t.Error("Invalid NodeRegistration.Name") } }, }, @@ -304,7 +304,7 @@ func TestNewJoinData(t *testing.T) { t.Fatalf("newJoinData returned unexpected error: %v", err) } if err == nil && tc.expectError { - t.Fatalf("newJoinData didn't return error when expected") + t.Fatal("newJoinData didn't return error when expected") } // exec additional validation on the returned value From 765ef1783b05c52feab5e60bee28f3da0cebdf02 Mon Sep 17 00:00:00 2001 From: Dave Chen Date: Thu, 3 Nov 2022 13:36:10 +0800 Subject: [PATCH 3/4] kubeadm: stop using of CRI endpoints without URL scheme run the testcase with `-v` flag will reveal the warning, e.g. `W1103 ... Usage of CRI endpoints without URL scheme is deprecated...` Signed-off-by: Dave Chen --- cmd/kubeadm/app/cmd/init_test.go | 2 +- cmd/kubeadm/app/cmd/join_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/kubeadm/app/cmd/init_test.go b/cmd/kubeadm/app/cmd/init_test.go index 075c8c7304f..9339277199f 100644 --- a/cmd/kubeadm/app/cmd/init_test.go +++ b/cmd/kubeadm/app/cmd/init_test.go @@ -36,7 +36,7 @@ localAPIEndpoint: bootstrapTokens: - token: "abcdef.0123456789abcdef" nodeRegistration: - criSocket: /run/containerd/containerd.sock + criSocket: unix:///var/run/containerd/containerd.sock name: someName ignorePreflightErrors: - c diff --git a/cmd/kubeadm/app/cmd/join_test.go b/cmd/kubeadm/app/cmd/join_test.go index 37ecf27d30b..268650215d1 100644 --- a/cmd/kubeadm/app/cmd/join_test.go +++ b/cmd/kubeadm/app/cmd/join_test.go @@ -42,7 +42,7 @@ discovery: controlPlane: certificateKey: c39a18bae4a72e71b178661f437363da218a3efb83ddb03f1cd91d9ae1da41bd nodeRegistration: - criSocket: /run/containerd/containerd.sock + criSocket: unix:///var/run/containerd/containerd.sock name: someName ignorePreflightErrors: - c From a825450707b812da619283c59b7c927f30eb2b59 Mon Sep 17 00:00:00 2001 From: Dave Chen Date: Thu, 3 Nov 2022 15:09:07 +0800 Subject: [PATCH 4/4] kubeadm: improve test coverage by validating the data structure data structure is what returned if everything okay, but this structure is not validated at all both in `init` and `join` cmd. Signed-off-by: Dave Chen --- cmd/kubeadm/app/cmd/init_test.go | 40 +++++++++++++++++++++++++++++++- cmd/kubeadm/app/cmd/join_test.go | 37 +++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/cmd/kubeadm/app/cmd/init_test.go b/cmd/kubeadm/app/cmd/init_test.go index 9339277199f..10112726489 100644 --- a/cmd/kubeadm/app/cmd/init_test.go +++ b/cmd/kubeadm/app/cmd/init_test.go @@ -22,10 +22,16 @@ import ( "path/filepath" "testing" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" + v1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/bootstraptoken/v1" + kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta3" "k8s.io/kubernetes/cmd/kubeadm/app/cmd/options" + "k8s.io/kubernetes/cmd/kubeadm/app/constants" ) var testInitConfig = fmt.Sprintf(`--- @@ -98,6 +104,39 @@ func TestNewInitData(t *testing.T) { flags: map[string]string{ options.CfgPath: configFilePath, }, + validate: func(t *testing.T, data *initData) { + validData := &initData{ + certificatesDir: kubeadmapiv1.DefaultCertificatesDir, + kubeconfigPath: constants.GetAdminKubeConfigPath(), + kubeconfigDir: constants.KubernetesDir, + ignorePreflightErrors: sets.New("c", "d"), + cfg: &kubeadmapi.InitConfiguration{ + NodeRegistration: kubeadmapi.NodeRegistrationOptions{ + Name: "somename", + CRISocket: "unix:///var/run/containerd/containerd.sock", + IgnorePreflightErrors: []string{"c", "d"}, + ImagePullPolicy: "IfNotPresent", + }, + LocalAPIEndpoint: kubeadmapi.APIEndpoint{ + AdvertiseAddress: "1.2.3.4", + BindPort: 6443, + }, + BootstrapTokens: []v1.BootstrapToken{ + { + Token: &v1.BootstrapTokenString{ID: "abcdef", Secret: "0123456789abcdef"}, + Usages: []string{"signing", "authentication"}, + TTL: &metav1.Duration{ + Duration: constants.DefaultTokenDuration, + }, + Groups: []string{"system:bootstrappers:kubeadm:default-node-token"}, + }, + }, + }, + } + if diff := cmp.Diff(validData, data, cmp.AllowUnexported(initData{}), cmpopts.IgnoreFields(initData{}, "client", "cfg.ClusterConfiguration", "cfg.NodeRegistration.Taints")); diff != "" { + t.Fatalf("newInitData returned data (-want,+got):\n%s", diff) + } + }, }, { name: "--node-name flags override config from file", @@ -164,7 +203,6 @@ func TestNewInitData(t *testing.T) { if err == nil && tc.expectError { t.Fatal("newInitData didn't return error when expected") } - // exec additional validation on the returned value if tc.validate != nil { tc.validate(t, data) diff --git a/cmd/kubeadm/app/cmd/join_test.go b/cmd/kubeadm/app/cmd/join_test.go index 268650215d1..4e323bb74e1 100644 --- a/cmd/kubeadm/app/cmd/join_test.go +++ b/cmd/kubeadm/app/cmd/join_test.go @@ -24,9 +24,15 @@ import ( "strings" "testing" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/klog/v2" + kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta3" "k8s.io/kubernetes/cmd/kubeadm/app/cmd/options" kubeconfigutil "k8s.io/kubernetes/cmd/kubeadm/app/util/kubeconfig" @@ -206,6 +212,37 @@ func TestNewJoinData(t *testing.T) { flags: map[string]string{ options.CfgPath: configFilePath, }, + validate: func(t *testing.T, data *joinData) { + validData := &joinData{ + cfg: &kubeadmapi.JoinConfiguration{ + TypeMeta: metav1.TypeMeta{Kind: "", APIVersion: ""}, + NodeRegistration: kubeadmapi.NodeRegistrationOptions{ + Name: "somename", + CRISocket: "unix:///var/run/containerd/containerd.sock", + IgnorePreflightErrors: []string{"c", "d"}, + ImagePullPolicy: "IfNotPresent", + Taints: []v1.Taint{{Key: "node-role.kubernetes.io/control-plane", Effect: "NoSchedule"}}, + }, + CACertPath: kubeadmapiv1.DefaultCACertPath, + Discovery: kubeadmapi.Discovery{ + BootstrapToken: &kubeadmapi.BootstrapTokenDiscovery{ + Token: "abcdef.0123456789abcdef", + APIServerEndpoint: "1.2.3.4:6443", + UnsafeSkipCAVerification: true, + }, + TLSBootstrapToken: "abcdef.0123456789abcdef", + Timeout: &metav1.Duration{Duration: kubeadmapiv1.DefaultDiscoveryTimeout}, + }, + ControlPlane: &kubeadmapi.JoinControlPlane{ + CertificateKey: "c39a18bae4a72e71b178661f437363da218a3efb83ddb03f1cd91d9ae1da41bd", + }, + }, + ignorePreflightErrors: sets.New("c", "d"), + } + if diff := cmp.Diff(validData, data, cmp.AllowUnexported(joinData{}), cmpopts.IgnoreFields(joinData{}, "client", "initCfg", "cfg.ControlPlane.LocalAPIEndpoint")); diff != "" { + t.Fatalf("newJoinData returned data (-want,+got):\n%s", diff) + } + }, }, { name: "--node-name flags override config from file",