Code Refactor: using struct option rather than a long list of parameters

Co-authored-by: Shida Qiu <shidaqiu2018@gmail.com>
This commit is contained in:
carlory 2023-10-11 14:51:03 +08:00
parent bdcb73d6b3
commit db8e106e3f
16 changed files with 84 additions and 35 deletions

View File

@ -136,7 +136,9 @@ func (o *genCSRConfig) load() (err error) {
o.kubeadmConfigPath,
&kubeadmapiv1.InitConfiguration{},
&kubeadmapiv1.ClusterConfiguration{},
true, /* skipCRIDetect */
configutil.LoadOrDefaultConfigurationOptions{
SkipCRIDetect: true,
},
)
if err != nil {
return err
@ -354,7 +356,9 @@ func getInternalCfg(cfgPath string, kubeconfigPath string, cfg kubeadmapiv1.Clus
}
// Read config from --config if provided. Otherwise, use the default configuration
return configutil.LoadOrDefaultInitConfiguration(cfgPath, &kubeadmapiv1.InitConfiguration{}, &cfg, true /* skipCRIDetect */)
return configutil.LoadOrDefaultInitConfiguration(cfgPath, &kubeadmapiv1.InitConfiguration{}, &cfg, configutil.LoadOrDefaultConfigurationOptions{
SkipCRIDetect: true,
})
}
// newCmdCertsExpiration creates a new `cert check-expiration` command.

View File

