From 6a59c98a9e85122444526377c40b32a8721d6029 Mon Sep 17 00:00:00 2001 From: HirazawaUi <695097494plus@gmail.com> Date: Mon, 3 Mar 2025 18:35:17 +0800 Subject: [PATCH 1/5] distinguish between YAML and JSON file formats during log output --- cmd/kubeadm/app/util/config/cluster.go | 4 ++-- cmd/kubeadm/app/util/config/common.go | 4 ++-- cmd/kubeadm/app/util/config/initconfiguration.go | 14 +++++++------- cmd/kubeadm/app/util/config/joinconfiguration.go | 4 ++-- cmd/kubeadm/app/util/config/resetconfiguration.go | 4 ++-- .../app/util/config/upgradeconfiguration.go | 14 ++++---------- 6 files changed, 19 insertions(+), 25 deletions(-) diff --git a/cmd/kubeadm/app/util/config/cluster.go b/cmd/kubeadm/app/util/config/cluster.go index 1c228b6a6a9..4547da040ee 100644 --- a/cmd/kubeadm/app/util/config/cluster.go +++ b/cmd/kubeadm/app/util/config/cluster.go @@ -60,7 +60,7 @@ func FetchInitConfigurationFromCluster(client clientset.Interface, printer outpu } _, _ = printer.Printf("[%s] Reading configuration from the %q ConfigMap in namespace %q...\n", logPrefix, constants.KubeadmConfigConfigMap, metav1.NamespaceSystem) - _, _ = printer.Printf("[%s] Use 'kubeadm init phase upload-config --config your-config.yaml' to re-upload it.\n", logPrefix) + _, _ = printer.Printf("[%s] Use 'kubeadm init phase upload-config --config your-config-file' to re-upload it.\n", logPrefix) // Fetch the actual config from cluster cfg, err := getInitConfigurationFromCluster(constants.KubernetesDir, client, newControlPlane, skipComponentConfigs) @@ -214,7 +214,7 @@ func GetNodeRegistration(kubeconfigFile string, client clientset.Interface, node // getNodeNameFromKubeletConfig gets the node name from a kubelet config file // TODO: in future we want to switch to a more canonical way for doing this e.g. by having this -// information in the local kubelet config.yaml +// information in the local kubelet config configuration file. func getNodeNameFromKubeletConfig(fileName string) (string, error) { // loads the kubelet.conf file config, err := clientcmd.LoadFromFile(fileName) diff --git a/cmd/kubeadm/app/util/config/common.go b/cmd/kubeadm/app/util/config/common.go index 4df881a0bcd..7edadd2d2fd 100644 --- a/cmd/kubeadm/app/util/config/common.go +++ b/cmd/kubeadm/app/util/config/common.go @@ -94,11 +94,11 @@ func validateSupportedVersion(gvk schema.GroupVersionKind, allowDeprecated, allo gvString := gvk.GroupVersion().String() if useKubeadmVersion := oldKnownAPIVersions[gvString]; useKubeadmVersion != "" { - return errors.Errorf("your configuration file uses an old API spec: %q (kind: %q). Please use kubeadm %s instead and run 'kubeadm config migrate --old-config old.yaml --new-config new.yaml', which will write the new, similar spec using a newer API version.", gvString, gvk.Kind, useKubeadmVersion) + return errors.Errorf("your configuration file uses an old API spec: %q (kind: %q). Please use kubeadm %s instead and run 'kubeadm config migrate --old-config old-config-file --new-config new-config-file', which will write the new, similar spec using a newer API version.", gvString, gvk.Kind, useKubeadmVersion) } if _, present := deprecatedAPIVersions[gvString]; present && !allowDeprecated { - klog.Warningf("your configuration file uses a deprecated API spec: %q (kind: %q). Please use 'kubeadm config migrate --old-config old.yaml --new-config new.yaml', which will write the new, similar spec using a newer API version.", gvString, gvk.Kind) + klog.Warningf("your configuration file uses a deprecated API spec: %q (kind: %q). Please use 'kubeadm config migrate --old-config old-config-file --new-config new-config-file', which will write the new, similar spec using a newer API version.", gvString, gvk.Kind) } if _, present := experimentalAPIVersions[gvString]; present && !allowExperimental { diff --git a/cmd/kubeadm/app/util/config/initconfiguration.go b/cmd/kubeadm/app/util/config/initconfiguration.go index 1566b8b1b5f..203d219c1f9 100644 --- a/cmd/kubeadm/app/util/config/initconfiguration.go +++ b/cmd/kubeadm/app/util/config/initconfiguration.go @@ -291,7 +291,7 @@ func LoadOrDefaultInitConfiguration(cfgPath string, versionedInitCfg *kubeadmapi } // BytesToInitConfiguration converts a byte slice to an internal, defaulted and validated InitConfiguration object. -// The map may contain many different YAML documents. These YAML documents are parsed one-by-one +// The map may contain many different YAML/JSON documents. These YAML/JSON documents are parsed one-by-one // and well-known ComponentConfig GroupVersionKinds are stored inside of the internal InitConfiguration struct. // The resulting InitConfiguration is then dynamically defaulted and validated prior to return. func BytesToInitConfiguration(b []byte, skipCRIDetect bool) (*kubeadmapi.InitConfiguration, error) { @@ -303,7 +303,7 @@ func BytesToInitConfiguration(b []byte, skipCRIDetect bool) (*kubeadmapi.InitCon return documentMapToInitConfiguration(gvkmap, false, false, false, skipCRIDetect) } -// documentMapToInitConfiguration converts a map of GVKs and YAML documents to defaulted and validated configuration object. +// documentMapToInitConfiguration converts a map of GVKs and YAML/JSON documents to defaulted and validated configuration object. func documentMapToInitConfiguration(gvkmap kubeadmapi.DocumentMap, allowDeprecated, allowExperimental, strictErrors, skipCRIDetect bool) (*kubeadmapi.InitConfiguration, error) { var initcfg *kubeadmapi.InitConfiguration var clustercfg *kubeadmapi.ClusterConfiguration @@ -326,7 +326,7 @@ func documentMapToInitConfiguration(gvkmap kubeadmapi.DocumentMap, allowDeprecat return nil, err } - // verify the validity of the YAML + // verify the validity of the JSON/YAML if err := strict.VerifyUnmarshalStrict([]*runtime.Scheme{kubeadmscheme.Scheme, componentconfigs.Scheme}, gvk, fileContent); err != nil { if !strictErrors { klog.Warning(err.Error()) @@ -358,13 +358,13 @@ func documentMapToInitConfiguration(gvkmap kubeadmapi.DocumentMap, allowDeprecat // If the group is neither a kubeadm core type or of a supported component config group, we dump a warning about it being ignored if !componentconfigs.Scheme.IsGroupRegistered(gvk.Group) { - klog.Warningf("[config] WARNING: Ignored YAML document with GroupVersionKind %v\n", gvk) + klog.Warningf("[config] WARNING: Ignored configuration document with GroupVersionKind %v\n", gvk) } } - // Enforce that InitConfiguration and/or ClusterConfiguration has to exist among the YAML documents + // Enforce that InitConfiguration and/or ClusterConfiguration has to exist among the configuration documents if initcfg == nil && clustercfg == nil { - return nil, errors.New("no InitConfiguration or ClusterConfiguration kind was found in the YAML file") + return nil, errors.New("no InitConfiguration or ClusterConfiguration kind was found in the configuration file") } // If InitConfiguration wasn't given, default it by creating an external struct instance, default it and convert into the internal type @@ -408,7 +408,7 @@ func documentMapToInitConfiguration(gvkmap kubeadmapi.DocumentMap, allowDeprecat } // MarshalInitConfigurationToBytes marshals the internal InitConfiguration object to bytes. It writes the embedded -// ClusterConfiguration object with ComponentConfigs out as separate YAML documents +// ClusterConfiguration object with ComponentConfigs out as separate YAML/JSON documents func MarshalInitConfigurationToBytes(cfg *kubeadmapi.InitConfiguration, gv schema.GroupVersion) ([]byte, error) { initbytes, err := kubeadmutil.MarshalToYamlForCodecs(cfg, gv, kubeadmscheme.Codecs) if err != nil { diff --git a/cmd/kubeadm/app/util/config/joinconfiguration.go b/cmd/kubeadm/app/util/config/joinconfiguration.go index 6aa4f4c5b74..133763074dd 100644 --- a/cmd/kubeadm/app/util/config/joinconfiguration.go +++ b/cmd/kubeadm/app/util/config/joinconfiguration.go @@ -95,7 +95,7 @@ func LoadJoinConfigurationFromFile(cfgPath string, opts LoadOrDefaultConfigurati return documentMapToJoinConfiguration(gvkmap, false, false, false, opts.SkipCRIDetect) } -// documentMapToJoinConfiguration takes a map between GVKs and YAML documents (as returned by SplitYAMLDocuments), +// documentMapToJoinConfiguration takes a map between GVKs and YAML/JSON documents (as returned by SplitYAMLDocuments), // finds a JoinConfiguration, decodes it, dynamically defaults it and then validates it prior to return. func documentMapToJoinConfiguration(gvkmap kubeadmapi.DocumentMap, allowDeprecated, allowExperimental, strictErrors, skipCRIDetect bool) (*kubeadmapi.JoinConfiguration, error) { joinBytes := []byte{} @@ -110,7 +110,7 @@ func documentMapToJoinConfiguration(gvkmap kubeadmapi.DocumentMap, allowDeprecat return nil, err } - // verify the validity of the YAML + // verify the validity of the YAML/JSON if err := strict.VerifyUnmarshalStrict([]*runtime.Scheme{kubeadmscheme.Scheme}, gvk, bytes); err != nil { if !strictErrors { klog.Warning(err.Error()) diff --git a/cmd/kubeadm/app/util/config/resetconfiguration.go b/cmd/kubeadm/app/util/config/resetconfiguration.go index 06616bc18e3..6e08b0b3c36 100644 --- a/cmd/kubeadm/app/util/config/resetconfiguration.go +++ b/cmd/kubeadm/app/util/config/resetconfiguration.go @@ -99,7 +99,7 @@ func LoadResetConfigurationFromFile(cfgPath string, opts LoadOrDefaultConfigurat return documentMapToResetConfiguration(gvkmap, false, opts.AllowExperimental, false, opts.SkipCRIDetect) } -// documentMapToResetConfiguration takes a map between GVKs and YAML documents (as returned by SplitYAMLDocuments), +// documentMapToResetConfiguration takes a map between GVKs and YAML/JSON documents (as returned by SplitYAMLDocuments), // finds a ResetConfiguration, decodes it, dynamically defaults it and then validates it prior to return. func documentMapToResetConfiguration(gvkmap kubeadmapi.DocumentMap, allowDeprecated, allowExperimental bool, strictErrors bool, skipCRIDetect bool) (*kubeadmapi.ResetConfiguration, error) { resetBytes := []byte{} @@ -114,7 +114,7 @@ func documentMapToResetConfiguration(gvkmap kubeadmapi.DocumentMap, allowDepreca return nil, err } - // verify the validity of the YAML + // verify the validity of the YAML/JSON if err := strict.VerifyUnmarshalStrict([]*runtime.Scheme{kubeadmscheme.Scheme}, gvk, bytes); err != nil { if !strictErrors { klog.Warning(err.Error()) diff --git a/cmd/kubeadm/app/util/config/upgradeconfiguration.go b/cmd/kubeadm/app/util/config/upgradeconfiguration.go index c5688a551f7..19488422d4a 100644 --- a/cmd/kubeadm/app/util/config/upgradeconfiguration.go +++ b/cmd/kubeadm/app/util/config/upgradeconfiguration.go @@ -28,25 +28,19 @@ import ( kubeadmscheme "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/scheme" kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta4" "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/validation" - "k8s.io/kubernetes/cmd/kubeadm/app/componentconfigs" "k8s.io/kubernetes/cmd/kubeadm/app/constants" kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" "k8s.io/kubernetes/cmd/kubeadm/app/util/config/strict" ) -// documentMapToUpgradeConfiguration takes a map between GVKs and YAML documents (as returned by SplitYAMLDocuments), +// documentMapToUpgradeConfiguration takes a map between GVKs and YAML/JSON documents (as returned by SplitYAMLDocuments), // finds a UpgradeConfiguration, decodes it, dynamically defaults it and then validates it prior to return. func documentMapToUpgradeConfiguration(gvkmap kubeadmapi.DocumentMap, allowDeprecated bool) (*kubeadmapi.UpgradeConfiguration, error) { upgradeBytes := []byte{} for gvk, bytes := range gvkmap { - if kubeadmutil.GroupVersionKindsHasInitConfiguration(gvk) || kubeadmutil.GroupVersionKindsHasClusterConfiguration(gvk) || componentconfigs.Scheme.IsGroupRegistered(gvk.Group) { - klog.Warningf("[config] WARNING: YAML document with GroupVersionKind %v is deprecated for upgrade, please use config file with kind of UpgradeConfiguration instead \n", gvk) - continue - } - if gvk.Kind != constants.UpgradeConfigurationKind { - klog.Warningf("[config] WARNING: Ignored YAML document with GroupVersionKind %v\n", gvk) + klog.Warningf("[config] WARNING: Ignored configuration document with GroupVersionKind %v\n", gvk) continue } @@ -55,7 +49,7 @@ func documentMapToUpgradeConfiguration(gvkmap kubeadmapi.DocumentMap, allowDepre return nil, err } - // verify the validity of the YAML + // verify the validity of the YAML/JSON if err := strict.VerifyUnmarshalStrict([]*runtime.Scheme{kubeadmscheme.Scheme}, gvk, bytes); err != nil { klog.Warning(err.Error()) } @@ -99,7 +93,7 @@ func LoadUpgradeConfigurationFromFile(cfgPath string, _ LoadOrDefaultConfigurati return nil, errors.Wrapf(err, "unable to load config from file %q", cfgPath) } - // Split the YAML documents in the file into a DocumentMap + // Split the YAML/JSON documents in the file into a DocumentMap docmap, err := kubeadmutil.SplitYAMLDocuments(configBytes) if err != nil { return nil, err From 2d8d972cb85243b82fa20aac009e034e808bc7c3 Mon Sep 17 00:00:00 2001 From: HirazawaUi <695097494plus@gmail.com> Date: Fri, 21 Feb 2025 22:30:38 +0800 Subject: [PATCH 2/5] Add warning logs for uninteresting kind --- cmd/kubeadm/app/util/config/joinconfiguration.go | 1 + cmd/kubeadm/app/util/config/resetconfiguration.go | 1 + 2 files changed, 2 insertions(+) diff --git a/cmd/kubeadm/app/util/config/joinconfiguration.go b/cmd/kubeadm/app/util/config/joinconfiguration.go index 133763074dd..08540d568a0 100644 --- a/cmd/kubeadm/app/util/config/joinconfiguration.go +++ b/cmd/kubeadm/app/util/config/joinconfiguration.go @@ -102,6 +102,7 @@ func documentMapToJoinConfiguration(gvkmap kubeadmapi.DocumentMap, allowDeprecat for gvk, bytes := range gvkmap { // not interested in anything other than JoinConfiguration if gvk.Kind != constants.JoinConfigurationKind { + klog.Warningf("[config] WARNING: Ignored configuration document with GroupVersionKind %v\n", gvk) continue } diff --git a/cmd/kubeadm/app/util/config/resetconfiguration.go b/cmd/kubeadm/app/util/config/resetconfiguration.go index 6e08b0b3c36..2cc02e6fff7 100644 --- a/cmd/kubeadm/app/util/config/resetconfiguration.go +++ b/cmd/kubeadm/app/util/config/resetconfiguration.go @@ -106,6 +106,7 @@ func documentMapToResetConfiguration(gvkmap kubeadmapi.DocumentMap, allowDepreca for gvk, bytes := range gvkmap { // not interested in anything other than ResetConfiguration if gvk.Kind != constants.ResetConfigurationKind { + klog.Warningf("[config] WARNING: Ignored configuration document with GroupVersionKind %v\n", gvk) continue } From 77647cdfc3ef0f2e22de53c1a7b8f10574e55cea Mon Sep 17 00:00:00 2001 From: HirazawaUi <695097494plus@gmail.com> Date: Fri, 21 Feb 2025 22:34:52 +0800 Subject: [PATCH 3/5] rename SplitYAMLDocuments function --- cmd/kubeadm/app/cmd/config_test.go | 4 ++-- cmd/kubeadm/app/componentconfigs/configset.go | 2 +- cmd/kubeadm/app/componentconfigs/configset_test.go | 4 ++-- cmd/kubeadm/app/componentconfigs/fakeconfig_test.go | 8 ++++---- cmd/kubeadm/app/phases/kubelet/config.go | 2 +- cmd/kubeadm/app/util/config/common.go | 4 ++-- cmd/kubeadm/app/util/config/initconfiguration.go | 2 +- cmd/kubeadm/app/util/config/joinconfiguration.go | 2 +- cmd/kubeadm/app/util/config/resetconfiguration.go | 2 +- cmd/kubeadm/app/util/config/upgradeconfiguration.go | 2 +- cmd/kubeadm/app/util/config/upgradeconfiguration_test.go | 4 ++-- cmd/kubeadm/app/util/marshal.go | 8 ++++---- cmd/kubeadm/app/util/marshal_test.go | 2 +- 13 files changed, 23 insertions(+), 23 deletions(-) diff --git a/cmd/kubeadm/app/cmd/config_test.go b/cmd/kubeadm/app/cmd/config_test.go index d65c8105740..7a9d3813d9f 100644 --- a/cmd/kubeadm/app/cmd/config_test.go +++ b/cmd/kubeadm/app/cmd/config_test.go @@ -399,9 +399,9 @@ func TestNewCmdConfigPrintActionDefaults(t *testing.T) { t.Fatalf("Error from running the print command: %v", err) } - gvkmap, err := kubeadmutil.SplitYAMLDocuments(output.Bytes()) + gvkmap, err := kubeadmutil.SplitConfigDocuments(output.Bytes()) if err != nil { - t.Fatalf("unexpected failure of SplitYAMLDocuments: %v", err) + t.Fatalf("unexpected failure of SplitConfigDocuments: %v", err) } gotKinds := []string{} diff --git a/cmd/kubeadm/app/componentconfigs/configset.go b/cmd/kubeadm/app/componentconfigs/configset.go index a25d96ad4b3..693d5a60b75 100644 --- a/cmd/kubeadm/app/componentconfigs/configset.go +++ b/cmd/kubeadm/app/componentconfigs/configset.go @@ -86,7 +86,7 @@ func (h *handler) fromConfigMap(client clientset.Interface, cmName, cmKey string return nil, errors.Errorf("unexpected error when reading %s ConfigMap: %s key value pair missing", cmName, cmKey) } - gvkmap, err := kubeadmutil.SplitYAMLDocuments([]byte(configData)) + gvkmap, err := kubeadmutil.SplitConfigDocuments([]byte(configData)) if err != nil { return nil, err } diff --git a/cmd/kubeadm/app/componentconfigs/configset_test.go b/cmd/kubeadm/app/componentconfigs/configset_test.go index cfa2837d199..d41317997bb 100644 --- a/cmd/kubeadm/app/componentconfigs/configset_test.go +++ b/cmd/kubeadm/app/componentconfigs/configset_test.go @@ -78,9 +78,9 @@ func TestFetchFromDocumentMap(t *testing.T) { apiVersion: kubelet.config.k8s.io/v1beta1 kind: KubeletConfiguration `) - gvkmap, err := kubeadmutil.SplitYAMLDocuments([]byte(test)) + gvkmap, err := kubeadmutil.SplitConfigDocuments([]byte(test)) if err != nil { - t.Fatalf("unexpected failure of SplitYAMLDocuments: %v", err) + t.Fatalf("unexpected failure of SplitConfigDocuments: %v", err) } clusterCfg := testClusterCfg() diff --git a/cmd/kubeadm/app/componentconfigs/fakeconfig_test.go b/cmd/kubeadm/app/componentconfigs/fakeconfig_test.go index 1c1ac7301a4..a8f3bf90eae 100644 --- a/cmd/kubeadm/app/componentconfigs/fakeconfig_test.go +++ b/cmd/kubeadm/app/componentconfigs/fakeconfig_test.go @@ -293,9 +293,9 @@ func TestConfigBaseUnmarshal(t *testing.T) { config: validUnmarshallableClusterConfig.obj, } - gvkmap, err := kubeadmutil.SplitYAMLDocuments([]byte(validUnmarshallableClusterConfig.yaml)) + gvkmap, err := kubeadmutil.SplitConfigDocuments([]byte(validUnmarshallableClusterConfig.yaml)) if err != nil { - t.Fatalf("unexpected failure of SplitYAMLDocuments: %v", err) + t.Fatalf("unexpected failure of SplitConfigDocuments: %v", err) } got := &clusterConfig{ @@ -461,9 +461,9 @@ func runClusterConfigFromTest(t *testing.T, perform func(t *testing.T, in string func TestLoadingFromDocumentMap(t *testing.T) { runClusterConfigFromTest(t, func(t *testing.T, in string) (kubeadmapi.ComponentConfig, error) { - gvkmap, err := kubeadmutil.SplitYAMLDocuments([]byte(in)) + gvkmap, err := kubeadmutil.SplitConfigDocuments([]byte(in)) if err != nil { - t.Fatalf("unexpected failure of SplitYAMLDocuments: %v", err) + t.Fatalf("unexpected failure of SplitConfigDocuments: %v", err) } return clusterConfigHandler.FromDocumentMap(gvkmap) diff --git a/cmd/kubeadm/app/phases/kubelet/config.go b/cmd/kubeadm/app/phases/kubelet/config.go index 5005d443bbf..df835639526 100644 --- a/cmd/kubeadm/app/phases/kubelet/config.go +++ b/cmd/kubeadm/app/phases/kubelet/config.go @@ -110,7 +110,7 @@ func ApplyPatchesToConfig(cfg *kubeadmapi.ClusterConfiguration, patchesDir strin } } - gvkmap, err := kubeadmutil.SplitYAMLDocuments(kubeletBytes) + gvkmap, err := kubeadmutil.SplitConfigDocuments(kubeletBytes) if err != nil { return err } diff --git a/cmd/kubeadm/app/util/config/common.go b/cmd/kubeadm/app/util/config/common.go index 7edadd2d2fd..353bef93751 100644 --- a/cmd/kubeadm/app/util/config/common.go +++ b/cmd/kubeadm/app/util/config/common.go @@ -256,7 +256,7 @@ func MigrateOldConfig(oldConfig []byte, allowExperimental bool, mutators migrate mutators = defaultMigrateMutators() } - gvkmap, err := kubeadmutil.SplitYAMLDocuments(oldConfig) + gvkmap, err := kubeadmutil.SplitConfigDocuments(oldConfig) if err != nil { return []byte{}, err } @@ -329,7 +329,7 @@ func MigrateOldConfig(oldConfig []byte, allowExperimental bool, mutators migrate // ValidateConfig takes a byte slice containing a kubeadm configuration and performs conversion // to internal types and validation. func ValidateConfig(config []byte, allowExperimental bool) error { - gvkmap, err := kubeadmutil.SplitYAMLDocuments(config) + gvkmap, err := kubeadmutil.SplitConfigDocuments(config) if err != nil { return err } diff --git a/cmd/kubeadm/app/util/config/initconfiguration.go b/cmd/kubeadm/app/util/config/initconfiguration.go index 203d219c1f9..b04ecafde1c 100644 --- a/cmd/kubeadm/app/util/config/initconfiguration.go +++ b/cmd/kubeadm/app/util/config/initconfiguration.go @@ -295,7 +295,7 @@ func LoadOrDefaultInitConfiguration(cfgPath string, versionedInitCfg *kubeadmapi // and well-known ComponentConfig GroupVersionKinds are stored inside of the internal InitConfiguration struct. // The resulting InitConfiguration is then dynamically defaulted and validated prior to return. func BytesToInitConfiguration(b []byte, skipCRIDetect bool) (*kubeadmapi.InitConfiguration, error) { - gvkmap, err := kubeadmutil.SplitYAMLDocuments(b) + gvkmap, err := kubeadmutil.SplitConfigDocuments(b) if err != nil { return nil, err } diff --git a/cmd/kubeadm/app/util/config/joinconfiguration.go b/cmd/kubeadm/app/util/config/joinconfiguration.go index 08540d568a0..f1676133879 100644 --- a/cmd/kubeadm/app/util/config/joinconfiguration.go +++ b/cmd/kubeadm/app/util/config/joinconfiguration.go @@ -87,7 +87,7 @@ func LoadJoinConfigurationFromFile(cfgPath string, opts LoadOrDefaultConfigurati return nil, errors.Wrapf(err, "unable to read config from %q ", cfgPath) } - gvkmap, err := kubeadmutil.SplitYAMLDocuments(b) + gvkmap, err := kubeadmutil.SplitConfigDocuments(b) if err != nil { return nil, err } diff --git a/cmd/kubeadm/app/util/config/resetconfiguration.go b/cmd/kubeadm/app/util/config/resetconfiguration.go index 2cc02e6fff7..769c899b37c 100644 --- a/cmd/kubeadm/app/util/config/resetconfiguration.go +++ b/cmd/kubeadm/app/util/config/resetconfiguration.go @@ -91,7 +91,7 @@ func LoadResetConfigurationFromFile(cfgPath string, opts LoadOrDefaultConfigurat return nil, errors.Wrapf(err, "unable to read config from %q ", cfgPath) } - gvkmap, err := kubeadmutil.SplitYAMLDocuments(b) + gvkmap, err := kubeadmutil.SplitConfigDocuments(b) if err != nil { return nil, err } diff --git a/cmd/kubeadm/app/util/config/upgradeconfiguration.go b/cmd/kubeadm/app/util/config/upgradeconfiguration.go index 19488422d4a..855da37ab79 100644 --- a/cmd/kubeadm/app/util/config/upgradeconfiguration.go +++ b/cmd/kubeadm/app/util/config/upgradeconfiguration.go @@ -94,7 +94,7 @@ func LoadUpgradeConfigurationFromFile(cfgPath string, _ LoadOrDefaultConfigurati } // Split the YAML/JSON documents in the file into a DocumentMap - docmap, err := kubeadmutil.SplitYAMLDocuments(configBytes) + docmap, err := kubeadmutil.SplitConfigDocuments(configBytes) if err != nil { return nil, err } diff --git a/cmd/kubeadm/app/util/config/upgradeconfiguration_test.go b/cmd/kubeadm/app/util/config/upgradeconfiguration_test.go index 8143cbd47be..f682ee32582 100644 --- a/cmd/kubeadm/app/util/config/upgradeconfiguration_test.go +++ b/cmd/kubeadm/app/util/config/upgradeconfiguration_test.go @@ -122,9 +122,9 @@ func TestDocMapToUpgradeConfiguration(t *testing.T) { if err != nil { t.Fatalf("unexpected error while marshalling to YAML: %v", err) } - docmap, err := kubeadmutil.SplitYAMLDocuments(b) + docmap, err := kubeadmutil.SplitConfigDocuments(b) if err != nil { - t.Fatalf("Unexpected error of SplitYAMLDocuments: %v", err) + t.Fatalf("Unexpected error of SplitConfigDocuments: %v", err) } cfg, err := DocMapToUpgradeConfiguration(docmap) if (err != nil) != tc.expectedError { diff --git a/cmd/kubeadm/app/util/marshal.go b/cmd/kubeadm/app/util/marshal.go index c82e3022016..68c42f3fd7c 100644 --- a/cmd/kubeadm/app/util/marshal.go +++ b/cmd/kubeadm/app/util/marshal.go @@ -65,13 +65,13 @@ func UniversalUnmarshal(buffer []byte) (runtime.Object, error) { return obj, nil } -// SplitYAMLDocuments reads the YAML bytes per-document, unmarshals the TypeMeta information from each document +// SplitConfigDocuments reads the YAML/JSON bytes per-document, unmarshals the TypeMeta information from each document // and returns a map between the GroupVersionKind of the document and the document bytes -func SplitYAMLDocuments(yamlBytes []byte) (kubeadmapi.DocumentMap, error) { +func SplitConfigDocuments(documentBytes []byte) (kubeadmapi.DocumentMap, error) { gvkmap := kubeadmapi.DocumentMap{} knownKinds := map[string]bool{} errs := []error{} - buf := bytes.NewBuffer(yamlBytes) + buf := bytes.NewBuffer(documentBytes) reader := utilyaml.NewYAMLReader(bufio.NewReader(buf)) for { // Read one YAML document at a time, until io.EOF is returned @@ -111,7 +111,7 @@ func SplitYAMLDocuments(yamlBytes []byte) (kubeadmapi.DocumentMap, error) { // GroupVersionKindsFromBytes parses the bytes and returns a gvk slice func GroupVersionKindsFromBytes(b []byte) ([]schema.GroupVersionKind, error) { - gvkmap, err := SplitYAMLDocuments(b) + gvkmap, err := SplitConfigDocuments(b) if err != nil { return nil, err } diff --git a/cmd/kubeadm/app/util/marshal_test.go b/cmd/kubeadm/app/util/marshal_test.go index 4814fc6292d..246d22ba482 100644 --- a/cmd/kubeadm/app/util/marshal_test.go +++ b/cmd/kubeadm/app/util/marshal_test.go @@ -199,7 +199,7 @@ func TestSplitYAMLDocuments(t *testing.T) { for _, rt := range tests { t.Run(rt.name, func(t2 *testing.T) { - gvkmap, err := SplitYAMLDocuments(rt.fileContents) + gvkmap, err := SplitConfigDocuments(rt.fileContents) if (err != nil) != rt.expectedErr { t2.Errorf("expected error: %t, actual: %t", rt.expectedErr, err != nil) } From a94403e942908bb3cbfa8d84ce6432e50804e34e Mon Sep 17 00:00:00 2001 From: HirazawaUi <695097494plus@gmail.com> Date: Mon, 3 Mar 2025 21:08:02 +0800 Subject: [PATCH 4/5] add BytesToXConfiguration function --- .../app/util/config/initconfiguration.go | 3 ++- .../app/util/config/joinconfiguration.go | 8 +++++++ .../app/util/config/resetconfiguration.go | 8 +++++++ .../app/util/config/upgradeconfiguration.go | 21 ++++++++++++------- 4 files changed, 32 insertions(+), 8 deletions(-) diff --git a/cmd/kubeadm/app/util/config/initconfiguration.go b/cmd/kubeadm/app/util/config/initconfiguration.go index b04ecafde1c..6b15b6bc5bb 100644 --- a/cmd/kubeadm/app/util/config/initconfiguration.go +++ b/cmd/kubeadm/app/util/config/initconfiguration.go @@ -291,10 +291,11 @@ func LoadOrDefaultInitConfiguration(cfgPath string, versionedInitCfg *kubeadmapi } // BytesToInitConfiguration converts a byte slice to an internal, defaulted and validated InitConfiguration object. -// The map may contain many different YAML/JSON documents. These YAML/JSON documents are parsed one-by-one +// The map may contain many different YAML/JSON documents. These documents are parsed one-by-one // and well-known ComponentConfig GroupVersionKinds are stored inside of the internal InitConfiguration struct. // The resulting InitConfiguration is then dynamically defaulted and validated prior to return. func BytesToInitConfiguration(b []byte, skipCRIDetect bool) (*kubeadmapi.InitConfiguration, error) { + // Split the YAML/JSON documents in the file into a DocumentMap gvkmap, err := kubeadmutil.SplitConfigDocuments(b) if err != nil { return nil, err diff --git a/cmd/kubeadm/app/util/config/joinconfiguration.go b/cmd/kubeadm/app/util/config/joinconfiguration.go index f1676133879..19af901ceb8 100644 --- a/cmd/kubeadm/app/util/config/joinconfiguration.go +++ b/cmd/kubeadm/app/util/config/joinconfiguration.go @@ -87,6 +87,14 @@ func LoadJoinConfigurationFromFile(cfgPath string, opts LoadOrDefaultConfigurati return nil, errors.Wrapf(err, "unable to read config from %q ", cfgPath) } + return BytesToJoinConfiguration(b, opts) +} + +// BytesToJoinConfiguration converts a byte slice to an internal, defaulted and validated JoinConfiguration object. +// The map may contain many different YAML/JSON documents. These documents are parsed one-by-one. +// The resulting JoinConfiguration is then dynamically defaulted and validated prior to return. +func BytesToJoinConfiguration(b []byte, opts LoadOrDefaultConfigurationOptions) (*kubeadmapi.JoinConfiguration, error) { + // Split the YAML/JSON documents in the file into a DocumentMap gvkmap, err := kubeadmutil.SplitConfigDocuments(b) if err != nil { return nil, err diff --git a/cmd/kubeadm/app/util/config/resetconfiguration.go b/cmd/kubeadm/app/util/config/resetconfiguration.go index 769c899b37c..ae9b57c82eb 100644 --- a/cmd/kubeadm/app/util/config/resetconfiguration.go +++ b/cmd/kubeadm/app/util/config/resetconfiguration.go @@ -91,6 +91,14 @@ func LoadResetConfigurationFromFile(cfgPath string, opts LoadOrDefaultConfigurat return nil, errors.Wrapf(err, "unable to read config from %q ", cfgPath) } + return BytesToResetConfiguration(b, opts) +} + +// BytesToResetConfiguration converts a byte slice to an internal, defaulted and validated ResetConfiguration object. +// The map may contain many different YAML/JSON documents. These documents are parsed one-by-one. +// The resulting ResetConfiguration is then dynamically defaulted and validated prior to return. +func BytesToResetConfiguration(b []byte, opts LoadOrDefaultConfigurationOptions) (*kubeadmapi.ResetConfiguration, error) { + // Split the YAML/JSON documents in the file into a DocumentMap gvkmap, err := kubeadmutil.SplitConfigDocuments(b) if err != nil { return nil, err diff --git a/cmd/kubeadm/app/util/config/upgradeconfiguration.go b/cmd/kubeadm/app/util/config/upgradeconfiguration.go index 855da37ab79..200818da075 100644 --- a/cmd/kubeadm/app/util/config/upgradeconfiguration.go +++ b/cmd/kubeadm/app/util/config/upgradeconfiguration.go @@ -93,22 +93,29 @@ func LoadUpgradeConfigurationFromFile(cfgPath string, _ LoadOrDefaultConfigurati return nil, errors.Wrapf(err, "unable to load config from file %q", cfgPath) } - // Split the YAML/JSON documents in the file into a DocumentMap - docmap, err := kubeadmutil.SplitConfigDocuments(configBytes) - if err != nil { - return nil, err - } - // Convert documentMap to internal UpgradeConfiguration, InitConfiguration and ClusterConfiguration from config file will be ignored. // Upgrade should respect the cluster configuration from the existing cluster, re-configure the cluster with a InitConfiguration and // ClusterConfiguration from the config file is not allowed for upgrade. - if upgradeCfg, err = DocMapToUpgradeConfiguration(docmap); err != nil { + if upgradeCfg, err = BytesToUpgradeConfiguration(configBytes); err != nil { return nil, err } return upgradeCfg, nil } +// BytesToUpgradeConfiguration converts a byte slice to an internal, defaulted and validated UpgradeConfiguration object. +// The map may contain many different YAML/JSON documents. These documents are parsed one-by-one. +// The resulting UpgradeConfiguration is then dynamically defaulted and validated prior to return. +func BytesToUpgradeConfiguration(b []byte) (*kubeadmapi.UpgradeConfiguration, error) { + // Split the YAML/JSON documents in the file into a DocumentMap + gvkmap, err := kubeadmutil.SplitConfigDocuments(b) + if err != nil { + return nil, err + } + + return documentMapToUpgradeConfiguration(gvkmap, false) +} + // LoadOrDefaultUpgradeConfiguration takes a path to a config file and a versioned configuration that can serve as the default config // If cfgPath is specified, defaultversionedcfg will always get overridden. Otherwise, the default config (often populated by flags) will be used. // Then the external, versioned configuration is defaulted and converted to the internal type. 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 5/5] 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) + } + }) + } } }