From 9a843790a30683ff9971b96e4e21f9ac494f15ad Mon Sep 17 00:00:00 2001 From: HirazawaUi <695097494plus@gmail.com> Date: Thu, 27 Feb 2025 21:35:22 +0800 Subject: [PATCH] Adding tests for consistency --- cmd/kubeadm/app/util/config/common_test.go | 10 + .../app/util/config/initconfiguration_test.go | 291 ++++++++++++------ .../app/util/config/joinconfiguration_test.go | 205 ++++++++---- .../util/config/resetconfiguration_test.go | 148 ++++++--- .../app/util/config/upgradeconfiguration.go | 5 - .../util/config/upgradeconfiguration_test.go | 124 +++----- 6 files changed, 503 insertions(+), 280 deletions(-) diff --git a/cmd/kubeadm/app/util/config/common_test.go b/cmd/kubeadm/app/util/config/common_test.go index 925a4ce90ea..8f8692cd7dd 100644 --- a/cmd/kubeadm/app/util/config/common_test.go +++ b/cmd/kubeadm/app/util/config/common_test.go @@ -29,8 +29,10 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/json" "k8s.io/apimachinery/pkg/util/version" apimachineryversion "k8s.io/apimachinery/pkg/version" + "sigs.k8s.io/yaml" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmapiv1old "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta3" @@ -41,6 +43,14 @@ import ( const KubeadmGroupName = "kubeadm.k8s.io" +var formats = []struct { + name string + marshal func(interface{}) ([]byte, error) +}{ + {name: "JSON", marshal: json.Marshal}, + {name: "YAML", marshal: yaml.Marshal}, +} + func TestValidateSupportedVersion(t *testing.T) { tests := []struct { gvk schema.GroupVersionKind diff --git a/cmd/kubeadm/app/util/config/initconfiguration_test.go b/cmd/kubeadm/app/util/config/initconfiguration_test.go index a7ebc0d7452..8b95c4260df 100644 --- a/cmd/kubeadm/app/util/config/initconfiguration_test.go +++ b/cmd/kubeadm/app/util/config/initconfiguration_test.go @@ -17,110 +17,76 @@ limitations under the License. package config import ( - "bytes" "fmt" "os" "path/filepath" "testing" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/lithammer/dedent" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/yaml" + "k8s.io/utils/ptr" - "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" + kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta4" "k8s.io/kubernetes/cmd/kubeadm/app/constants" ) func TestLoadInitConfigurationFromFile(t *testing.T) { - certDir := "/tmp/foo" - clusterCfg := []byte(fmt.Sprintf(` -apiVersion: %s -kind: ClusterConfiguration -certificatesDir: %s -kubernetesVersion: %s`, kubeadmapiv1.SchemeGroupVersion.String(), certDir, constants.MinimumControlPlaneVersion.String())) - - // Create temp folder for the test case tmpdir, err := os.MkdirTemp("", "") if err != nil { t.Fatalf("Couldn't create tmpdir: %v", err) } - defer os.RemoveAll(tmpdir) + defer func() { + if err := os.RemoveAll(tmpdir); err != nil { + t.Fatalf("Couldn't remove tmpdir: %v", err) + } + }() + filename := "kubeadmConfig" + filePath := filepath.Join(tmpdir, filename) + options := LoadOrDefaultConfigurationOptions{} - // cfgFiles is in cluster_test.go - var tests = []struct { + tests := []struct { name string - fileContents []byte - expectErr bool - validate func(*testing.T, *kubeadm.InitConfiguration) + cfgPath string + fileContents string + wantErr bool }{ { - name: "v1beta3.partial1", - fileContents: cfgFiles["InitConfiguration_v1beta3"], + name: "Config file does not exists", + cfgPath: "tmp", + wantErr: true, }, { - name: "v1beta3.partial2", - fileContents: bytes.Join([][]byte{ - cfgFiles["InitConfiguration_v1beta3"], - clusterCfg, - }, []byte(constants.YAMLDocumentSeparator)), - validate: func(t *testing.T, cfg *kubeadm.InitConfiguration) { - if cfg.ClusterConfiguration.CertificatesDir != certDir { - t.Errorf("CertificatesDir from ClusterConfiguration holds the wrong value, Expected: %v. Actual: %v", certDir, cfg.ClusterConfiguration.CertificatesDir) - } - }, - }, - { - name: "v1beta3.full", - fileContents: bytes.Join([][]byte{ - cfgFiles["InitConfiguration_v1beta3"], - cfgFiles["ClusterConfiguration_v1beta3"], - cfgFiles["Kube-proxy_componentconfig"], - cfgFiles["Kubelet_componentconfig"], - }, []byte(constants.YAMLDocumentSeparator)), - }, - { - name: "v1beta4.full", - fileContents: bytes.Join([][]byte{ - cfgFiles["InitConfiguration_v1beta4"], - cfgFiles["ClusterConfiguration_v1beta4"], - cfgFiles["Kube-proxy_componentconfig"], - cfgFiles["Kubelet_componentconfig"], - }, []byte(constants.YAMLDocumentSeparator)), + name: "Valid kubeadm config", + cfgPath: filePath, + fileContents: dedent.Dedent(` + apiVersion: kubeadm.k8s.io/v1beta4 + kind: InitConfiguration + `), + wantErr: false, }, } - - for _, rt := range tests { - t.Run(rt.name, func(t2 *testing.T) { - cfgPath := filepath.Join(tmpdir, rt.name) - err := os.WriteFile(cfgPath, rt.fileContents, 0644) - if err != nil { - t.Errorf("Couldn't create file: %v", err) - return - } - - opts := LoadOrDefaultConfigurationOptions{ - SkipCRIDetect: true, - } - - obj, err := LoadInitConfigurationFromFile(cfgPath, opts) - if rt.expectErr { - if err == nil { - t.Error("Unexpected success") - } - } else { + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.cfgPath == filePath { + err = os.WriteFile(tt.cfgPath, []byte(tt.fileContents), 0644) if err != nil { - t.Errorf("Error reading file: %v", err) - return - } - - if obj == nil { - t.Error("Unexpected nil return value") + t.Fatalf("Couldn't write content to file: %v", err) } + defer func() { + if err := os.RemoveAll(filePath); err != nil { + t.Fatalf("Couldn't remove filePath: %v", err) + } + }() } - // exec additional validation on the returned value - if rt.validate != nil { - rt.validate(t, obj) + + _, err = LoadInitConfigurationFromFile(tt.cfgPath, options) + if (err != nil) != tt.wantErr { + t.Errorf("LoadInitConfigurationFromFile() error = %v, wantErr %v", err, tt.wantErr) } }) } @@ -184,20 +150,167 @@ func TestDefaultTaintsMarshaling(t *testing.T) { } for _, tc := range tests { - t.Run(tc.desc, func(t *testing.T) { - b, err := yaml.Marshal(tc.cfg) - if err != nil { - t.Fatalf("unexpected error while marshalling to YAML: %v", err) - } + for _, format := range formats { + t.Run(fmt.Sprintf("%s_%s", tc.desc, format.name), func(t *testing.T) { + b, err := format.marshal(tc.cfg) + if err != nil { + t.Fatalf("unexpected error while marshalling to %s: %v", format.name, err) + } - cfg, err := BytesToInitConfiguration(b, true) - if err != nil { - t.Fatalf("unexpected error of BytesToInitConfiguration: %v\nconfig: %s", err, string(b)) - } + cfg, err := BytesToInitConfiguration(b, true) + if err != nil { + t.Fatalf("unexpected error of BytesToInitConfiguration: %v\nconfig: %s", err, string(b)) + } - if tc.expectedTaintCnt != len(cfg.NodeRegistration.Taints) { - t.Fatalf("unexpected taints count\nexpected: %d\ngot: %d\ntaints: %v", tc.expectedTaintCnt, len(cfg.NodeRegistration.Taints), cfg.NodeRegistration.Taints) - } - }) + if tc.expectedTaintCnt != len(cfg.NodeRegistration.Taints) { + t.Fatalf("unexpected taints count\nexpected: %d\ngot: %d\ntaints: %v", tc.expectedTaintCnt, len(cfg.NodeRegistration.Taints), cfg.NodeRegistration.Taints) + } + }) + } + } +} + +func TestBytesToInitConfiguration(t *testing.T) { + tests := []struct { + name string + cfg interface{} + expectedCfg kubeadmapi.InitConfiguration + expectedError bool + skipCRIDetect bool + }{ + { + name: "default config is set correctly", + cfg: kubeadmapiv1.InitConfiguration{ + TypeMeta: metav1.TypeMeta{ + APIVersion: kubeadmapiv1.SchemeGroupVersion.String(), + Kind: constants.InitConfigurationKind, + }, + }, + expectedCfg: kubeadmapi.InitConfiguration{ + LocalAPIEndpoint: kubeadmapi.APIEndpoint{ + AdvertiseAddress: "", + BindPort: 0, + }, + NodeRegistration: kubeadmapi.NodeRegistrationOptions{ + CRISocket: "unix:///var/run/containerd/containerd.sock", + Name: "", + Taints: []v1.Taint{ + { + Key: "node-role.kubernetes.io/control-plane", + Effect: "NoSchedule", + }, + }, + ImagePullPolicy: "IfNotPresent", + ImagePullSerial: ptr.To(true), + }, + ClusterConfiguration: kubeadmapi.ClusterConfiguration{ + Etcd: kubeadmapi.Etcd{ + Local: &kubeadmapi.LocalEtcd{ + DataDir: "/var/lib/etcd", + }, + }, + KubernetesVersion: "stable-1", + ImageRepository: kubeadmapiv1.DefaultImageRepository, + ClusterName: kubeadmapiv1.DefaultClusterName, + EncryptionAlgorithm: kubeadmapi.EncryptionAlgorithmType(kubeadmapiv1.DefaultEncryptionAlgorithm), + Networking: kubeadmapi.Networking{ + ServiceSubnet: "10.96.0.0/12", + DNSDomain: "cluster.local", + }, + CertificatesDir: "/etc/kubernetes/pki", + }, + }, + skipCRIDetect: true, + }, + { + name: "partial config with custom values", + cfg: kubeadmapiv1.InitConfiguration{ + TypeMeta: metav1.TypeMeta{ + APIVersion: kubeadmapiv1.SchemeGroupVersion.String(), + Kind: constants.InitConfigurationKind, + }, + NodeRegistration: kubeadmapiv1.NodeRegistrationOptions{ + Name: "test-node", + CRISocket: "unix:///var/run/containerd/containerd.sock", + }, + }, + expectedCfg: kubeadmapi.InitConfiguration{ + LocalAPIEndpoint: kubeadmapi.APIEndpoint{ + AdvertiseAddress: "", + BindPort: 0, + }, + NodeRegistration: kubeadmapi.NodeRegistrationOptions{ + CRISocket: "unix:///var/run/containerd/containerd.sock", + Name: "test-node", + Taints: []v1.Taint{ + { + Key: "node-role.kubernetes.io/control-plane", + Effect: "NoSchedule", + }, + }, + ImagePullPolicy: "IfNotPresent", + ImagePullSerial: ptr.To(true), + }, + ClusterConfiguration: kubeadmapi.ClusterConfiguration{ + Etcd: kubeadmapi.Etcd{ + Local: &kubeadmapi.LocalEtcd{ + DataDir: "/var/lib/etcd", + }, + }, + KubernetesVersion: "stable-1", + ImageRepository: kubeadmapiv1.DefaultImageRepository, + ClusterName: kubeadmapiv1.DefaultClusterName, + EncryptionAlgorithm: kubeadmapi.EncryptionAlgorithmType(kubeadmapiv1.DefaultEncryptionAlgorithm), + Networking: kubeadmapi.Networking{ + ServiceSubnet: "10.96.0.0/12", + DNSDomain: "cluster.local", + }, + CertificatesDir: "/etc/kubernetes/pki", + }, + }, + skipCRIDetect: true, + }, + { + name: "invalid configuration type", + cfg: kubeadmapiv1.UpgradeConfiguration{ + TypeMeta: metav1.TypeMeta{ + APIVersion: kubeadmapiv1.SchemeGroupVersion.String(), + Kind: constants.UpgradeConfigurationKind, + }, + }, + expectedError: true, + skipCRIDetect: true, + }, + } + + for _, tc := range tests { + for _, format := range formats { + t.Run(fmt.Sprintf("%s_%s", tc.name, format.name), func(t *testing.T) { + b, err := format.marshal(tc.cfg) + if err != nil { + t.Fatalf("unexpected error marshaling %s: %v", format.name, err) + } + + cfg, err := BytesToInitConfiguration(b, tc.skipCRIDetect) + if (err != nil) != tc.expectedError { + t.Fatalf("expected error: %v, got error: %v\nError: %v", tc.expectedError, err != nil, err) + } + + if !tc.expectedError { + // Ignore dynamic fields that may be set during defaulting + diffOpts := []cmp.Option{ + cmpopts.IgnoreFields(kubeadmapi.NodeRegistrationOptions{}, "Name"), + cmpopts.IgnoreFields(kubeadmapi.InitConfiguration{}, "Timeouts", "BootstrapTokens", "LocalAPIEndpoint"), + cmpopts.IgnoreFields(kubeadmapi.APIServer{}, "TimeoutForControlPlane"), + cmpopts.IgnoreFields(kubeadmapi.ClusterConfiguration{}, "ComponentConfigs", "KubernetesVersion", + "CertificateValidityPeriod", "CACertificateValidityPeriod"), + } + + if diff := cmp.Diff(*cfg, tc.expectedCfg, diffOpts...); diff != "" { + t.Fatalf("unexpected configuration difference (-want +got):\n%s", diff) + } + } + }) + } } } diff --git a/cmd/kubeadm/app/util/config/joinconfiguration_test.go b/cmd/kubeadm/app/util/config/joinconfiguration_test.go index 6459472c6fe..76ff1594120 100644 --- a/cmd/kubeadm/app/util/config/joinconfiguration_test.go +++ b/cmd/kubeadm/app/util/config/joinconfiguration_test.go @@ -17,85 +17,184 @@ limitations under the License. package config import ( + "fmt" "os" "path/filepath" "testing" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/lithammer/dedent" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + + kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" + kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta4" + "k8s.io/kubernetes/cmd/kubeadm/app/constants" ) +func TestBytesToJoinConfiguration(t *testing.T) { + options := LoadOrDefaultConfigurationOptions{} + + tests := []struct { + name string + cfg *kubeadmapiv1.JoinConfiguration + want *kubeadmapi.JoinConfiguration + }{ + { + name: "Normal configuration", + cfg: &kubeadmapiv1.JoinConfiguration{ + TypeMeta: metav1.TypeMeta{ + APIVersion: kubeadmapiv1.SchemeGroupVersion.String(), + Kind: constants.JoinConfigurationKind, + }, + NodeRegistration: kubeadmapiv1.NodeRegistrationOptions{ + Name: "node-1", + CRISocket: "unix:///var/run/crio/crio.sock", + }, + CACertPath: "/some/cert.crt", + Discovery: kubeadmapiv1.Discovery{ + BootstrapToken: &kubeadmapiv1.BootstrapTokenDiscovery{ + Token: "abcdef.1234567890123456", + APIServerEndpoint: "1.2.3.4:6443", + CACertHashes: []string{"aaaa"}, + }, + TLSBootstrapToken: "abcdef.1234567890123456", + }, + }, + want: &kubeadmapi.JoinConfiguration{ + NodeRegistration: kubeadmapi.NodeRegistrationOptions{ + Name: "node-1", + CRISocket: "unix:///var/run/crio/crio.sock", + ImagePullPolicy: "IfNotPresent", + ImagePullSerial: ptr.To(true), + }, + CACertPath: "/some/cert.crt", + Discovery: kubeadmapi.Discovery{ + BootstrapToken: &kubeadmapi.BootstrapTokenDiscovery{ + Token: "abcdef.1234567890123456", + APIServerEndpoint: "1.2.3.4:6443", + CACertHashes: []string{"aaaa"}, + }, + TLSBootstrapToken: "abcdef.1234567890123456", + }, + ControlPlane: nil, + }, + }, + { + name: "Only contains Discovery configuration", + cfg: &kubeadmapiv1.JoinConfiguration{ + TypeMeta: metav1.TypeMeta{ + APIVersion: kubeadmapiv1.SchemeGroupVersion.String(), + Kind: constants.JoinConfigurationKind, + }, + Discovery: kubeadmapiv1.Discovery{ + BootstrapToken: &kubeadmapiv1.BootstrapTokenDiscovery{ + Token: "abcdef.1234567890123456", + APIServerEndpoint: "1.2.3.4:6443", + CACertHashes: []string{"aaaa"}, + }, + TLSBootstrapToken: "abcdef.1234567890123456", + }, + }, + want: &kubeadmapi.JoinConfiguration{ + NodeRegistration: kubeadmapi.NodeRegistrationOptions{ + CRISocket: "unix:///var/run/containerd/containerd.sock", + ImagePullPolicy: "IfNotPresent", + ImagePullSerial: ptr.To(true), + }, + CACertPath: "/etc/kubernetes/pki/ca.crt", + Discovery: kubeadmapi.Discovery{ + BootstrapToken: &kubeadmapi.BootstrapTokenDiscovery{ + Token: "abcdef.1234567890123456", + APIServerEndpoint: "1.2.3.4:6443", + CACertHashes: []string{"aaaa"}, + }, + TLSBootstrapToken: "abcdef.1234567890123456", + }, + ControlPlane: nil, + }, + }, + } + + for _, tt := range tests { + for _, format := range formats { + t.Run(fmt.Sprintf("%s_%s", tt.name, format.name), func(t *testing.T) { + bytes, err := format.marshal(tt.cfg) + if err != nil { + t.Fatalf("Could not marshal test config: %v", err) + } + + got, _ := BytesToJoinConfiguration(bytes, options) + if diff := cmp.Diff(got, tt.want, cmpopts.IgnoreFields(kubeadmapi.JoinConfiguration{}, "Timeouts"), + cmpopts.IgnoreFields(kubeadmapi.Discovery{}, "Timeout"), cmpopts.IgnoreFields(kubeadmapi.NodeRegistrationOptions{}, "Name")); diff != "" { + t.Errorf("LoadJoinConfigurationFromFile returned unexpected diff (-want,+got):\n%s", diff) + } + }) + } + } +} + func TestLoadJoinConfigurationFromFile(t *testing.T) { // Create temp folder for the test case tmpdir, err := os.MkdirTemp("", "") if err != nil { t.Fatalf("Couldn't create tmpdir: %v", err) } - defer os.RemoveAll(tmpdir) + defer func() { + if err := os.RemoveAll(tmpdir); err != nil { + t.Fatalf("Couldn't remove tmpdir: %v", err) + } + }() + filename := "kubeadmConfig" + filePath := filepath.Join(tmpdir, filename) + options := LoadOrDefaultConfigurationOptions{} - // cfgFiles is in cluster_test.go - var tests = []struct { + tests := []struct { name string + cfgPath string fileContents string - expectErr bool + wantErr bool }{ { - name: "empty file causes error", - expectErr: true, + name: "Config file does not exists", + cfgPath: "tmp", + wantErr: true, }, { - name: "Invalid v1beta4 causes error", + name: "Valid kubeadm config", + cfgPath: filePath, fileContents: dedent.Dedent(` - apiVersion: kubeadm.k8s.io/v1beta4 - kind: JoinConfiguration - `), - expectErr: true, - }, - { - name: "valid v1beta4 is loaded", - fileContents: dedent.Dedent(` - apiVersion: kubeadm.k8s.io/v1beta4 - kind: JoinConfiguration - caCertPath: /etc/kubernetes/pki/ca.crt - nodeRegistration: - criSocket: "unix:///var/run/unknown.sock" - discovery: - bootstrapToken: - apiServerEndpoint: kube-apiserver:6443 - token: abcdef.0123456789abcdef - unsafeSkipCAVerification: true - timeout: 5m0s - tlsBootstrapToken: abcdef.0123456789abcdef - `), +apiVersion: kubeadm.k8s.io/v1beta4 +discovery: + bootstrapToken: + apiServerEndpoint: 1.2.3.4:6443 + caCertHashes: + - aaaa + token: abcdef.1234567890123456 + tlsBootstrapToken: abcdef.1234567890123456 +kind: JoinConfiguration`), + wantErr: false, }, } - - for _, rt := range tests { - t.Run(rt.name, func(t2 *testing.T) { - cfgPath := filepath.Join(tmpdir, rt.name) - err := os.WriteFile(cfgPath, []byte(rt.fileContents), 0644) - if err != nil { - t.Errorf("Couldn't create file: %v", err) - return - } - - opts := LoadOrDefaultConfigurationOptions{ - SkipCRIDetect: true, - } - - obj, err := LoadJoinConfigurationFromFile(cfgPath, opts) - if rt.expectErr { - if err == nil { - t.Error("Unexpected success") - } - } else { + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.cfgPath == filePath { + err = os.WriteFile(tt.cfgPath, []byte(tt.fileContents), 0644) if err != nil { - t.Errorf("Error reading file: %v", err) - return + t.Fatalf("Couldn't write content to file: %v", err) } + defer func() { + if err := os.RemoveAll(filePath); err != nil { + t.Fatalf("Couldn't remove filePath: %v", err) + } + }() + } - if obj == nil { - t.Error("Unexpected nil return value") - } + _, err = LoadJoinConfigurationFromFile(tt.cfgPath, options) + if (err != nil) != tt.wantErr { + t.Errorf("LoadJoinConfigurationFromFile() error = %v, wantErr %v", err, tt.wantErr) } }) } diff --git a/cmd/kubeadm/app/util/config/resetconfiguration_test.go b/cmd/kubeadm/app/util/config/resetconfiguration_test.go index 1d9676a133f..b631940bf71 100644 --- a/cmd/kubeadm/app/util/config/resetconfiguration_test.go +++ b/cmd/kubeadm/app/util/config/resetconfiguration_test.go @@ -17,86 +17,132 @@ limitations under the License. package config import ( + "fmt" "os" "path/filepath" "testing" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/lithammer/dedent" "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" + kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta4" + "k8s.io/kubernetes/cmd/kubeadm/app/constants" ) +func TestBytesToResetConfiguration(t *testing.T) { + options := LoadOrDefaultConfigurationOptions{} + + tests := []struct { + name string + cfg *kubeadmapiv1.ResetConfiguration + want *kubeadmapi.ResetConfiguration + }{ + { + name: "Normal configuration", + cfg: &kubeadmapiv1.ResetConfiguration{ + TypeMeta: metav1.TypeMeta{ + APIVersion: kubeadmapiv1.SchemeGroupVersion.String(), + Kind: constants.ResetConfigurationKind, + }, + CertificatesDir: "/custom/certs", + CleanupTmpDir: true, + }, + want: &kubeadmapi.ResetConfiguration{ + CertificatesDir: "/custom/certs", + CleanupTmpDir: true, + SkipPhases: nil, + CRISocket: "unix:///var/run/containerd/containerd.sock", + }, + }, + { + name: "Default configuration", + cfg: &kubeadmapiv1.ResetConfiguration{ + TypeMeta: metav1.TypeMeta{ + APIVersion: kubeadmapiv1.SchemeGroupVersion.String(), + Kind: constants.ResetConfigurationKind, + }, + }, + want: &kubeadmapi.ResetConfiguration{ + CertificatesDir: "/etc/kubernetes/pki", + CleanupTmpDir: false, + SkipPhases: nil, + CRISocket: "unix:///var/run/containerd/containerd.sock", + }, + }, + } + + for _, tt := range tests { + for _, format := range formats { + t.Run(fmt.Sprintf("%s_%s", tt.name, format.name), func(t *testing.T) { + bytes, err := format.marshal(tt.cfg) + if err != nil { + t.Fatalf("Could not marshal test config: %v", err) + } + got, _ := BytesToResetConfiguration(bytes, options) + if diff := cmp.Diff(got, tt.want, cmpopts.IgnoreFields(kubeadmapi.ResetConfiguration{}, "Timeouts")); diff != "" { + t.Errorf("LoadResetConfigurationFromFile returned unexpected diff (-want,+got):\n%s", diff) + } + }) + } + } +} + func TestLoadResetConfigurationFromFile(t *testing.T) { - // Create temp folder for the test case tmpdir, err := os.MkdirTemp("", "") if err != nil { t.Fatalf("Couldn't create tmpdir: %v", err) } - defer os.RemoveAll(tmpdir) + defer func() { + if err := os.RemoveAll(tmpdir); err != nil { + t.Fatalf("Couldn't remove tmpdir: %v", err) + } + }() + filename := "kubeadmConfig" + filePath := filepath.Join(tmpdir, filename) + options := LoadOrDefaultConfigurationOptions{} - var tests = []struct { + tests := []struct { name string + cfgPath string fileContents string - expectErr bool + wantErr bool }{ { - name: "empty file causes error", - expectErr: true, + name: "Config file does not exists", + cfgPath: "tmp", + wantErr: true, }, { - name: "Invalid v1beta4 causes error", - fileContents: dedent.Dedent(` - apiVersion: kubeadm.k8s.io/unknownVersion - kind: ResetConfiguration - criSocket: unix:///var/run/containerd/containerd.sock - `), - expectErr: true, - }, - { - name: "valid v1beta4 is loaded", + name: "Valid kubeadm config", + cfgPath: filePath, fileContents: dedent.Dedent(` apiVersion: kubeadm.k8s.io/v1beta4 - kind: ResetConfiguration - force: true - cleanupTmpDir: true - criSocket: unix:///var/run/containerd/containerd.sock - certificatesDir: /etc/kubernetes/pki - ignorePreflightErrors: - - a - - b - `), + kind: ResetConfiguration`), + wantErr: false, }, } - - for _, rt := range tests { - t.Run(rt.name, func(t2 *testing.T) { - cfgPath := filepath.Join(tmpdir, rt.name) - err := os.WriteFile(cfgPath, []byte(rt.fileContents), 0644) - if err != nil { - t.Errorf("Couldn't create file: %v", err) - return - } - - opts := LoadOrDefaultConfigurationOptions{ - AllowExperimental: true, - SkipCRIDetect: true, - } - - obj, err := LoadResetConfigurationFromFile(cfgPath, opts) - if rt.expectErr { - if err == nil { - t.Error("Unexpected success") - } - } else { + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.cfgPath == filePath { + err = os.WriteFile(tt.cfgPath, []byte(tt.fileContents), 0644) if err != nil { - t.Errorf("Error reading file: %v", err) - return + t.Fatalf("Couldn't write content to file: %v", err) } + defer func() { + if err := os.RemoveAll(filePath); err != nil { + t.Fatalf("Couldn't remove filePath: %v", err) + } + }() + } - if obj == nil { - t.Error("Unexpected nil return value") - } + _, err = LoadResetConfigurationFromFile(tt.cfgPath, options) + if (err != nil) != tt.wantErr { + t.Errorf("LoadResetConfigurationFromFile() error = %v, wantErr %v", err, tt.wantErr) } }) } diff --git a/cmd/kubeadm/app/util/config/upgradeconfiguration.go b/cmd/kubeadm/app/util/config/upgradeconfiguration.go index 200818da075..d42da035d6b 100644 --- a/cmd/kubeadm/app/util/config/upgradeconfiguration.go +++ b/cmd/kubeadm/app/util/config/upgradeconfiguration.go @@ -77,11 +77,6 @@ func documentMapToUpgradeConfiguration(gvkmap kubeadmapi.DocumentMap, allowDepre return internalcfg, nil } -// DocMapToUpgradeConfiguration converts documentMap to an internal, defaulted and validated UpgradeConfiguration object. -func DocMapToUpgradeConfiguration(gvkmap kubeadmapi.DocumentMap) (*kubeadmapi.UpgradeConfiguration, error) { - return documentMapToUpgradeConfiguration(gvkmap, false) -} - // LoadUpgradeConfigurationFromFile loads UpgradeConfiguration from a file. func LoadUpgradeConfigurationFromFile(cfgPath string, _ LoadOrDefaultConfigurationOptions) (*kubeadmapi.UpgradeConfiguration, error) { var err error diff --git a/cmd/kubeadm/app/util/config/upgradeconfiguration_test.go b/cmd/kubeadm/app/util/config/upgradeconfiguration_test.go index f682ee32582..1dafa4348b4 100644 --- a/cmd/kubeadm/app/util/config/upgradeconfiguration_test.go +++ b/cmd/kubeadm/app/util/config/upgradeconfiguration_test.go @@ -17,6 +17,7 @@ limitations under the License. package config import ( + "fmt" "os" "path/filepath" "testing" @@ -26,17 +27,15 @@ import ( "github.com/lithammer/dedent" v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" - "sigs.k8s.io/yaml" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta4" "k8s.io/kubernetes/cmd/kubeadm/app/constants" - kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" ) -func TestDocMapToUpgradeConfiguration(t *testing.T) { +func TestBytesToUpgradeConfiguration(t *testing.T) { tests := []struct { name string cfg interface{} @@ -117,25 +116,25 @@ func TestDocMapToUpgradeConfiguration(t *testing.T) { } for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - b, err := yaml.Marshal(tc.cfg) - if err != nil { - t.Fatalf("unexpected error while marshalling to YAML: %v", err) - } - docmap, err := kubeadmutil.SplitConfigDocuments(b) - if err != nil { - t.Fatalf("Unexpected error of SplitConfigDocuments: %v", err) - } - cfg, err := DocMapToUpgradeConfiguration(docmap) - if (err != nil) != tc.expectedError { - t.Fatalf("failed DocMapToUpgradeConfiguration:\n\texpected error: %t\n\t actual error: %v", tc.expectedError, err) - } - if err == nil { - if diff := cmp.Diff(*cfg, tc.expectedCfg, cmpopts.IgnoreFields(kubeadmapi.UpgradeConfiguration{}, "Timeouts")); diff != "" { - t.Fatalf("DocMapToUpgradeConfiguration returned unexpected diff (-want,+got):\n%s", diff) + for _, format := range formats { + t.Run(fmt.Sprintf("%s_%s", tc.name, format.name), func(t *testing.T) { + b, err := format.marshal(tc.cfg) + if err != nil { + t.Fatalf("unexpected error while marshalling to %s: %v", format.name, err) } - } - }) + + cfg, err := BytesToUpgradeConfiguration(b) + if (err != nil) != tc.expectedError { + t.Fatalf("failed BytesToUpgradeConfiguration:\n\texpected error: %t\n\t actual error: %v", tc.expectedError, err) + } + + if err == nil { + if diff := cmp.Diff(*cfg, tc.expectedCfg, cmpopts.IgnoreFields(kubeadmapi.UpgradeConfiguration{}, "Timeouts")); diff != "" { + t.Fatalf("BytesToUpgradeConfiguration returned unexpected diff (-want,+got):\n%s", diff) + } + } + }) + } } } @@ -157,30 +156,11 @@ func TestLoadUpgradeConfigurationFromFile(t *testing.T) { name string cfgPath string fileContents string - want *kubeadmapi.UpgradeConfiguration wantErr bool }{ { name: "Config file does not exists", cfgPath: "tmp", - want: nil, - wantErr: true, - }, - { - name: "Config file format is basic text", - cfgPath: filePath, - want: nil, - fileContents: "some-text", - wantErr: true, - }, - { - name: "Unknown kind UpgradeConfiguration for kubeadm.k8s.io/unknown", - cfgPath: filePath, - fileContents: dedent.Dedent(` - apiVersion: kubeadm.k8s.io/unknown - kind: UpgradeConfiguration - `), - want: nil, wantErr: true, }, { @@ -188,24 +168,8 @@ func TestLoadUpgradeConfigurationFromFile(t *testing.T) { cfgPath: filePath, fileContents: dedent.Dedent(` apiVersion: kubeadm.k8s.io/v1beta4 - kind: UpgradeConfiguration`), - want: &kubeadmapi.UpgradeConfiguration{ - Apply: kubeadmapi.UpgradeApplyConfiguration{ - CertificateRenewal: ptr.To(true), - EtcdUpgrade: ptr.To(true), - ImagePullPolicy: v1.PullIfNotPresent, - ImagePullSerial: ptr.To(true), - }, - Node: kubeadmapi.UpgradeNodeConfiguration{ - CertificateRenewal: ptr.To(true), - EtcdUpgrade: ptr.To(true), - ImagePullPolicy: v1.PullIfNotPresent, - ImagePullSerial: ptr.To(true), - }, - Plan: kubeadmapi.UpgradePlanConfiguration{ - EtcdUpgrade: ptr.To(true), - }, - }, + kind: UpgradeConfiguration + `), wantErr: false, }, } @@ -223,17 +187,10 @@ func TestLoadUpgradeConfigurationFromFile(t *testing.T) { }() } - got, err := LoadUpgradeConfigurationFromFile(tt.cfgPath, options) + _, err = LoadUpgradeConfigurationFromFile(tt.cfgPath, options) if (err != nil) != tt.wantErr { t.Errorf("LoadUpgradeConfigurationFromFile() error = %v, wantErr %v", err, tt.wantErr) } - if tt.want == nil && got != tt.want { - t.Errorf("LoadUpgradeConfigurationFromFile() got = %v, want %v", got, tt.want) - } else if tt.want != nil { - if diff := cmp.Diff(got, tt.want, cmpopts.IgnoreFields(kubeadmapi.UpgradeConfiguration{}, "Timeouts")); diff != "" { - t.Errorf("LoadUpgradeConfigurationFromFile returned unexpected diff (-want,+got):\n%s", diff) - } - } }) } } @@ -415,21 +372,24 @@ func TestLoadOrDefaultUpgradeConfiguration(t *testing.T) { }, }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - bytes, err := yaml.Marshal(tt.cfg) - if err != nil { - t.Fatalf("Could not marshal test config: %v", err) - } - err = os.WriteFile(filePath, bytes, 0644) - if err != nil { - t.Fatalf("Couldn't write content to file: %v", err) - } - got, _ := LoadOrDefaultUpgradeConfiguration(tt.cfgPath, tt.cfg, options) - if diff := cmp.Diff(got, tt.want, cmpopts.IgnoreFields(kubeadmapi.UpgradeConfiguration{}, "Timeouts")); diff != "" { - t.Errorf("LoadOrDefaultUpgradeConfiguration returned unexpected diff (-want,+got):\n%s", diff) - } - }) + for _, tt := range tests { + for _, format := range formats { + t.Run(fmt.Sprintf("%s_%s", tt.name, format.name), func(t *testing.T) { + bytes, err := format.marshal(tt.cfg) + if err != nil { + t.Fatalf("Could not marshal test config: %v", err) + } + err = os.WriteFile(filePath, bytes, 0644) + if err != nil { + t.Fatalf("Couldn't write content to file: %v", err) + } + + got, _ := LoadOrDefaultUpgradeConfiguration(tt.cfgPath, tt.cfg, options) + if diff := cmp.Diff(got, tt.want, cmpopts.IgnoreFields(kubeadmapi.UpgradeConfiguration{}, "Timeouts")); diff != "" { + t.Errorf("LoadOrDefaultUpgradeConfiguration returned unexpected diff (-want,+got):\n%s", diff) + } + }) + } } }