@ -212,6 +212,9 @@ func getDefaultInitConfigBytes() ([]byte, error) {
}
func getDefaultNodeConfigBytes() ([]byte, error) {
opts := configutil.LoadOrDefaultConfigurationOptions{
SkipCRIDetect: true,
}
internalcfg, err := configutil.DefaultedJoinConfiguration(&kubeadmapiv1old.JoinConfiguration{
Discovery: kubeadmapiv1old.Discovery{
BootstrapToken: &kubeadmapiv1old.BootstrapTokenDiscovery{
@ -223,7 +226,7 @@ func getDefaultNodeConfigBytes() ([]byte, error) {
NodeRegistration: kubeadmapiv1old.NodeRegistrationOptions{
CRISocket: constants.DefaultCRISocket, // avoid CRI detection
},
}, true /* skipCRIDetect */)
}, opts)
if err != nil {
return []byte{}, err
}
@ -232,9 +235,12 @@ func getDefaultNodeConfigBytes() ([]byte, error) {
}
func getDefaultResetConfigBytes() ([]byte, error) {
opts := configutil.LoadOrDefaultConfigurationOptions{
SkipCRIDetect: true,
}
internalcfg, err := configutil.DefaultedResetConfiguration(&kubeadmapiv1.ResetConfiguration{
CRISocket: constants.DefaultCRISocket, // avoid CRI detection
}, true /* skipCRIDetect */)
}, opts)
if err != nil {
return []byte{}, err
}
@ -367,7 +373,7 @@ func newCmdConfigImagesPull() *cobra.Command {
if err != nil {
return err
}
internalcfg, err := configutil.LoadOrDefaultInitConfiguration(cfgPath, externalInitCfg, externalClusterCfg, false)
internalcfg, err := configutil.LoadOrDefaultInitConfiguration(cfgPath, externalInitCfg, externalClusterCfg, configutil.LoadOrDefaultConfigurationOptions{})
if err != nil {
return err
}
@ -442,7 +448,9 @@ func newCmdConfigImagesList(out io.Writer, mockK8sVersion *string) *cobra.Comman
// NewImagesList returns the underlying struct for the "kubeadm config images list" command
func NewImagesList(cfgPath string, cfg *kubeadmapiv1old.ClusterConfiguration) (*ImagesList, error) {
initcfg, err := configutil.LoadOrDefaultInitConfiguration(cfgPath, &kubeadmapiv1old.InitConfiguration{}, cfg, true /* skipCRIDetect */)
initcfg, err := configutil.LoadOrDefaultInitConfiguration(cfgPath, &kubeadmapiv1old.InitConfiguration{}, cfg, configutil.LoadOrDefaultConfigurationOptions{
SkipCRIDetect: true,
})
if err != nil {
return nil, errors.Wrap(err, "could not convert cfg to an internal cfg")
}

View File

@ -302,7 +302,9 @@ func newInitData(cmd *cobra.Command, args []string, initOptions *initOptions, ou
// Either use the config file if specified, or convert public kubeadm API to the internal InitConfiguration
// and validates InitConfiguration
cfg, err := configutil.LoadOrDefaultInitConfiguration(initOptions.cfgPath, initOptions.externalInitCfg, initOptions.externalClusterCfg, initOptions.skipCRIDetect)
cfg, err := configutil.LoadOrDefaultInitConfiguration(initOptions.cfgPath, initOptions.externalInitCfg, initOptions.externalClusterCfg, configutil.LoadOrDefaultConfigurationOptions{
SkipCRIDetect: initOptions.skipCRIDetect,
})
if err != nil {
return nil, err
}

View File

@ -427,7 +427,9 @@ func newJoinData(cmd *cobra.Command, args []string, opt *joinOptions, out io.Wri
opt.externalcfg.Discovery.BootstrapToken = nil //NB. this could be removed when we get better control on args (e.g. phases without discovery should have NoArgs )
}
cfg, err := configutil.LoadOrDefaultJoinConfiguration(opt.cfgPath, opt.externalcfg, opt.skipCRIDetect)
cfg, err := configutil.LoadOrDefaultJoinConfiguration(opt.cfgPath, opt.externalcfg, configutil.LoadOrDefaultConfigurationOptions{
SkipCRIDetect: opt.skipCRIDetect,
})
if err != nil {
return nil, err
}

View File

@ -82,7 +82,9 @@ func newCmdUserKubeConfig(out io.Writer) *cobra.Command {
Example: userKubeconfigExample,
RunE: func(cmd *cobra.Command, args []string) error {
// This call returns the ready-to-use configuration based on the defaults populated by flags
internalCfg, err := configutil.LoadOrDefaultInitConfiguration(cfgPath, initCfg, clusterCfg, true /* skipCRIDetect */)
internalCfg, err := configutil.LoadOrDefaultInitConfiguration(cfgPath, initCfg, clusterCfg, configutil.LoadOrDefaultConfigurationOptions{
SkipCRIDetect: true,
})
if err != nil {
return err
}

View File

@ -108,7 +108,10 @@ func newResetData(cmd *cobra.Command, opts *resetOptions, in io.Reader, out io.W
var initCfg *kubeadmapi.InitConfiguration
// Either use the config file if specified, or convert public kubeadm API to the internal ResetConfiguration and validates cfg.
resetCfg, err := configutil.LoadOrDefaultResetConfiguration(opts.cfgPath, opts.externalcfg, allowExperimental, opts.skipCRIDetect)
resetCfg, err := configutil.LoadOrDefaultResetConfiguration(opts.cfgPath, opts.externalcfg, configutil.LoadOrDefaultConfigurationOptions{
AllowExperimental: allowExperimental,
SkipCRIDetect: opts.skipCRIDetect,
})
if err != nil {
return nil, err
}

View File

@ -242,7 +242,9 @@ func RunCreateToken(out io.Writer, client clientset.Interface, cfgPath string, i
// This call returns the ready-to-use configuration based on the configuration file that might or might not exist and the default cfg populated by flags
klog.V(1).Infoln("[token] loading configurations")
internalcfg, err := configutil.LoadOrDefaultInitConfiguration(cfgPath, initCfg, clusterCfg, true /* skipCRIDetect */)
internalcfg, err := configutil.LoadOrDefaultInitConfiguration(cfgPath, initCfg, clusterCfg, configutil.LoadOrDefaultConfigurationOptions{
SkipCRIDetect: true,
})
if err != nil {
return err
}

View File

@ -111,7 +111,7 @@ func runDiff(flags *diffFlags, args []string) error {
var err error
var cfg *kubeadmapi.InitConfiguration
if flags.cfgPath != "" {
cfg, err = configutil.LoadInitConfigurationFromFile(flags.cfgPath, false)
cfg, err = configutil.LoadInitConfigurationFromFile(flags.cfgPath, configutil.LoadOrDefaultConfigurationOptions{})
} else {
var client *client.Clientset
client, err = kubeconfigutil.ClientSetFromFile(flags.kubeConfigPath)

View File

@ -1071,6 +1071,11 @@ func TestJoinIPCheck(t *testing.T) {
if _, err := isPrivileged.Check(); err != nil {
t.Skip("not a privileged user")
}
opts := configutil.LoadOrDefaultConfigurationOptions{
SkipCRIDetect: true,
}
internalcfg, err := configutil.DefaultedJoinConfiguration(&kubeadmapiv1.JoinConfiguration{
Discovery: kubeadmapiv1.Discovery{
BootstrapToken: &kubeadmapiv1.BootstrapTokenDiscovery{
@ -1079,7 +1084,7 @@ func TestJoinIPCheck(t *testing.T) {
UnsafeSkipCAVerification: true,
},
},
}, true)
}, opts)
if err != nil {
t.Fatalf("unexpected failure when defaulting JoinConfiguration: %v", err)
}

View File

@ -44,6 +44,14 @@ import (
kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
)
// LoadOrDefaultConfigurationOptions holds the common LoadOrDefaultConfiguration options.
type LoadOrDefaultConfigurationOptions struct {
// AllowExperimental indicates whether the experimental / work in progress APIs can be used.
AllowExperimental bool
// SkipCRIDetect indicates whether to skip the CRI socket detection when no CRI socket is provided.
SkipCRIDetect bool
}
// MarshalKubeadmConfigObject marshals an Object registered in the kubeadm scheme. If the object is a InitConfiguration or ClusterConfiguration, some extra logic is run
func MarshalKubeadmConfigObject(obj runtime.Object, gv schema.GroupVersion) ([]byte, error) {
switch internalcfg := obj.(type) {

View File

@ -256,7 +256,7 @@ func DefaultedInitConfiguration(versionedInitCfg *kubeadmapiv1.InitConfiguration
}
// LoadInitConfigurationFromFile loads a supported versioned InitConfiguration from a file, converts it into internal config, defaults it and verifies it.
func LoadInitConfigurationFromFile(cfgPath string, skipCRIDetect bool) (*kubeadmapi.InitConfiguration, error) {
func LoadInitConfigurationFromFile(cfgPath string, opts LoadOrDefaultConfigurationOptions) (*kubeadmapi.InitConfiguration, error) {
klog.V(1).Infof("loading configuration from %q", cfgPath)
b, err := os.ReadFile(cfgPath)
@ -264,7 +264,7 @@ func LoadInitConfigurationFromFile(cfgPath string, skipCRIDetect bool) (*kubeadm
return nil, errors.Wrapf(err, "unable to read config from %q ", cfgPath)
}
return BytesToInitConfiguration(b, skipCRIDetect)
return BytesToInitConfiguration(b, opts.SkipCRIDetect)
}
// LoadOrDefaultInitConfiguration takes a path to a config file and a versioned configuration that can serve as the default config
@ -272,14 +272,14 @@ func LoadInitConfigurationFromFile(cfgPath string, skipCRIDetect bool) (*kubeadm
// The external, versioned configuration is defaulted and converted to the internal type.
// Right thereafter, the configuration is defaulted again with dynamic values (like IP addresses of a machine, etc)
// Lastly, the internal config is validated and returned.
func LoadOrDefaultInitConfiguration(cfgPath string, versionedInitCfg *kubeadmapiv1.InitConfiguration, versionedClusterCfg *kubeadmapiv1.ClusterConfiguration, skipCRIDetect bool) (*kubeadmapi.InitConfiguration, error) {
func LoadOrDefaultInitConfiguration(cfgPath string, versionedInitCfg *kubeadmapiv1.InitConfiguration, versionedClusterCfg *kubeadmapiv1.ClusterConfiguration, opts LoadOrDefaultConfigurationOptions) (*kubeadmapi.InitConfiguration, error) {
if cfgPath != "" {
// Loads configuration from config file, if provided
// Nb. --config overrides command line flags
return LoadInitConfigurationFromFile(cfgPath, skipCRIDetect)
return LoadInitConfigurationFromFile(cfgPath, opts)
}
return DefaultedInitConfiguration(versionedInitCfg, versionedClusterCfg, skipCRIDetect)
return DefaultedInitConfiguration(versionedInitCfg, versionedClusterCfg, opts.SkipCRIDetect)
}
// BytesToInitConfiguration converts a byte slice to an internal, defaulted and validated InitConfiguration object.

View File

@ -90,7 +90,11 @@ kubernetesVersion: %s`, kubeadmapiv1.SchemeGroupVersion.String(), certDir, const
return
}
obj, err := LoadInitConfigurationFromFile(cfgPath, true)
opts := LoadOrDefaultConfigurationOptions{
SkipCRIDetect: true,
}
obj, err := LoadInitConfigurationFromFile(cfgPath, opts)
if rt.expectErr {
if err == nil {
t.Error("Unexpected success")

View File

@ -61,18 +61,18 @@ func SetJoinControlPlaneDefaults(cfg *kubeadmapi.JoinControlPlane) error {
// Then the external, versioned configuration is defaulted and converted to the internal type.
// Right thereafter, the configuration is defaulted again with dynamic values (like IP addresses of a machine, etc)
// Lastly, the internal config is validated and returned.
func LoadOrDefaultJoinConfiguration(cfgPath string, defaultversionedcfg *kubeadmapiv1.JoinConfiguration, skipCRIDetect bool) (*kubeadmapi.JoinConfiguration, error) {
func LoadOrDefaultJoinConfiguration(cfgPath string, defaultversionedcfg *kubeadmapiv1.JoinConfiguration, opts LoadOrDefaultConfigurationOptions) (*kubeadmapi.JoinConfiguration, error) {
if cfgPath != "" {
// Loads configuration from config file, if provided
// Nb. --config overrides command line flags, TODO: fix this
return LoadJoinConfigurationFromFile(cfgPath, skipCRIDetect)
return LoadJoinConfigurationFromFile(cfgPath, opts)
}
return DefaultedJoinConfiguration(defaultversionedcfg, skipCRIDetect)
return DefaultedJoinConfiguration(defaultversionedcfg, opts)
}
// LoadJoinConfigurationFromFile loads versioned JoinConfiguration from file, converts it to internal, defaults and validates it
func LoadJoinConfigurationFromFile(cfgPath string, skipCRIDetect bool) (*kubeadmapi.JoinConfiguration, error) {
func LoadJoinConfigurationFromFile(cfgPath string, opts LoadOrDefaultConfigurationOptions) (*kubeadmapi.JoinConfiguration, error) {
klog.V(1).Infof("loading configuration from %q", cfgPath)
b, err := os.ReadFile(cfgPath)
@ -85,7 +85,7 @@ func LoadJoinConfigurationFromFile(cfgPath string, skipCRIDetect bool) (*kubeadm
return nil, err
}
return documentMapToJoinConfiguration(gvkmap, false, false, false, skipCRIDetect)
return documentMapToJoinConfiguration(gvkmap, false, false, false, opts.SkipCRIDetect)
}
// documentMapToJoinConfiguration takes a map between GVKs and YAML documents (as returned by SplitYAMLDocuments),
@ -137,7 +137,7 @@ func documentMapToJoinConfiguration(gvkmap kubeadmapi.DocumentMap, allowDeprecat
}
// DefaultedJoinConfiguration takes a versioned JoinConfiguration (usually filled in by command line parameters), defaults it, converts it to internal and validates it
func DefaultedJoinConfiguration(defaultversionedcfg *kubeadmapiv1.JoinConfiguration, skipCRIDetect bool) (*kubeadmapi.JoinConfiguration, error) {
func DefaultedJoinConfiguration(defaultversionedcfg *kubeadmapiv1.JoinConfiguration, opts LoadOrDefaultConfigurationOptions) (*kubeadmapi.JoinConfiguration, error) {
internalcfg := &kubeadmapi.JoinConfiguration{}
// Takes passed flags into account; the defaulting is executed once again enforcing assignment of
@ -148,7 +148,7 @@ func DefaultedJoinConfiguration(defaultversionedcfg *kubeadmapiv1.JoinConfigurat
}
// Applies dynamic defaults to settings not provided with flags
if err := SetJoinDynamicDefaults(internalcfg, skipCRIDetect); err != nil {
if err := SetJoinDynamicDefaults(internalcfg, opts.SkipCRIDetect); err != nil {
return nil, err
}
// Validates cfg (flags/configs + defaults)

View File

@ -78,7 +78,11 @@ func TestLoadJoinConfigurationFromFile(t *testing.T) {
return
}
obj, err := LoadJoinConfigurationFromFile(cfgPath, true)
opts := LoadOrDefaultConfigurationOptions{
SkipCRIDetect: true,
}
obj, err := LoadJoinConfigurationFromFile(cfgPath, opts)
if rt.expectErr {
if err == nil {
t.Error("Unexpected success")

View File

@ -65,17 +65,17 @@ func SetResetDynamicDefaults(cfg *kubeadmapi.ResetConfiguration, skipCRIDetect b
// Then the external, versioned configuration is defaulted and converted to the internal type.
// Right thereafter, the configuration is defaulted again with dynamic values
// Lastly, the internal config is validated and returned.
func LoadOrDefaultResetConfiguration(cfgPath string, defaultversionedcfg *kubeadmapiv1.ResetConfiguration, allowExperimental, skipCRIDetect bool) (*kubeadmapi.ResetConfiguration, error) {
func LoadOrDefaultResetConfiguration(cfgPath string, defaultversionedcfg *kubeadmapiv1.ResetConfiguration, opts LoadOrDefaultConfigurationOptions) (*kubeadmapi.ResetConfiguration, error) {
if cfgPath != "" {
// Loads configuration from config file, if provided
return LoadResetConfigurationFromFile(cfgPath, allowExperimental, skipCRIDetect)
return LoadResetConfigurationFromFile(cfgPath, opts)
}
return DefaultedResetConfiguration(defaultversionedcfg, skipCRIDetect)
return DefaultedResetConfiguration(defaultversionedcfg, opts)
}
// LoadResetConfigurationFromFile loads versioned ResetConfiguration from file, converts it to internal, defaults and validates it
func LoadResetConfigurationFromFile(cfgPath string, allowExperimental, skipCRIDetect bool) (*kubeadmapi.ResetConfiguration, error) {
func LoadResetConfigurationFromFile(cfgPath string, opts LoadOrDefaultConfigurationOptions) (*kubeadmapi.ResetConfiguration, error) {
klog.V(1).Infof("loading configuration from %q", cfgPath)
b, err := os.ReadFile(cfgPath)
@ -88,7 +88,7 @@ func LoadResetConfigurationFromFile(cfgPath string, allowExperimental, skipCRIDe
return nil, err
}
return documentMapToResetConfiguration(gvkmap, false, allowExperimental, false, skipCRIDetect)
return documentMapToResetConfiguration(gvkmap, false, opts.AllowExperimental, false, opts.SkipCRIDetect)
}
// documentMapToResetConfiguration takes a map between GVKs and YAML documents (as returned by SplitYAMLDocuments),
@ -140,7 +140,7 @@ func documentMapToResetConfiguration(gvkmap kubeadmapi.DocumentMap, allowDepreca
}
// DefaultedResetConfiguration takes a versioned ResetConfiguration (usually filled in by command line parameters), defaults it, converts it to internal and validates it
func DefaultedResetConfiguration(defaultversionedcfg *kubeadmapiv1.ResetConfiguration, skipCRIDetect bool) (*kubeadmapi.ResetConfiguration, error) {
func DefaultedResetConfiguration(defaultversionedcfg *kubeadmapiv1.ResetConfiguration, opts LoadOrDefaultConfigurationOptions) (*kubeadmapi.ResetConfiguration, error) {
internalcfg := &kubeadmapi.ResetConfiguration{}
// Takes passed flags into account; the defaulting is executed once again enforcing assignment of
@ -151,7 +151,7 @@ func DefaultedResetConfiguration(defaultversionedcfg *kubeadmapiv1.ResetConfigur
}
// Applies dynamic defaults to settings not provided with flags
if err := SetResetDynamicDefaults(internalcfg, skipCRIDetect); err != nil {
if err := SetResetDynamicDefaults(internalcfg, opts.SkipCRIDetect); err != nil {
return nil, err
}
// Validates cfg

View File

@ -75,7 +75,12 @@ func TestLoadResetConfigurationFromFile(t *testing.T) {
return
}
obj, err := LoadResetConfigurationFromFile(cfgPath, true, true)
opts := LoadOrDefaultConfigurationOptions{
AllowExperimental: true,
SkipCRIDetect: true,
}
obj, err := LoadResetConfigurationFromFile(cfgPath, opts)
if rt.expectErr {
if err == nil {
t.Error("Unexpected success")