From 2b3d2303a505a13b819b8f5f5ce664ab34ffb733 Mon Sep 17 00:00:00 2001 From: cici37 Date: Thu, 14 Jan 2021 13:07:53 -0800 Subject: [PATCH 01/16] Fix flag passing in CCM. --- cmd/cloud-controller-manager/main.go | 93 ++++++-------- .../cloud-provider/app/controllermanager.go | 56 ++++++--- .../cloud-provider/app/testing/testserver.go | 2 +- .../src/k8s.io/cloud-provider/sample/BUILD | 0 .../k8s.io/cloud-provider/sample/README.md | 4 +- .../cloud-provider/sample/advanced_main.go | 115 ------------------ .../cloud-provider/sample/basic_main.go | 68 +++++------ 7 files changed, 111 insertions(+), 227 deletions(-) create mode 100644 staging/src/k8s.io/cloud-provider/sample/BUILD delete mode 100644 staging/src/k8s.io/cloud-provider/sample/advanced_main.go diff --git a/cmd/cloud-controller-manager/main.go b/cmd/cloud-controller-manager/main.go index 5629c22a092..9a1b9f6ab38 100644 --- a/cmd/cloud-controller-manager/main.go +++ b/cmd/cloud-controller-manager/main.go @@ -19,14 +19,12 @@ limitations under the License. // This file should be written by each cloud provider. // For an minimal working example, please refer to k8s.io/cloud-provider/sample/basic_main.go -// For an advanced example, please refer to k8s.io/cloud-provider/sample/advanced_main.go // For more details, please refer to k8s.io/kubernetes/cmd/cloud-controller-manager/main.go // The current file demonstrate how other cloud provider should leverage CCM and it uses fake parameters. Please modify for your own use. package main import ( - "fmt" "math/rand" "net/http" "os" @@ -57,47 +55,41 @@ const ( func main() { rand.Seed(time.Now().UnixNano()) - pflag.CommandLine.ParseErrorsWhitelist.UnknownFlags = true - _ = pflag.CommandLine.Parse(os.Args[1:]) - - // this is an example of allow-listing specific controller loops - controllerList := []string{"cloud-node", "cloud-node-lifecycle", "service", "route"} - - s, err := options.NewCloudControllerManagerOptions() + ccmOptions, err := options.NewCloudControllerManagerOptions() if err != nil { klog.Fatalf("unable to initialize command options: %v", err) } - c, err := s.Config(controllerList, app.ControllersDisabledByDefault.List()) - if err != nil { - fmt.Fprintf(os.Stderr, "%v\n", err) - os.Exit(1) - } - cloud, err := cloudprovider.InitCloudProvider(cloudProviderName, c.ComponentConfig.KubeCloudShared.CloudProvider.CloudConfigFile) - if err != nil { - klog.Fatalf("Cloud provider could not be initialized: %v", err) - } - if cloud == nil { - klog.Fatalf("cloud provider is nil") - } - - if !cloud.HasClusterID() { - if c.ComponentConfig.KubeCloudShared.AllowUntaggedCloud { - klog.Warning("detected a cluster without a ClusterID. A ClusterID will be required in the future. Please tag your cluster to avoid any future issues") - } else { - klog.Fatalf("no ClusterID found. A ClusterID is required for the cloud provider to function properly. This check can be bypassed by setting the allow-untagged-cloud option") + cloudInitializer := func(config *cloudcontrollerconfig.CompletedConfig) cloudprovider.Interface { + cloudConfigFile := config.ComponentConfig.KubeCloudShared.CloudProvider.CloudConfigFile + // initialize cloud provider with the cloud provider name and config file provided + cloud, err := cloudprovider.InitCloudProvider(cloudProviderName, cloudConfigFile) + if err != nil { + klog.Fatalf("Cloud provider could not be initialized: %v", err) } + if cloud == nil { + klog.Fatalf("Cloud provider is nil") + } + + if !cloud.HasClusterID() { + if config.ComponentConfig.KubeCloudShared.AllowUntaggedCloud { + klog.Warning("detected a cluster without a ClusterID. A ClusterID will be required in the future. Please tag your cluster to avoid any future issues") + } else { + klog.Fatalf("no ClusterID found. A ClusterID is required for the cloud provider to function properly. This check can be bypassed by setting the allow-untagged-cloud option") + } + } + + // Initialize the cloud provider with a reference to the clientBuilder + cloud.Initialize(config.ClientBuilder, make(chan struct{})) + // Set the informer on the user cloud object + if informerUserCloud, ok := cloud.(cloudprovider.InformerUser); ok { + informerUserCloud.SetInformers(config.SharedInformers) + } + + return cloud } - // Initialize the cloud provider with a reference to the clientBuilder - cloud.Initialize(c.ClientBuilder, make(chan struct{})) - // Set the informer on the user cloud object - if informerUserCloud, ok := cloud.(cloudprovider.InformerUser); ok { - informerUserCloud.SetInformers(c.SharedInformers) - } - - controllerInitializers := app.DefaultControllerInitializers(c.Complete(), cloud) - + controllerInitializers := app.DefaultInitFuncConstructors // Here is an example to remove the controller which is not needed. // e.g. remove the cloud-node-lifecycle controller which current cloud provider does not need. //delete(controllerInitializers, "cloud-node-lifecycle") @@ -105,16 +97,9 @@ func main() { // Here is an example to add an controller(NodeIpamController) which will be used by cloud provider // generate nodeIPAMConfig. Here is an sample code. Please pass the right parameter in your code. // If you do not need additional controller, please ignore. - nodeIPAMConfig := nodeipamconfig.NodeIPAMControllerConfiguration{ - ServiceCIDR: "sample", - SecondaryServiceCIDR: "sample", - NodeCIDRMaskSize: 11, - NodeCIDRMaskSizeIPv4: 11, - NodeCIDRMaskSizeIPv6: 111, - } - controllerInitializers["nodeipam"] = startNodeIpamControllerWrapper(c.Complete(), nodeIPAMConfig, cloud) + controllerInitializers["nodeipam"] = startNodeIpamControllerWrapper - command := app.NewCloudControllerManagerCommand(s, c, controllerInitializers) + command := app.NewCloudControllerManagerCommand(cloudProviderName, ccmOptions, cloudInitializer, controllerInitializers) // TODO: once we switch everything over to Cobra commands, we can go back to calling // utilflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the @@ -125,20 +110,20 @@ func main() { logs.InitLogs() defer logs.FlushLogs() - // the flags could be set before execute - command.Flags().VisitAll(func(flag *pflag.Flag) { - if flag.Name == "cloud-provider" { - flag.Value.Set("SampleCloudProviderFlagValue") - return - } - }) if err := command.Execute(); err != nil { os.Exit(1) } } -func startNodeIpamControllerWrapper(ccmconfig *cloudcontrollerconfig.CompletedConfig, nodeIPAMConfig nodeipamconfig.NodeIPAMControllerConfiguration, cloud cloudprovider.Interface) func(ctx genericcontrollermanager.ControllerContext) (http.Handler, bool, error) { +func startNodeIpamControllerWrapper(completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) app.InitFunc { + nodeIPAMConfig := nodeipamconfig.NodeIPAMControllerConfiguration{ + ServiceCIDR: "sample", + SecondaryServiceCIDR: "sample", + NodeCIDRMaskSize: 11, + NodeCIDRMaskSizeIPv4: 11, + NodeCIDRMaskSizeIPv6: 111, + } return func(ctx genericcontrollermanager.ControllerContext) (http.Handler, bool, error) { - return startNodeIpamController(ccmconfig, nodeIPAMConfig, ctx, cloud) + return startNodeIpamController(completedConfig, nodeIPAMConfig, ctx, cloud) } } diff --git a/staging/src/k8s.io/cloud-provider/app/controllermanager.go b/staging/src/k8s.io/cloud-provider/app/controllermanager.go index 7c7e9a90061..f5db9bb2e45 100644 --- a/staging/src/k8s.io/cloud-provider/app/controllermanager.go +++ b/staging/src/k8s.io/cloud-provider/app/controllermanager.go @@ -64,7 +64,7 @@ const ( ) // NewCloudControllerManagerCommand creates a *cobra.Command object with default parameters -func NewCloudControllerManagerCommand(s *options.CloudControllerManagerOptions, c *cloudcontrollerconfig.Config, controllerInitializers map[string]InitFunc) *cobra.Command { +func NewCloudControllerManagerCommand(cloudProviderName string, s *options.CloudControllerManagerOptions, cloudInitializer InitCloudFunc, initFuncConstructor map[string]InitFuncConstructor) *cobra.Command { cmd := &cobra.Command{ Use: "cloud-controller-manager", @@ -72,13 +72,27 @@ func NewCloudControllerManagerCommand(s *options.CloudControllerManagerOptions, the cloud specific control loops shipped with Kubernetes.`, Run: func(cmd *cobra.Command, args []string) { verflag.PrintAndExitIfRequested() + + cloudProviderFlag := cmd.Flags().Lookup("cloud-provider") + if cloudProviderFlag.Value.String() == "" { + cloudProviderFlag.Value.Set(cloudProviderName) + } cliflag.PrintFlags(cmd.Flags()) - if err := Run(c.Complete(), controllerInitializers, wait.NeverStop); err != nil { + c, err := s.Config(ControllerNames(initFuncConstructor), ControllersDisabledByDefault.List()) + if err != nil { fmt.Fprintf(os.Stderr, "%v\n", err) os.Exit(1) } + completedConfig := c.Complete() + cloud := cloudInitializer(completedConfig) + controllerInitializers := ConstructControllerInitializers(initFuncConstructor, completedConfig, cloud) + + if err := Run(completedConfig, controllerInitializers, wait.NeverStop); err != nil { + fmt.Fprintf(os.Stderr, "%v\n", err) + os.Exit(1) + } }, Args: func(cmd *cobra.Command, args []string) error { for _, arg := range args { @@ -91,7 +105,7 @@ the cloud specific control loops shipped with Kubernetes.`, } fs := cmd.Flags() - namedFlagSets := s.Flags(KnownControllers(controllerInitializers), ControllersDisabledByDefault.List()) + namedFlagSets := s.Flags(ControllerNames(initFuncConstructor), ControllersDisabledByDefault.List()) verflag.AddFlags(namedFlagSets.FlagSet("global")) globalflag.AddGlobalFlags(namedFlagSets.FlagSet("global"), cmd.Name()) @@ -121,6 +135,8 @@ the cloud specific control loops shipped with Kubernetes.`, return cmd } +type InitCloudFunc func(config *cloudcontrollerconfig.CompletedConfig) cloudprovider.Interface + // Run runs the ExternalCMServer. This should never exit. func Run(c *cloudcontrollerconfig.CompletedConfig, controllerInitializers map[string]InitFunc, stopCh <-chan struct{}) error { // To help debugging, immediately log version @@ -256,54 +272,62 @@ func startControllers(ctx genericcontrollermanager.ControllerContext, c *cloudco // The bool indicates whether the controller was enabled. type InitFunc func(ctx genericcontrollermanager.ControllerContext) (debuggingHandler http.Handler, enabled bool, err error) -// KnownControllers indicate the default controller we are known. -func KnownControllers(controllerInitializers map[string]InitFunc) []string { - ret := sets.StringKeySet(controllerInitializers) +type InitFuncConstructor func(completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) InitFunc + +// ControllerNames indicate the default controller we are known. +func ControllerNames(initFuncConstructors map[string]InitFuncConstructor) []string { + ret := sets.StringKeySet(initFuncConstructors) return ret.List() } // ControllersDisabledByDefault is the controller disabled default when starting cloud-controller managers. var ControllersDisabledByDefault = sets.NewString() -// DefaultControllerInitializers is a private map of named controller groups (you can start more than one in an init func) +// ConstructControllerInitializers is a private map of named controller groups (you can start more than one in an init func) // paired to their InitFunc. This allows for structured downstream composition and subdivision. -func DefaultControllerInitializers(completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) map[string]InitFunc { +func ConstructControllerInitializers(initFuncConstructors map[string]InitFuncConstructor, completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) map[string]InitFunc { controllers := map[string]InitFunc{} - controllers["cloud-node"] = StartCloudNodeControllerWrapper(completedConfig, cloud) - controllers["cloud-node-lifecycle"] = startCloudNodeLifecycleControllerWrapper(completedConfig, cloud) - controllers["service"] = startServiceControllerWrapper(completedConfig, cloud) - controllers["route"] = startRouteControllerWrapper(completedConfig, cloud) + for name, constructor := range initFuncConstructors { + controllers[name] = constructor(completedConfig, cloud) + } return controllers } // StartCloudNodeControllerWrapper is used to take cloud cofig as input and start cloud node controller -func StartCloudNodeControllerWrapper(completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) func(ctx genericcontrollermanager.ControllerContext) (http.Handler, bool, error) { +func StartCloudNodeControllerWrapper(completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) InitFunc { return func(ctx genericcontrollermanager.ControllerContext) (http.Handler, bool, error) { return startCloudNodeController(completedConfig, cloud, ctx.Stop) } } // startCloudNodeLifecycleControllerWrapper is used to take cloud cofig as input and start cloud node lifecycle controller -func startCloudNodeLifecycleControllerWrapper(completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) func(ctx genericcontrollermanager.ControllerContext) (http.Handler, bool, error) { +func startCloudNodeLifecycleControllerWrapper(completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) InitFunc { return func(ctx genericcontrollermanager.ControllerContext) (http.Handler, bool, error) { return startCloudNodeLifecycleController(completedConfig, cloud, ctx.Stop) } } // startServiceControllerWrapper is used to take cloud cofig as input and start service controller -func startServiceControllerWrapper(completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) func(ctx genericcontrollermanager.ControllerContext) (http.Handler, bool, error) { +func startServiceControllerWrapper(completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) InitFunc { return func(ctx genericcontrollermanager.ControllerContext) (http.Handler, bool, error) { return startServiceController(completedConfig, cloud, ctx.Stop) } } // startRouteControllerWrapper is used to take cloud cofig as input and start route controller -func startRouteControllerWrapper(completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) func(ctx genericcontrollermanager.ControllerContext) (http.Handler, bool, error) { +func startRouteControllerWrapper(completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) InitFunc { return func(ctx genericcontrollermanager.ControllerContext) (http.Handler, bool, error) { return startRouteController(completedConfig, cloud, ctx.Stop) } } +var DefaultInitFuncConstructors = map[string]InitFuncConstructor{ + "cloud-node": StartCloudNodeControllerWrapper, + "cloud-node-lifecycle": startCloudNodeLifecycleControllerWrapper, + "service": startServiceControllerWrapper, + "route": startRouteControllerWrapper, +} + // CreateControllerContext creates a context struct containing references to resources needed by the // controllers such as the cloud provider and clientBuilder. rootClientBuilder is only used for // the shared-informers client and token controller. diff --git a/staging/src/k8s.io/cloud-provider/app/testing/testserver.go b/staging/src/k8s.io/cloud-provider/app/testing/testserver.go index aecd90a8ec9..044ee7cbaae 100644 --- a/staging/src/k8s.io/cloud-provider/app/testing/testserver.go +++ b/staging/src/k8s.io/cloud-provider/app/testing/testserver.go @@ -124,7 +124,7 @@ func StartTestServer(t Logger, customFlags []string) (result TestServer, err err errCh := make(chan error) go func(stopCh <-chan struct{}) { - if err := app.Run(config.Complete(), app.DefaultControllerInitializers(config.Complete(), cloud), stopCh); err != nil { + if err := app.Run(config.Complete(), app.ConstructControllerInitializers(app.DefaultInitFuncConstructors, config.Complete(), cloud), stopCh); err != nil { errCh <- err } }(stopCh) diff --git a/staging/src/k8s.io/cloud-provider/sample/BUILD b/staging/src/k8s.io/cloud-provider/sample/BUILD new file mode 100644 index 00000000000..e69de29bb2d diff --git a/staging/src/k8s.io/cloud-provider/sample/README.md b/staging/src/k8s.io/cloud-provider/sample/README.md index 04ce9d0da5f..26f8ac19ea4 100644 --- a/staging/src/k8s.io/cloud-provider/sample/README.md +++ b/staging/src/k8s.io/cloud-provider/sample/README.md @@ -9,8 +9,8 @@ Begin with 1.20, all cloud providers should not copy over or vender in `k8s.io/k ## Steps cloud providers shoud follow 1. Have your external repo under k8s.io. e.g. `k8s.io/cloud-provider-` -2. Create `main.go` file under your external repo CCM directory. Please refer to `basic_main.go` for a minial working sample and `advanced_main.go` for advanced configuration samples. -Note: If you have a requirement of adding/deleting controllers within CCM, please refer to `k8s.io/kubernetes/cmd/cloud-controller-manager/main.go` for detailed samples. +2. Create `main.go` file under your external repo CCM directory. Please refer to `basic_main.go` for a minimum working sample. +Note: If you have a requirement of adding/deleting controllers within CCM, please refer to `k8s.io/kubernetes/cmd/cloud-controller-manager/main.go` for extra details. 3. Build/release CCM from your external repo. For existing cloud providers, the option to import legacy providers from `k8s.io/legacy-cloud-provider/` is still available. ## Things you should NOT do diff --git a/staging/src/k8s.io/cloud-provider/sample/advanced_main.go b/staging/src/k8s.io/cloud-provider/sample/advanced_main.go deleted file mode 100644 index b541ba531f3..00000000000 --- a/staging/src/k8s.io/cloud-provider/sample/advanced_main.go +++ /dev/null @@ -1,115 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// This file should be written by each cloud provider. -// For an minimal working example, please refer to k8s.io/cloud-provider/sample/basic_main.go -// For an advanced example, please refer to k8s.io/cloud-provider/sample/advanced_main.go -// For more details, please refer to k8s.io/kubernetes/cmd/cloud-controller-manager/main.go - -package sample - -import ( - "fmt" - "math/rand" - "os" - "time" - - "github.com/spf13/pflag" - - "k8s.io/cloud-provider" - "k8s.io/cloud-provider/app" - "k8s.io/cloud-provider/options" - "k8s.io/component-base/cli/flag" - "k8s.io/component-base/logs" - _ "k8s.io/component-base/metrics/prometheus/clientgo" // load all the prometheus client-go plugins - _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration - "k8s.io/klog/v2" - // For existing cloud providers, the option to import legacy providers is still available. - // e.g. _"k8s.io/legacy-cloud-providers/" -) - -const ( - // The variables below are samples, please edit the value for your case. - - // cloudProviderName shows an sample of using hard coded parameter - cloudProviderName = "SampleCloudProviderName" -) - -func advancedMain() { - rand.Seed(time.Now().UnixNano()) - - pflag.CommandLine.ParseErrorsWhitelist.UnknownFlags = true - _ = pflag.CommandLine.Parse(os.Args[1:]) - - // this is an example of allow-listing specific controller loops - controllerList := []string{"cloud-node", "cloud-node-lifecycle", "service", "route"} - - s, err := options.NewCloudControllerManagerOptions() - if err != nil { - klog.Fatalf("unable to initialize command options: %v", err) - } - c, err := s.Config(controllerList, app.ControllersDisabledByDefault.List()) - if err != nil { - fmt.Fprintf(os.Stderr, "%v\n", err) - os.Exit(1) - } - - cloud, err := cloudprovider.InitCloudProvider(cloudProviderName, c.ComponentConfig.KubeCloudShared.CloudProvider.CloudConfigFile) - if err != nil { - klog.Fatalf("Cloud provider could not be initialized: %v", err) - } - if cloud == nil { - klog.Fatalf("cloud provider is nil") - } - - if !cloud.HasClusterID() { - if c.ComponentConfig.KubeCloudShared.AllowUntaggedCloud { - klog.Warning("detected a cluster without a ClusterID. A ClusterID will be required in the future. Please tag your cluster to avoid any future issues") - } else { - klog.Fatalf("no ClusterID found. A ClusterID is required for the cloud provider to function properly. This check can be bypassed by setting the allow-untagged-cloud option") - } - } - - // Initialize the cloud provider with a reference to the clientBuilder - cloud.Initialize(c.ClientBuilder, make(chan struct{})) - // Set the informer on the user cloud object - if informerUserCloud, ok := cloud.(cloudprovider.InformerUser); ok { - informerUserCloud.SetInformers(c.SharedInformers) - } - - controllerInitializers := app.DefaultControllerInitializers(c.Complete(), cloud) - command := app.NewCloudControllerManagerCommand(s, c, controllerInitializers) - - // TODO: once we switch everything over to Cobra commands, we can go back to calling - // utilflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the - // normalize func and add the go flag set by hand. - // Here is an sample - pflag.CommandLine.SetNormalizeFunc(flag.WordSepNormalizeFunc) - // utilflag.InitFlags() - logs.InitLogs() - defer logs.FlushLogs() - - // the flags could be set before execute - command.Flags().VisitAll(func(flag *pflag.Flag) { - if flag.Name == "cloud-provider" { - flag.Value.Set("SampleCloudProviderFlagValue") - return - } - }) - if err := command.Execute(); err != nil { - os.Exit(1) - } -} diff --git a/staging/src/k8s.io/cloud-provider/sample/basic_main.go b/staging/src/k8s.io/cloud-provider/sample/basic_main.go index 73df23faeb6..3a51c4f74f9 100644 --- a/staging/src/k8s.io/cloud-provider/sample/basic_main.go +++ b/staging/src/k8s.io/cloud-provider/sample/basic_main.go @@ -16,21 +16,19 @@ limitations under the License. // This file should be written by each cloud provider. // For an minimal working example, please refer to k8s.io/cloud-provider/sample/basic_main.go -// For an advanced example, please refer to k8s.io/cloud-provider/sample/advanced_main.go // For more details, please refer to k8s.io/kubernetes/cmd/cloud-controller-manager/main.go package sample import ( - "fmt" "math/rand" "os" "time" "github.com/spf13/pflag" - "k8s.io/cloud-provider" "k8s.io/cloud-provider/app" + "k8s.io/cloud-provider/app/config" "k8s.io/cloud-provider/options" "k8s.io/component-base/cli/flag" "k8s.io/component-base/logs" @@ -51,42 +49,41 @@ const ( func main() { rand.Seed(time.Now().UnixNano()) - s, err := options.NewCloudControllerManagerOptions() + ccmOptions, err := options.NewCloudControllerManagerOptions() if err != nil { klog.Fatalf("unable to initialize command options: %v", err) } - c, err := s.Config([]string{}, app.ControllersDisabledByDefault.List()) - if err != nil { - fmt.Fprintf(os.Stderr, "%v\n", err) - os.Exit(1) - } - // initialize cloud provider with the cloud provider name and config file provided - cloud, err := cloudprovider.InitCloudProvider(sampleCloudProviderName, c.ComponentConfig.KubeCloudShared.CloudProvider.CloudConfigFile) - if err != nil { - klog.Fatalf("Cloud provider could not be initialized: %v", err) - } - if cloud == nil { - klog.Fatalf("cloud provider is nil") - } - - if !cloud.HasClusterID() { - if c.ComponentConfig.KubeCloudShared.AllowUntaggedCloud { - klog.Warning("detected a cluster without a ClusterID. A ClusterID will be required in the future. Please tag your cluster to avoid any future issues") - } else { - klog.Fatalf("no ClusterID found. A ClusterID is required for the cloud provider to function properly. This check can be bypassed by setting the allow-untagged-cloud option") + cloudInitializer := func(config *config.CompletedConfig) cloudprovider.Interface { + cloudConfigFile := config.ComponentConfig.KubeCloudShared.CloudProvider.CloudConfigFile + // initialize cloud provider with the cloud provider name and config file provided + cloud, err := cloudprovider.InitCloudProvider(sampleCloudProviderName, cloudConfigFile) + if err != nil { + klog.Fatalf("Cloud provider could not be initialized: %v", err) } + if cloud == nil { + klog.Fatalf("Cloud provider is nil") + } + + if !cloud.HasClusterID() { + if config.ComponentConfig.KubeCloudShared.AllowUntaggedCloud { + klog.Warning("detected a cluster without a ClusterID. A ClusterID will be required in the future. Please tag your cluster to avoid any future issues") + } else { + klog.Fatalf("no ClusterID found. A ClusterID is required for the cloud provider to function properly. This check can be bypassed by setting the allow-untagged-cloud option") + } + } + + // Initialize the cloud provider with a reference to the clientBuilder + cloud.Initialize(config.ClientBuilder, make(chan struct{})) + // Set the informer on the user cloud object + if informerUserCloud, ok := cloud.(cloudprovider.InformerUser); ok { + informerUserCloud.SetInformers(config.SharedInformers) + } + + return cloud } - // Initialize the cloud provider with a reference to the clientBuilder - cloud.Initialize(c.ClientBuilder, make(chan struct{})) - // Set the informer on the user cloud object - if informerUserCloud, ok := cloud.(cloudprovider.InformerUser); ok { - informerUserCloud.SetInformers(c.SharedInformers) - } - - controllerInitializers := app.DefaultControllerInitializers(c.Complete(), cloud) - command := app.NewCloudControllerManagerCommand(s, c, controllerInitializers) + command := app.NewCloudControllerManagerCommand(sampleCloudProviderName, ccmOptions, cloudInitializer, app.DefaultInitFuncConstructors) // TODO: once we switch everything over to Cobra commands, we can go back to calling // utilflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the @@ -97,13 +94,6 @@ func main() { logs.InitLogs() defer logs.FlushLogs() - // the flags could be set before execute - command.Flags().VisitAll(func(flag *pflag.Flag) { - if flag.Name == "cloud-provider" { - flag.Value.Set("SampleCloudProviderFlagValue") - return - } - }) if err := command.Execute(); err != nil { os.Exit(1) } From 4729fa49e1c5c32109997e09b54168eef6cbf5ad Mon Sep 17 00:00:00 2001 From: cici37 Date: Wed, 20 Jan 2021 01:28:44 -0800 Subject: [PATCH 02/16] Remove cloud provider name as input parameter. --- cmd/cloud-controller-manager/main.go | 2 +- staging/src/k8s.io/cloud-provider/app/controllermanager.go | 7 +------ staging/src/k8s.io/cloud-provider/sample/basic_main.go | 2 +- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/cmd/cloud-controller-manager/main.go b/cmd/cloud-controller-manager/main.go index 9a1b9f6ab38..8d59739a521 100644 --- a/cmd/cloud-controller-manager/main.go +++ b/cmd/cloud-controller-manager/main.go @@ -99,7 +99,7 @@ func main() { // If you do not need additional controller, please ignore. controllerInitializers["nodeipam"] = startNodeIpamControllerWrapper - command := app.NewCloudControllerManagerCommand(cloudProviderName, ccmOptions, cloudInitializer, controllerInitializers) + command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, controllerInitializers) // TODO: once we switch everything over to Cobra commands, we can go back to calling // utilflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the diff --git a/staging/src/k8s.io/cloud-provider/app/controllermanager.go b/staging/src/k8s.io/cloud-provider/app/controllermanager.go index f5db9bb2e45..6341213c7b7 100644 --- a/staging/src/k8s.io/cloud-provider/app/controllermanager.go +++ b/staging/src/k8s.io/cloud-provider/app/controllermanager.go @@ -64,7 +64,7 @@ const ( ) // NewCloudControllerManagerCommand creates a *cobra.Command object with default parameters -func NewCloudControllerManagerCommand(cloudProviderName string, s *options.CloudControllerManagerOptions, cloudInitializer InitCloudFunc, initFuncConstructor map[string]InitFuncConstructor) *cobra.Command { +func NewCloudControllerManagerCommand(s *options.CloudControllerManagerOptions, cloudInitializer InitCloudFunc, initFuncConstructor map[string]InitFuncConstructor) *cobra.Command { cmd := &cobra.Command{ Use: "cloud-controller-manager", @@ -72,11 +72,6 @@ func NewCloudControllerManagerCommand(cloudProviderName string, s *options.Cloud the cloud specific control loops shipped with Kubernetes.`, Run: func(cmd *cobra.Command, args []string) { verflag.PrintAndExitIfRequested() - - cloudProviderFlag := cmd.Flags().Lookup("cloud-provider") - if cloudProviderFlag.Value.String() == "" { - cloudProviderFlag.Value.Set(cloudProviderName) - } cliflag.PrintFlags(cmd.Flags()) c, err := s.Config(ControllerNames(initFuncConstructor), ControllersDisabledByDefault.List()) diff --git a/staging/src/k8s.io/cloud-provider/sample/basic_main.go b/staging/src/k8s.io/cloud-provider/sample/basic_main.go index 3a51c4f74f9..90f091294e7 100644 --- a/staging/src/k8s.io/cloud-provider/sample/basic_main.go +++ b/staging/src/k8s.io/cloud-provider/sample/basic_main.go @@ -83,7 +83,7 @@ func main() { return cloud } - command := app.NewCloudControllerManagerCommand(sampleCloudProviderName, ccmOptions, cloudInitializer, app.DefaultInitFuncConstructors) + command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, app.DefaultInitFuncConstructors) // TODO: once we switch everything over to Cobra commands, we can go back to calling // utilflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the From 7b0a6db0977d7dcca0978bbe854da9dd72941f75 Mon Sep 17 00:00:00 2001 From: cici37 Date: Wed, 20 Jan 2021 12:09:46 -0800 Subject: [PATCH 03/16] Add demonstration of wiring nodeIPAMController config object --- .../.import-restrictions | 7 +- cmd/cloud-controller-manager/main.go | 78 ++++++++++--------- .../cloud-provider/app/controllermanager.go | 14 +++- .../cloud-provider/sample/basic_main.go | 62 +++++++-------- 4 files changed, 89 insertions(+), 72 deletions(-) diff --git a/cmd/cloud-controller-manager/.import-restrictions b/cmd/cloud-controller-manager/.import-restrictions index 4515e2f7be5..151e8744804 100644 --- a/cmd/cloud-controller-manager/.import-restrictions +++ b/cmd/cloud-controller-manager/.import-restrictions @@ -1,6 +1,8 @@ rules: - selectorRegexp: k8s[.]io/kubernetes allowedPrefixes: + - k8s.io/kubernetes/cmd/kube-controller-manager/app/options + - k8s.io/kubernetes/cmd/kube-controller-manager/app/config - k8s.io/kubernetes/pkg/api/legacyscheme - k8s.io/kubernetes/pkg/api/service - k8s.io/kubernetes/pkg/api/v1/pod @@ -34,4 +36,7 @@ rules: - k8s.io/kubernetes/pkg/util/hash - k8s.io/kubernetes/pkg/util/node - k8s.io/kubernetes/pkg/util/parsers - - k8s.io/kubernetes/pkg/util/taints \ No newline at end of file + - k8s.io/kubernetes/pkg/util/taints + - k8s.io/kubernetes/pkg/proxy/util + - k8s.io/kubernetes/pkg/proxy/util/testing + - k8s.io/kubernetes/pkg/util/sysctl \ No newline at end of file diff --git a/cmd/cloud-controller-manager/main.go b/cmd/cloud-controller-manager/main.go index 8d59739a521..e5373b1e7ca 100644 --- a/cmd/cloud-controller-manager/main.go +++ b/cmd/cloud-controller-manager/main.go @@ -42,6 +42,7 @@ import ( _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration genericcontrollermanager "k8s.io/controller-manager/app" "k8s.io/klog/v2" + nodeipamcontrolleroptions "k8s.io/kubernetes/cmd/kube-controller-manager/app/options" nodeipamconfig "k8s.io/kubernetes/pkg/controller/nodeipam/config" // For existing cloud providers, the option to import legacy providers is still available. // e.g. _"k8s.io/legacy-cloud-providers/" @@ -60,35 +61,6 @@ func main() { klog.Fatalf("unable to initialize command options: %v", err) } - cloudInitializer := func(config *cloudcontrollerconfig.CompletedConfig) cloudprovider.Interface { - cloudConfigFile := config.ComponentConfig.KubeCloudShared.CloudProvider.CloudConfigFile - // initialize cloud provider with the cloud provider name and config file provided - cloud, err := cloudprovider.InitCloudProvider(cloudProviderName, cloudConfigFile) - if err != nil { - klog.Fatalf("Cloud provider could not be initialized: %v", err) - } - if cloud == nil { - klog.Fatalf("Cloud provider is nil") - } - - if !cloud.HasClusterID() { - if config.ComponentConfig.KubeCloudShared.AllowUntaggedCloud { - klog.Warning("detected a cluster without a ClusterID. A ClusterID will be required in the future. Please tag your cluster to avoid any future issues") - } else { - klog.Fatalf("no ClusterID found. A ClusterID is required for the cloud provider to function properly. This check can be bypassed by setting the allow-untagged-cloud option") - } - } - - // Initialize the cloud provider with a reference to the clientBuilder - cloud.Initialize(config.ClientBuilder, make(chan struct{})) - // Set the informer on the user cloud object - if informerUserCloud, ok := cloud.(cloudprovider.InformerUser); ok { - informerUserCloud.SetInformers(config.SharedInformers) - } - - return cloud - } - controllerInitializers := app.DefaultInitFuncConstructors // Here is an example to remove the controller which is not needed. // e.g. remove the cloud-node-lifecycle controller which current cloud provider does not need. @@ -99,7 +71,7 @@ func main() { // If you do not need additional controller, please ignore. controllerInitializers["nodeipam"] = startNodeIpamControllerWrapper - command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, controllerInitializers) + command := app.NewCloudControllerManagerCommand(cloudProviderName, ccmOptions, cloudInitializer, controllerInitializers) // TODO: once we switch everything over to Cobra commands, we can go back to calling // utilflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the @@ -115,14 +87,46 @@ func main() { } } -func startNodeIpamControllerWrapper(completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) app.InitFunc { - nodeIPAMConfig := nodeipamconfig.NodeIPAMControllerConfiguration{ - ServiceCIDR: "sample", - SecondaryServiceCIDR: "sample", - NodeCIDRMaskSize: 11, - NodeCIDRMaskSizeIPv4: 11, - NodeCIDRMaskSizeIPv6: 111, +func cloudInitializer(config *cloudcontrollerconfig.CompletedConfig) cloudprovider.Interface { + cloudConfigFile := config.ComponentConfig.KubeCloudShared.CloudProvider.CloudConfigFile + // initialize cloud provider with the cloud provider name and config file provided + cloud, err := cloudprovider.InitCloudProvider(cloudProviderName, cloudConfigFile) + if err != nil { + klog.Fatalf("Cloud provider could not be initialized: %v", err) } + if cloud == nil { + klog.Fatalf("Cloud provider is nil") + } + + if !cloud.HasClusterID() { + if config.ComponentConfig.KubeCloudShared.AllowUntaggedCloud { + klog.Warning("detected a cluster without a ClusterID. A ClusterID will be required in the future. Please tag your cluster to avoid any future issues") + } else { + klog.Fatalf("no ClusterID found. A ClusterID is required for the cloud provider to function properly. This check can be bypassed by setting the allow-untagged-cloud option") + } + } + + // Initialize the cloud provider with a reference to the clientBuilder + cloud.Initialize(config.ClientBuilder, make(chan struct{})) + // Set the informer on the user cloud object + if informerUserCloud, ok := cloud.(cloudprovider.InformerUser); ok { + informerUserCloud.SetInformers(config.SharedInformers) + } + + return cloud +} + +func startNodeIpamControllerWrapper(completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) app.InitFunc { + fs := pflag.NewFlagSet("fs", pflag.ContinueOnError) + var nodeIPAMControllerOptions nodeipamcontrolleroptions.NodeIPAMControllerOptions + nodeIPAMControllerOptions.AddFlags(fs) + errors := nodeIPAMControllerOptions.Validate() + if len(errors) > 0 { + klog.Fatal("NodeIPAM controller values are not properly.") + } + var nodeIPAMConfig nodeipamconfig.NodeIPAMControllerConfiguration + nodeIPAMControllerOptions.ApplyTo(&nodeIPAMConfig) + return func(ctx genericcontrollermanager.ControllerContext) (http.Handler, bool, error) { return startNodeIpamController(completedConfig, nodeIPAMConfig, ctx, cloud) } diff --git a/staging/src/k8s.io/cloud-provider/app/controllermanager.go b/staging/src/k8s.io/cloud-provider/app/controllermanager.go index 6341213c7b7..58c93bd80e9 100644 --- a/staging/src/k8s.io/cloud-provider/app/controllermanager.go +++ b/staging/src/k8s.io/cloud-provider/app/controllermanager.go @@ -64,7 +64,7 @@ const ( ) // NewCloudControllerManagerCommand creates a *cobra.Command object with default parameters -func NewCloudControllerManagerCommand(s *options.CloudControllerManagerOptions, cloudInitializer InitCloudFunc, initFuncConstructor map[string]InitFuncConstructor) *cobra.Command { +func NewCloudControllerManagerCommand(cloudProviderName string, s *options.CloudControllerManagerOptions, cloudInitializer InitCloudFunc, initFuncConstructor map[string]InitFuncConstructor) *cobra.Command { cmd := &cobra.Command{ Use: "cloud-controller-manager", @@ -72,6 +72,11 @@ func NewCloudControllerManagerCommand(s *options.CloudControllerManagerOptions, the cloud specific control loops shipped with Kubernetes.`, Run: func(cmd *cobra.Command, args []string) { verflag.PrintAndExitIfRequested() + + cloudProviderFlag := cmd.Flags().Lookup("cloud-provider") + if cloudProviderFlag.Value.String() == "" { + cloudProviderFlag.Value.Set(cloudProviderName) + } cliflag.PrintFlags(cmd.Flags()) c, err := s.Config(ControllerNames(initFuncConstructor), ControllersDisabledByDefault.List()) @@ -130,8 +135,6 @@ the cloud specific control loops shipped with Kubernetes.`, return cmd } -type InitCloudFunc func(config *cloudcontrollerconfig.CompletedConfig) cloudprovider.Interface - // Run runs the ExternalCMServer. This should never exit. func Run(c *cloudcontrollerconfig.CompletedConfig, controllerInitializers map[string]InitFunc, stopCh <-chan struct{}) error { // To help debugging, immediately log version @@ -262,11 +265,15 @@ func startControllers(ctx genericcontrollermanager.ControllerContext, c *cloudco select {} } +// InitCloudFunc is used to initialize cloud +type InitCloudFunc func(config *cloudcontrollerconfig.CompletedConfig) cloudprovider.Interface + // InitFunc is used to launch a particular controller. It may run additional "should I activate checks". // Any error returned will cause the controller process to `Fatal` // The bool indicates whether the controller was enabled. type InitFunc func(ctx genericcontrollermanager.ControllerContext) (debuggingHandler http.Handler, enabled bool, err error) +// InitFuncConstructor is used to construct InitFunc type InitFuncConstructor func(completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) InitFunc // ControllerNames indicate the default controller we are known. @@ -316,6 +323,7 @@ func startRouteControllerWrapper(completedConfig *cloudcontrollerconfig.Complete } } +// DefaultInitFuncConstructors is a map of default named controller groups paired with InitFuncConstructor var DefaultInitFuncConstructors = map[string]InitFuncConstructor{ "cloud-node": StartCloudNodeControllerWrapper, "cloud-node-lifecycle": startCloudNodeLifecycleControllerWrapper, diff --git a/staging/src/k8s.io/cloud-provider/sample/basic_main.go b/staging/src/k8s.io/cloud-provider/sample/basic_main.go index 90f091294e7..50592eb85ca 100644 --- a/staging/src/k8s.io/cloud-provider/sample/basic_main.go +++ b/staging/src/k8s.io/cloud-provider/sample/basic_main.go @@ -18,7 +18,7 @@ limitations under the License. // For an minimal working example, please refer to k8s.io/cloud-provider/sample/basic_main.go // For more details, please refer to k8s.io/kubernetes/cmd/cloud-controller-manager/main.go -package sample +package main import ( "math/rand" @@ -54,36 +54,7 @@ func main() { klog.Fatalf("unable to initialize command options: %v", err) } - cloudInitializer := func(config *config.CompletedConfig) cloudprovider.Interface { - cloudConfigFile := config.ComponentConfig.KubeCloudShared.CloudProvider.CloudConfigFile - // initialize cloud provider with the cloud provider name and config file provided - cloud, err := cloudprovider.InitCloudProvider(sampleCloudProviderName, cloudConfigFile) - if err != nil { - klog.Fatalf("Cloud provider could not be initialized: %v", err) - } - if cloud == nil { - klog.Fatalf("Cloud provider is nil") - } - - if !cloud.HasClusterID() { - if config.ComponentConfig.KubeCloudShared.AllowUntaggedCloud { - klog.Warning("detected a cluster without a ClusterID. A ClusterID will be required in the future. Please tag your cluster to avoid any future issues") - } else { - klog.Fatalf("no ClusterID found. A ClusterID is required for the cloud provider to function properly. This check can be bypassed by setting the allow-untagged-cloud option") - } - } - - // Initialize the cloud provider with a reference to the clientBuilder - cloud.Initialize(config.ClientBuilder, make(chan struct{})) - // Set the informer on the user cloud object - if informerUserCloud, ok := cloud.(cloudprovider.InformerUser); ok { - informerUserCloud.SetInformers(config.SharedInformers) - } - - return cloud - } - - command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, app.DefaultInitFuncConstructors) + command := app.NewCloudControllerManagerCommand(sampleCloudProviderName, ccmOptions, cloudInitializer, app.DefaultInitFuncConstructors) // TODO: once we switch everything over to Cobra commands, we can go back to calling // utilflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the @@ -98,3 +69,32 @@ func main() { os.Exit(1) } } + +func cloudInitializer(config *config.CompletedConfig) cloudprovider.Interface { + cloudConfigFile := config.ComponentConfig.KubeCloudShared.CloudProvider.CloudConfigFile + // initialize cloud provider with the cloud provider name and config file provided + cloud, err := cloudprovider.InitCloudProvider(sampleCloudProviderName, cloudConfigFile) + if err != nil { + klog.Fatalf("Cloud provider could not be initialized: %v", err) + } + if cloud == nil { + klog.Fatalf("Cloud provider is nil") + } + + if !cloud.HasClusterID() { + if config.ComponentConfig.KubeCloudShared.AllowUntaggedCloud { + klog.Warning("detected a cluster without a ClusterID. A ClusterID will be required in the future. Please tag your cluster to avoid any future issues") + } else { + klog.Fatalf("no ClusterID found. A ClusterID is required for the cloud provider to function properly. This check can be bypassed by setting the allow-untagged-cloud option") + } + } + + // Initialize the cloud provider with a reference to the clientBuilder + cloud.Initialize(config.ClientBuilder, make(chan struct{})) + // Set the informer on the user cloud object + if informerUserCloud, ok := cloud.(cloudprovider.InformerUser); ok { + informerUserCloud.SetInformers(config.SharedInformers) + } + + return cloud +} From 52bab024af652a8247e5395ea74354adbf93399f Mon Sep 17 00:00:00 2001 From: cici37 Date: Thu, 21 Jan 2021 12:47:45 -0800 Subject: [PATCH 04/16] Separate func --- cmd/cloud-controller-manager/main.go | 2 +- staging/src/k8s.io/cloud-provider/app/controllermanager.go | 7 +------ staging/src/k8s.io/cloud-provider/sample/basic_main.go | 2 +- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/cmd/cloud-controller-manager/main.go b/cmd/cloud-controller-manager/main.go index e5373b1e7ca..f9ff9a57080 100644 --- a/cmd/cloud-controller-manager/main.go +++ b/cmd/cloud-controller-manager/main.go @@ -71,7 +71,7 @@ func main() { // If you do not need additional controller, please ignore. controllerInitializers["nodeipam"] = startNodeIpamControllerWrapper - command := app.NewCloudControllerManagerCommand(cloudProviderName, ccmOptions, cloudInitializer, controllerInitializers) + command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, controllerInitializers) // TODO: once we switch everything over to Cobra commands, we can go back to calling // utilflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the diff --git a/staging/src/k8s.io/cloud-provider/app/controllermanager.go b/staging/src/k8s.io/cloud-provider/app/controllermanager.go index 58c93bd80e9..513d12213fd 100644 --- a/staging/src/k8s.io/cloud-provider/app/controllermanager.go +++ b/staging/src/k8s.io/cloud-provider/app/controllermanager.go @@ -64,7 +64,7 @@ const ( ) // NewCloudControllerManagerCommand creates a *cobra.Command object with default parameters -func NewCloudControllerManagerCommand(cloudProviderName string, s *options.CloudControllerManagerOptions, cloudInitializer InitCloudFunc, initFuncConstructor map[string]InitFuncConstructor) *cobra.Command { +func NewCloudControllerManagerCommand(s *options.CloudControllerManagerOptions, cloudInitializer InitCloudFunc, initFuncConstructor map[string]InitFuncConstructor) *cobra.Command { cmd := &cobra.Command{ Use: "cloud-controller-manager", @@ -72,11 +72,6 @@ func NewCloudControllerManagerCommand(cloudProviderName string, s *options.Cloud the cloud specific control loops shipped with Kubernetes.`, Run: func(cmd *cobra.Command, args []string) { verflag.PrintAndExitIfRequested() - - cloudProviderFlag := cmd.Flags().Lookup("cloud-provider") - if cloudProviderFlag.Value.String() == "" { - cloudProviderFlag.Value.Set(cloudProviderName) - } cliflag.PrintFlags(cmd.Flags()) c, err := s.Config(ControllerNames(initFuncConstructor), ControllersDisabledByDefault.List()) diff --git a/staging/src/k8s.io/cloud-provider/sample/basic_main.go b/staging/src/k8s.io/cloud-provider/sample/basic_main.go index 50592eb85ca..3bda02e55e9 100644 --- a/staging/src/k8s.io/cloud-provider/sample/basic_main.go +++ b/staging/src/k8s.io/cloud-provider/sample/basic_main.go @@ -54,7 +54,7 @@ func main() { klog.Fatalf("unable to initialize command options: %v", err) } - command := app.NewCloudControllerManagerCommand(sampleCloudProviderName, ccmOptions, cloudInitializer, app.DefaultInitFuncConstructors) + command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, app.DefaultInitFuncConstructors) // TODO: once we switch everything over to Cobra commands, we can go back to calling // utilflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the From edc5b58e23db379d680afbfcbc46a7eac3abe54a Mon Sep 17 00:00:00 2001 From: cici37 Date: Thu, 21 Jan 2021 13:39:26 -0800 Subject: [PATCH 05/16] Update BUILD and staticcheck. --- hack/.staticcheck_failures | 1 - 1 file changed, 1 deletion(-) diff --git a/hack/.staticcheck_failures b/hack/.staticcheck_failures index 5fae0288da2..fdbfbf3b5a3 100644 --- a/hack/.staticcheck_failures +++ b/hack/.staticcheck_failures @@ -30,4 +30,3 @@ vendor/k8s.io/client-go/discovery vendor/k8s.io/client-go/rest vendor/k8s.io/client-go/rest/watch vendor/k8s.io/client-go/transport -vendor/k8s.io/cloud-provider/sample From 498fa39af28e951a91e2b82f521a06b6f909cd62 Mon Sep 17 00:00:00 2001 From: cici37 Date: Mon, 25 Jan 2021 12:52:02 -0800 Subject: [PATCH 06/16] Separate example func and add README.md --- cmd/cloud-controller-manager/README.md | 14 +++++++++++++ cmd/cloud-controller-manager/main.go | 20 ------------------- .../nodeipamcontroller.go | 20 +++++++++++++++++++ .../k8s.io/cloud-provider/sample/README.md | 4 ++-- 4 files changed, 36 insertions(+), 22 deletions(-) create mode 100644 cmd/cloud-controller-manager/README.md diff --git a/cmd/cloud-controller-manager/README.md b/cmd/cloud-controller-manager/README.md new file mode 100644 index 00000000000..7248ce39e47 --- /dev/null +++ b/cmd/cloud-controller-manager/README.md @@ -0,0 +1,14 @@ +# cloud-controller-manager/example + +This directory provides an example of how to leverage CCM extension mechanism. + +## Purpose + +Begin with 1.20, all cloud providers should not copy over or vendor in `k8s.io/kubernetes/cmd/cloud-controller-manager`. Inside this directory, an example is included to demonstrate how to leverage CCM extension mechanism to add a controller. +Please refer to `k8s.io/cloud-provider/sample` if you do not have the requirement of adding/deleting controllers in CCM. + +## Things you should NOT do + +1. Vendor in `k8s.io/cmd/cloud-controller-manager`. +2. Directly modify anything under `k8s.io/cmd/cloud-controller-manager` in this repo. +3. Make specific cloud provider changes here. diff --git a/cmd/cloud-controller-manager/main.go b/cmd/cloud-controller-manager/main.go index f9ff9a57080..73d0feb1bc1 100644 --- a/cmd/cloud-controller-manager/main.go +++ b/cmd/cloud-controller-manager/main.go @@ -26,7 +26,6 @@ package main import ( "math/rand" - "net/http" "os" "time" @@ -40,10 +39,7 @@ import ( "k8s.io/component-base/logs" _ "k8s.io/component-base/metrics/prometheus/clientgo" // load all the prometheus client-go plugins _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration - genericcontrollermanager "k8s.io/controller-manager/app" "k8s.io/klog/v2" - nodeipamcontrolleroptions "k8s.io/kubernetes/cmd/kube-controller-manager/app/options" - nodeipamconfig "k8s.io/kubernetes/pkg/controller/nodeipam/config" // For existing cloud providers, the option to import legacy providers is still available. // e.g. _"k8s.io/legacy-cloud-providers/" ) @@ -115,19 +111,3 @@ func cloudInitializer(config *cloudcontrollerconfig.CompletedConfig) cloudprovid return cloud } - -func startNodeIpamControllerWrapper(completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) app.InitFunc { - fs := pflag.NewFlagSet("fs", pflag.ContinueOnError) - var nodeIPAMControllerOptions nodeipamcontrolleroptions.NodeIPAMControllerOptions - nodeIPAMControllerOptions.AddFlags(fs) - errors := nodeIPAMControllerOptions.Validate() - if len(errors) > 0 { - klog.Fatal("NodeIPAM controller values are not properly.") - } - var nodeIPAMConfig nodeipamconfig.NodeIPAMControllerConfiguration - nodeIPAMControllerOptions.ApplyTo(&nodeIPAMConfig) - - return func(ctx genericcontrollermanager.ControllerContext) (http.Handler, bool, error) { - return startNodeIpamController(completedConfig, nodeIPAMConfig, ctx, cloud) - } -} diff --git a/cmd/cloud-controller-manager/nodeipamcontroller.go b/cmd/cloud-controller-manager/nodeipamcontroller.go index b96fa6c80c7..3d6d928db60 100644 --- a/cmd/cloud-controller-manager/nodeipamcontroller.go +++ b/cmd/cloud-controller-manager/nodeipamcontroller.go @@ -22,16 +22,20 @@ package main import ( "errors" "fmt" + "github.com/spf13/pflag" + "net" "net/http" "strings" utilfeature "k8s.io/apiserver/pkg/util/feature" cloudprovider "k8s.io/cloud-provider" + "k8s.io/cloud-provider/app" cloudcontrollerconfig "k8s.io/cloud-provider/app/config" genericcontrollermanager "k8s.io/controller-manager/app" "k8s.io/controller-manager/pkg/features" "k8s.io/klog/v2" + nodeipamcontrolleroptions "k8s.io/kubernetes/cmd/kube-controller-manager/app/options" nodeipamcontroller "k8s.io/kubernetes/pkg/controller/nodeipam" nodeipamconfig "k8s.io/kubernetes/pkg/controller/nodeipam/config" "k8s.io/kubernetes/pkg/controller/nodeipam/ipam" @@ -45,6 +49,22 @@ const ( defaultNodeMaskCIDRIPv6 = 64 ) +func startNodeIpamControllerWrapper(completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) app.InitFunc { + fs := pflag.NewFlagSet("fs", pflag.ContinueOnError) + var nodeIPAMControllerOptions nodeipamcontrolleroptions.NodeIPAMControllerOptions + nodeIPAMControllerOptions.AddFlags(fs) + errors := nodeIPAMControllerOptions.Validate() + if len(errors) > 0 { + klog.Fatal("NodeIPAM controller values are not properly.") + } + var nodeIPAMConfig nodeipamconfig.NodeIPAMControllerConfiguration + nodeIPAMControllerOptions.ApplyTo(&nodeIPAMConfig) + + return func(ctx genericcontrollermanager.ControllerContext) (http.Handler, bool, error) { + return startNodeIpamController(completedConfig, nodeIPAMConfig, ctx, cloud) + } +} + func startNodeIpamController(ccmConfig *cloudcontrollerconfig.CompletedConfig, nodeIPAMConfig nodeipamconfig.NodeIPAMControllerConfiguration, ctx genericcontrollermanager.ControllerContext, cloud cloudprovider.Interface) (http.Handler, bool, error) { var serviceCIDR *net.IPNet var secondaryServiceCIDR *net.IPNet diff --git a/staging/src/k8s.io/cloud-provider/sample/README.md b/staging/src/k8s.io/cloud-provider/sample/README.md index 26f8ac19ea4..369ee82ba55 100644 --- a/staging/src/k8s.io/cloud-provider/sample/README.md +++ b/staging/src/k8s.io/cloud-provider/sample/README.md @@ -4,9 +4,9 @@ This directory provides sample code about how all cloud providers should leverag ## Purpose -Begin with 1.20, all cloud providers should not copy over or vender in `k8s.io/kubernetes/cmd/cloud-controller-manager`. Inside this directory, some sample code will be provided to demonstrate how cloud providers should leverage cloud-controller-manager. +Begin with 1.20, all cloud providers should not copy over or vendor in `k8s.io/kubernetes/cmd/cloud-controller-manager`. Inside this directory, some sample code will be provided to demonstrate how cloud providers should leverage cloud-controller-manager. -## Steps cloud providers shoud follow +## Steps cloud providers should follow 1. Have your external repo under k8s.io. e.g. `k8s.io/cloud-provider-` 2. Create `main.go` file under your external repo CCM directory. Please refer to `basic_main.go` for a minimum working sample. From 27aaee2b40c7694261f72810175971c942e39405 Mon Sep 17 00:00:00 2001 From: cici37 Date: Tue, 2 Feb 2021 13:25:33 -0800 Subject: [PATCH 07/16] Move initialize cloud provider with client builder reference inside controller start func --- cmd/cloud-controller-manager/main.go | 7 ------- .../k8s.io/cloud-provider/app/controllermanager.go | 14 ++++++++++---- .../src/k8s.io/cloud-provider/sample/basic_main.go | 8 -------- 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/cmd/cloud-controller-manager/main.go b/cmd/cloud-controller-manager/main.go index 73d0feb1bc1..227897e2004 100644 --- a/cmd/cloud-controller-manager/main.go +++ b/cmd/cloud-controller-manager/main.go @@ -102,12 +102,5 @@ func cloudInitializer(config *cloudcontrollerconfig.CompletedConfig) cloudprovid } } - // Initialize the cloud provider with a reference to the clientBuilder - cloud.Initialize(config.ClientBuilder, make(chan struct{})) - // Set the informer on the user cloud object - if informerUserCloud, ok := cloud.(cloudprovider.InformerUser); ok { - informerUserCloud.SetInformers(config.SharedInformers) - } - return cloud } diff --git a/staging/src/k8s.io/cloud-provider/app/controllermanager.go b/staging/src/k8s.io/cloud-provider/app/controllermanager.go index 513d12213fd..3f7d18ed15e 100644 --- a/staging/src/k8s.io/cloud-provider/app/controllermanager.go +++ b/staging/src/k8s.io/cloud-provider/app/controllermanager.go @@ -84,7 +84,7 @@ the cloud specific control loops shipped with Kubernetes.`, cloud := cloudInitializer(completedConfig) controllerInitializers := ConstructControllerInitializers(initFuncConstructor, completedConfig, cloud) - if err := Run(completedConfig, controllerInitializers, wait.NeverStop); err != nil { + if err := Run(completedConfig, cloud, controllerInitializers, wait.NeverStop); err != nil { fmt.Fprintf(os.Stderr, "%v\n", err) os.Exit(1) } @@ -131,7 +131,7 @@ the cloud specific control loops shipped with Kubernetes.`, } // Run runs the ExternalCMServer. This should never exit. -func Run(c *cloudcontrollerconfig.CompletedConfig, controllerInitializers map[string]InitFunc, stopCh <-chan struct{}) error { +func Run(c *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface, controllerInitializers map[string]InitFunc, stopCh <-chan struct{}) error { // To help debugging, immediately log version klog.Infof("Version: %+v", version.Get()) @@ -176,7 +176,7 @@ func Run(c *cloudcontrollerconfig.CompletedConfig, controllerInitializers map[st if err != nil { klog.Fatalf("error building controller context: %v", err) } - if err := startControllers(controllerContext, c, ctx.Done(), controllerInitializers); err != nil { + if err := startControllers(cloud, controllerContext, c, ctx.Done(), controllerInitializers); err != nil { klog.Fatalf("error running controllers: %v", err) } } @@ -227,7 +227,13 @@ func Run(c *cloudcontrollerconfig.CompletedConfig, controllerInitializers map[st } // startControllers starts the cloud specific controller loops. -func startControllers(ctx genericcontrollermanager.ControllerContext, c *cloudcontrollerconfig.CompletedConfig, stopCh <-chan struct{}, controllers map[string]InitFunc) error { +func startControllers(cloud cloudprovider.Interface, ctx genericcontrollermanager.ControllerContext, c *cloudcontrollerconfig.CompletedConfig, stopCh <-chan struct{}, controllers map[string]InitFunc) error { + // Initialize the cloud provider with a reference to the clientBuilder + cloud.Initialize(c.ClientBuilder, stopCh) + // Set the informer on the user cloud object + if informerUserCloud, ok := cloud.(cloudprovider.InformerUser); ok { + informerUserCloud.SetInformers(c.SharedInformers) + } for controllerName, initFn := range controllers { if !genericcontrollermanager.IsControllerEnabled(controllerName, ControllersDisabledByDefault, c.ComponentConfig.Generic.Controllers) { klog.Warningf("%q is disabled", controllerName) diff --git a/staging/src/k8s.io/cloud-provider/sample/basic_main.go b/staging/src/k8s.io/cloud-provider/sample/basic_main.go index 3bda02e55e9..573f1fd6b35 100644 --- a/staging/src/k8s.io/cloud-provider/sample/basic_main.go +++ b/staging/src/k8s.io/cloud-provider/sample/basic_main.go @@ -88,13 +88,5 @@ func cloudInitializer(config *config.CompletedConfig) cloudprovider.Interface { klog.Fatalf("no ClusterID found. A ClusterID is required for the cloud provider to function properly. This check can be bypassed by setting the allow-untagged-cloud option") } } - - // Initialize the cloud provider with a reference to the clientBuilder - cloud.Initialize(config.ClientBuilder, make(chan struct{})) - // Set the informer on the user cloud object - if informerUserCloud, ok := cloud.(cloudprovider.InformerUser); ok { - informerUserCloud.SetInformers(config.SharedInformers) - } - return cloud } From 3c150e8e1f49cdbae523bc2a095434376e5956ba Mon Sep 17 00:00:00 2001 From: cici37 Date: Tue, 2 Feb 2021 14:05:11 -0800 Subject: [PATCH 08/16] Update test --- staging/src/k8s.io/cloud-provider/app/testing/testserver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/staging/src/k8s.io/cloud-provider/app/testing/testserver.go b/staging/src/k8s.io/cloud-provider/app/testing/testserver.go index 044ee7cbaae..daf22909bd6 100644 --- a/staging/src/k8s.io/cloud-provider/app/testing/testserver.go +++ b/staging/src/k8s.io/cloud-provider/app/testing/testserver.go @@ -124,7 +124,7 @@ func StartTestServer(t Logger, customFlags []string) (result TestServer, err err errCh := make(chan error) go func(stopCh <-chan struct{}) { - if err := app.Run(config.Complete(), app.ConstructControllerInitializers(app.DefaultInitFuncConstructors, config.Complete(), cloud), stopCh); err != nil { + if err := app.Run(config.Complete(), cloud, app.ConstructControllerInitializers(app.DefaultInitFuncConstructors, config.Complete(), cloud), stopCh); err != nil { errCh <- err } }(stopCh) From b669f0464899c37f3c64d3a397e7ae297c8d78df Mon Sep 17 00:00:00 2001 From: cici37 Date: Wed, 10 Feb 2021 15:00:44 -0800 Subject: [PATCH 09/16] Modify integration test to fill CCM test gap --- cmd/cloud-controller-manager/main.go | 12 +- .../cloud-provider/app/controllermanager.go | 13 ++- .../cloud-provider/app/testing/testserver.go | 109 ++++++++++++------ .../cloud-provider/sample/basic_main.go | 15 +-- 4 files changed, 91 insertions(+), 58 deletions(-) diff --git a/cmd/cloud-controller-manager/main.go b/cmd/cloud-controller-manager/main.go index 227897e2004..f47e009b6ee 100644 --- a/cmd/cloud-controller-manager/main.go +++ b/cmd/cloud-controller-manager/main.go @@ -31,6 +31,7 @@ import ( "github.com/spf13/pflag" + "k8s.io/apimachinery/pkg/util/wait" "k8s.io/cloud-provider" "k8s.io/cloud-provider/app" cloudcontrollerconfig "k8s.io/cloud-provider/app/config" @@ -44,11 +45,6 @@ import ( // e.g. _"k8s.io/legacy-cloud-providers/" ) -const ( - // cloudProviderName shows an sample of using hard coded parameter, please edit the value for your case. - cloudProviderName = "SampleCloudProviderName" -) - func main() { rand.Seed(time.Now().UnixNano()) @@ -67,7 +63,7 @@ func main() { // If you do not need additional controller, please ignore. controllerInitializers["nodeipam"] = startNodeIpamControllerWrapper - command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, controllerInitializers) + command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, controllerInitializers, wait.NeverStop) // TODO: once we switch everything over to Cobra commands, we can go back to calling // utilflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the @@ -84,9 +80,9 @@ func main() { } func cloudInitializer(config *cloudcontrollerconfig.CompletedConfig) cloudprovider.Interface { - cloudConfigFile := config.ComponentConfig.KubeCloudShared.CloudProvider.CloudConfigFile + cloudConfig := config.ComponentConfig.KubeCloudShared.CloudProvider // initialize cloud provider with the cloud provider name and config file provided - cloud, err := cloudprovider.InitCloudProvider(cloudProviderName, cloudConfigFile) + cloud, err := cloudprovider.InitCloudProvider(cloudConfig.Name, cloudConfig.CloudConfigFile) if err != nil { klog.Fatalf("Cloud provider could not be initialized: %v", err) } diff --git a/staging/src/k8s.io/cloud-provider/app/controllermanager.go b/staging/src/k8s.io/cloud-provider/app/controllermanager.go index 3f7d18ed15e..6e09069889d 100644 --- a/staging/src/k8s.io/cloud-provider/app/controllermanager.go +++ b/staging/src/k8s.io/cloud-provider/app/controllermanager.go @@ -64,30 +64,33 @@ const ( ) // NewCloudControllerManagerCommand creates a *cobra.Command object with default parameters -func NewCloudControllerManagerCommand(s *options.CloudControllerManagerOptions, cloudInitializer InitCloudFunc, initFuncConstructor map[string]InitFuncConstructor) *cobra.Command { +func NewCloudControllerManagerCommand(s *options.CloudControllerManagerOptions, cloudInitializer InitCloudFunc, initFuncConstructor map[string]InitFuncConstructor, stopCh <-chan struct{}) *cobra.Command { cmd := &cobra.Command{ Use: "cloud-controller-manager", Long: `The Cloud controller manager is a daemon that embeds the cloud specific control loops shipped with Kubernetes.`, - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { verflag.PrintAndExitIfRequested() cliflag.PrintFlags(cmd.Flags()) c, err := s.Config(ControllerNames(initFuncConstructor), ControllersDisabledByDefault.List()) if err != nil { fmt.Fprintf(os.Stderr, "%v\n", err) - os.Exit(1) + return err + //os.Exit(1) } completedConfig := c.Complete() cloud := cloudInitializer(completedConfig) controllerInitializers := ConstructControllerInitializers(initFuncConstructor, completedConfig, cloud) - if err := Run(completedConfig, cloud, controllerInitializers, wait.NeverStop); err != nil { + if err := Run(completedConfig, cloud, controllerInitializers, stopCh); err != nil { fmt.Fprintf(os.Stderr, "%v\n", err) - os.Exit(1) + return err + //os.Exit(1) } + return nil }, Args: func(cmd *cobra.Command, args []string) error { for _, arg := range args { diff --git a/staging/src/k8s.io/cloud-provider/app/testing/testserver.go b/staging/src/k8s.io/cloud-provider/app/testing/testserver.go index daf22909bd6..1a5f2669c8f 100644 --- a/staging/src/k8s.io/cloud-provider/app/testing/testserver.go +++ b/staging/src/k8s.io/cloud-provider/app/testing/testserver.go @@ -22,17 +22,18 @@ import ( "io/ioutil" "net" "os" + "strings" "time" "github.com/spf13/pflag" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/kubernetes" + restclient "k8s.io/client-go/rest" cloudprovider "k8s.io/cloud-provider" "k8s.io/cloud-provider/app" "k8s.io/cloud-provider/app/config" "k8s.io/cloud-provider/options" - - "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/client-go/kubernetes" - restclient "k8s.io/client-go/rest" + "k8s.io/component-base/cli/flag" ) // TearDownFunc is to be called to tear down a test server. @@ -42,7 +43,7 @@ type TearDownFunc func() type TestServer struct { LoopbackClientConfig *restclient.Config // Rest client config using the magic token Options *options.CloudControllerManagerOptions - Config *config.Config + Config *config.CompletedConfig TearDownFn TearDownFunc // TearDown function TmpDir string // Temp Dir used, by the apiserver } @@ -62,6 +63,8 @@ type Logger interface { // enough time to remove temporary files. func StartTestServer(t Logger, customFlags []string) (result TestServer, err error) { stopCh := make(chan struct{}) + configDoneCh := make(chan string) + var capturedConfig config.CompletedConfig tearDown := func() { close(stopCh) if len(result.TmpDir) != 0 { @@ -79,58 +82,94 @@ func StartTestServer(t Logger, customFlags []string) (result TestServer, err err return result, fmt.Errorf("failed to create temp dir: %v", err) } - fs := pflag.NewFlagSet("test", pflag.PanicOnError) - s, err := options.NewCloudControllerManagerOptions() if err != nil { return TestServer{}, err } - namedFlagSets := s.Flags([]string{}, []string{}) - for _, f := range namedFlagSets.FlagSets { - fs.AddFlagSet(f) - } - fs.Parse(customFlags) - if s.SecureServing.BindPort != 0 { - s.SecureServing.Listener, s.SecureServing.BindPort, err = createListenerOnFreePort() + + cloudInitializer := func(config *config.CompletedConfig) cloudprovider.Interface { + capturedConfig = *config + configDoneCh <- "" + close(configDoneCh) + cloudConfig := config.ComponentConfig.KubeCloudShared.CloudProvider + cloud, err := cloudprovider.InitCloudProvider(cloudConfig.Name, cloudConfig.CloudConfigFile) if err != nil { - return result, fmt.Errorf("failed to create listener: %v", err) + t.Fatalf("Cloud provider could not be initialized: %v", err) } s.SecureServing.ServerCert.CertDirectory = result.TmpDir + if cloud == nil { + t.Fatalf("Cloud provider is nil") + } + return cloud + } + command := app.NewCloudControllerManagerCommand(s, cloudInitializer, app.DefaultInitFuncConstructors, stopCh) + pflag.CommandLine.SetNormalizeFunc(flag.WordSepNormalizeFunc) - t.Logf("cloud-controller-manager will listen securely on port %d...", s.SecureServing.BindPort) + commandArgs := []string{} + listeners := []net.Listener{} + disableInsecure := false + disableSecure := false + for _, arg := range customFlags { + if strings.HasPrefix(arg, "--secure-port=") { + if arg == "--secure-port=0" { + commandArgs = append(commandArgs, arg) + disableSecure = true + } + } else if strings.HasPrefix(arg, "--port=") { + if arg == "--port=0" { + commandArgs = append(commandArgs, arg) + disableInsecure = true + } + } else if strings.HasPrefix(arg, "--cert-dir=") { + // skip it + } else { + commandArgs = append(commandArgs, arg) + } } - if s.InsecureServing.BindPort != 0 { - s.InsecureServing.Listener, s.InsecureServing.BindPort, err = createListenerOnFreePort() + if !disableSecure { + listener, bindPort, err := createListenerOnFreePort() if err != nil { return result, fmt.Errorf("failed to create listener: %v", err) } + listeners = append(listeners, listener) + commandArgs = append(commandArgs, fmt.Sprintf("--secure-port=%d", bindPort)) + commandArgs = append(commandArgs, fmt.Sprintf("--cert-dir=%s", result.TmpDir)) - t.Logf("cloud-controller-manager will listen insecurely on port %d...", s.InsecureServing.BindPort) + t.Logf("cloud-controller-manager will listen securely on port %d...", bindPort) } + if !disableInsecure { + listener, bindPort, err := createListenerOnFreePort() + if err != nil { + return result, fmt.Errorf("failed to create listener: %v", err) + } + listeners = append(listeners, listener) + commandArgs = append(commandArgs, fmt.Sprintf("--port=%d", bindPort)) - config, err := s.Config([]string{}, []string{}) - if err != nil { - return result, fmt.Errorf("failed to create config from options: %v", err) + t.Logf("cloud-controller-manager will listen securely on port %d...", bindPort) } - cloudConfig := config.Complete().ComponentConfig.KubeCloudShared.CloudProvider - cloud, err := cloudprovider.InitCloudProvider(cloudConfig.Name, cloudConfig.CloudConfigFile) - if err != nil { - return result, fmt.Errorf("cloud provider could not be initialized: %v", err) - } - if cloud == nil { - return result, fmt.Errorf("cloud provider is nil") + for _, listener := range listeners { + listener.Close() } errCh := make(chan error) - go func(stopCh <-chan struct{}) { - if err := app.Run(config.Complete(), cloud, app.ConstructControllerInitializers(app.DefaultInitFuncConstructors, config.Complete(), cloud), stopCh); err != nil { + go func() { + command.SetArgs(commandArgs) + if err := command.Execute(); err != nil { errCh <- err } - }(stopCh) + close(errCh) + }() + + select { + case <-configDoneCh: + + case err := <-errCh: + return result, err + } t.Logf("Waiting for /healthz to be ok...") - client, err := kubernetes.NewForConfig(config.LoopbackClientConfig) + client, err := kubernetes.NewForConfig(capturedConfig.LoopbackClientConfig) if err != nil { return result, fmt.Errorf("failed to create a client: %v", err) } @@ -154,9 +193,9 @@ func StartTestServer(t Logger, customFlags []string) (result TestServer, err err } // from here the caller must call tearDown - result.LoopbackClientConfig = config.LoopbackClientConfig + result.LoopbackClientConfig = capturedConfig.LoopbackClientConfig result.Options = s - result.Config = config + result.Config = &capturedConfig result.TearDownFn = tearDown return result, nil diff --git a/staging/src/k8s.io/cloud-provider/sample/basic_main.go b/staging/src/k8s.io/cloud-provider/sample/basic_main.go index 573f1fd6b35..e5749ae8121 100644 --- a/staging/src/k8s.io/cloud-provider/sample/basic_main.go +++ b/staging/src/k8s.io/cloud-provider/sample/basic_main.go @@ -21,6 +21,7 @@ limitations under the License. package main import ( + "k8s.io/apimachinery/pkg/util/wait" "math/rand" "os" "time" @@ -39,13 +40,6 @@ import ( // e.g. _"k8s.io/legacy-cloud-providers/" ) -const ( - // The variables below are samples, please edit the value for your case. - - // sampleCloudProviderName shows an sample of using hard coded parameter for CloudProviderName - sampleCloudProviderName = "SampleCloudProviderName" -) - func main() { rand.Seed(time.Now().UnixNano()) @@ -54,7 +48,7 @@ func main() { klog.Fatalf("unable to initialize command options: %v", err) } - command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, app.DefaultInitFuncConstructors) + command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, app.DefaultInitFuncConstructors, wait.NeverStop) // TODO: once we switch everything over to Cobra commands, we can go back to calling // utilflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the @@ -71,9 +65,10 @@ func main() { } func cloudInitializer(config *config.CompletedConfig) cloudprovider.Interface { - cloudConfigFile := config.ComponentConfig.KubeCloudShared.CloudProvider.CloudConfigFile + cloudConfig := config.ComponentConfig.KubeCloudShared.CloudProvider + // initialize cloud provider with the cloud provider name and config file provided - cloud, err := cloudprovider.InitCloudProvider(sampleCloudProviderName, cloudConfigFile) + cloud, err := cloudprovider.InitCloudProvider(cloudConfig.Name, cloudConfig.CloudConfigFile) if err != nil { klog.Fatalf("Cloud provider could not be initialized: %v", err) } From 9849079d4eeed380499e71355808fa540e57c2e3 Mon Sep 17 00:00:00 2001 From: cici37 Date: Wed, 17 Feb 2021 16:39:03 -0800 Subject: [PATCH 10/16] Address review comments --- cmd/cloud-controller-manager/nodeipamcontroller.go | 2 +- staging/src/k8s.io/cloud-provider/app/controllermanager.go | 2 -- staging/src/k8s.io/cloud-provider/app/testing/testserver.go | 1 + 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/cmd/cloud-controller-manager/nodeipamcontroller.go b/cmd/cloud-controller-manager/nodeipamcontroller.go index 3d6d928db60..a39c40458cd 100644 --- a/cmd/cloud-controller-manager/nodeipamcontroller.go +++ b/cmd/cloud-controller-manager/nodeipamcontroller.go @@ -55,7 +55,7 @@ func startNodeIpamControllerWrapper(completedConfig *cloudcontrollerconfig.Compl nodeIPAMControllerOptions.AddFlags(fs) errors := nodeIPAMControllerOptions.Validate() if len(errors) > 0 { - klog.Fatal("NodeIPAM controller values are not properly.") + klog.Fatal("NodeIPAM controller values are not properly set.") } var nodeIPAMConfig nodeipamconfig.NodeIPAMControllerConfiguration nodeIPAMControllerOptions.ApplyTo(&nodeIPAMConfig) diff --git a/staging/src/k8s.io/cloud-provider/app/controllermanager.go b/staging/src/k8s.io/cloud-provider/app/controllermanager.go index 6e09069889d..66da8a259f0 100644 --- a/staging/src/k8s.io/cloud-provider/app/controllermanager.go +++ b/staging/src/k8s.io/cloud-provider/app/controllermanager.go @@ -78,7 +78,6 @@ the cloud specific control loops shipped with Kubernetes.`, if err != nil { fmt.Fprintf(os.Stderr, "%v\n", err) return err - //os.Exit(1) } completedConfig := c.Complete() @@ -88,7 +87,6 @@ the cloud specific control loops shipped with Kubernetes.`, if err := Run(completedConfig, cloud, controllerInitializers, stopCh); err != nil { fmt.Fprintf(os.Stderr, "%v\n", err) return err - //os.Exit(1) } return nil }, diff --git a/staging/src/k8s.io/cloud-provider/app/testing/testserver.go b/staging/src/k8s.io/cloud-provider/app/testing/testserver.go index 1a5f2669c8f..cb5923a5f42 100644 --- a/staging/src/k8s.io/cloud-provider/app/testing/testserver.go +++ b/staging/src/k8s.io/cloud-provider/app/testing/testserver.go @@ -89,6 +89,7 @@ func StartTestServer(t Logger, customFlags []string) (result TestServer, err err cloudInitializer := func(config *config.CompletedConfig) cloudprovider.Interface { capturedConfig = *config + // send signal to indicate the capturedConfig has been properly set configDoneCh <- "" close(configDoneCh) cloudConfig := config.ComponentConfig.KubeCloudShared.CloudProvider From 781d7cf134dbbd0112876786a77fe36df4d86441 Mon Sep 17 00:00:00 2001 From: cici37 Date: Thu, 18 Feb 2021 14:45:44 -0800 Subject: [PATCH 11/16] Address review comments --- staging/src/k8s.io/cloud-provider/app/controllermanager.go | 1 + staging/src/k8s.io/cloud-provider/app/testing/testserver.go | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/staging/src/k8s.io/cloud-provider/app/controllermanager.go b/staging/src/k8s.io/cloud-provider/app/controllermanager.go index 66da8a259f0..f848c6ac82c 100644 --- a/staging/src/k8s.io/cloud-provider/app/controllermanager.go +++ b/staging/src/k8s.io/cloud-provider/app/controllermanager.go @@ -64,6 +64,7 @@ const ( ) // NewCloudControllerManagerCommand creates a *cobra.Command object with default parameters +// initFuncConstructor is a map of named controller groups (you can start more than one in an init func) paired to their InitFuncConstructor. func NewCloudControllerManagerCommand(s *options.CloudControllerManagerOptions, cloudInitializer InitCloudFunc, initFuncConstructor map[string]InitFuncConstructor, stopCh <-chan struct{}) *cobra.Command { cmd := &cobra.Command{ diff --git a/staging/src/k8s.io/cloud-provider/app/testing/testserver.go b/staging/src/k8s.io/cloud-provider/app/testing/testserver.go index cb5923a5f42..685d9b11bdb 100644 --- a/staging/src/k8s.io/cloud-provider/app/testing/testserver.go +++ b/staging/src/k8s.io/cloud-provider/app/testing/testserver.go @@ -63,7 +63,7 @@ type Logger interface { // enough time to remove temporary files. func StartTestServer(t Logger, customFlags []string) (result TestServer, err error) { stopCh := make(chan struct{}) - configDoneCh := make(chan string) + configDoneCh := make(chan struct{}) var capturedConfig config.CompletedConfig tearDown := func() { close(stopCh) @@ -90,7 +90,6 @@ func StartTestServer(t Logger, customFlags []string) (result TestServer, err err cloudInitializer := func(config *config.CompletedConfig) cloudprovider.Interface { capturedConfig = *config // send signal to indicate the capturedConfig has been properly set - configDoneCh <- "" close(configDoneCh) cloudConfig := config.ComponentConfig.KubeCloudShared.CloudProvider cloud, err := cloudprovider.InitCloudProvider(cloudConfig.Name, cloudConfig.CloudConfigFile) From 69fee369c96d2d62c08985a364ea9ca81b85c550 Mon Sep 17 00:00:00 2001 From: cici37 Date: Sun, 28 Feb 2021 01:50:52 -0800 Subject: [PATCH 12/16] Update extension mechanism and related sample. --- cmd/cloud-controller-manager/main.go | 12 ++++++++++-- .../k8s.io/cloud-provider/app/controllermanager.go | 6 +++++- .../k8s.io/cloud-provider/app/testing/testserver.go | 2 +- .../src/k8s.io/cloud-provider/sample/basic_main.go | 2 +- 4 files changed, 17 insertions(+), 5 deletions(-) diff --git a/cmd/cloud-controller-manager/main.go b/cmd/cloud-controller-manager/main.go index f47e009b6ee..cf366935dad 100644 --- a/cmd/cloud-controller-manager/main.go +++ b/cmd/cloud-controller-manager/main.go @@ -41,10 +41,15 @@ import ( _ "k8s.io/component-base/metrics/prometheus/clientgo" // load all the prometheus client-go plugins _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration "k8s.io/klog/v2" + nodeipamcontrolleroptions "k8s.io/kubernetes/cmd/kube-controller-manager/app/options" + nodeipamconfig "k8s.io/kubernetes/pkg/controller/nodeipam/config" // For existing cloud providers, the option to import legacy providers is still available. // e.g. _"k8s.io/legacy-cloud-providers/" ) +var nodeIPAMControllerConfiguration nodeipamconfig.NodeIPAMControllerConfiguration +var nodeIPAMControllerOptions nodeipamcontrolleroptions.NodeIPAMControllerOptions + func main() { rand.Seed(time.Now().UnixNano()) @@ -59,11 +64,14 @@ func main() { //delete(controllerInitializers, "cloud-node-lifecycle") // Here is an example to add an controller(NodeIpamController) which will be used by cloud provider - // generate nodeIPAMConfig. Here is an sample code. Please pass the right parameter in your code. + // generate nodeIPAMConfig. Here is an sample code. // If you do not need additional controller, please ignore. + nodeIPAMControllerOptions.NodeIPAMControllerConfiguration = &nodeIPAMControllerConfiguration + fs := pflag.NewFlagSet("fs", pflag.ContinueOnError) + nodeIPAMControllerOptions.AddFlags(fs) controllerInitializers["nodeipam"] = startNodeIpamControllerWrapper - command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, controllerInitializers, wait.NeverStop) + command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, controllerInitializers, fs, wait.NeverStop) // TODO: once we switch everything over to Cobra commands, we can go back to calling // utilflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the diff --git a/staging/src/k8s.io/cloud-provider/app/controllermanager.go b/staging/src/k8s.io/cloud-provider/app/controllermanager.go index f848c6ac82c..42c0cc94353 100644 --- a/staging/src/k8s.io/cloud-provider/app/controllermanager.go +++ b/staging/src/k8s.io/cloud-provider/app/controllermanager.go @@ -26,6 +26,7 @@ import ( "time" "github.com/spf13/cobra" + "github.com/spf13/pflag" "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" cacheddiscovery "k8s.io/client-go/discovery/cached" @@ -65,7 +66,7 @@ const ( // NewCloudControllerManagerCommand creates a *cobra.Command object with default parameters // initFuncConstructor is a map of named controller groups (you can start more than one in an init func) paired to their InitFuncConstructor. -func NewCloudControllerManagerCommand(s *options.CloudControllerManagerOptions, cloudInitializer InitCloudFunc, initFuncConstructor map[string]InitFuncConstructor, stopCh <-chan struct{}) *cobra.Command { +func NewCloudControllerManagerCommand(s *options.CloudControllerManagerOptions, cloudInitializer InitCloudFunc, initFuncConstructor map[string]InitFuncConstructor, additionalFlags *pflag.FlagSet, stopCh <-chan struct{}) *cobra.Command { cmd := &cobra.Command{ Use: "cloud-controller-manager", @@ -117,6 +118,9 @@ the cloud specific control loops shipped with Kubernetes.`, for _, f := range namedFlagSets.FlagSets { fs.AddFlagSet(f) } + if additionalFlags != nil { + fs.AddFlagSet(additionalFlags) + } usageFmt := "Usage:\n %s\n" cols, _, _ := term.TerminalSize(cmd.OutOrStdout()) cmd.SetUsageFunc(func(cmd *cobra.Command) error { diff --git a/staging/src/k8s.io/cloud-provider/app/testing/testserver.go b/staging/src/k8s.io/cloud-provider/app/testing/testserver.go index 685d9b11bdb..3f7a53656c5 100644 --- a/staging/src/k8s.io/cloud-provider/app/testing/testserver.go +++ b/staging/src/k8s.io/cloud-provider/app/testing/testserver.go @@ -102,7 +102,7 @@ func StartTestServer(t Logger, customFlags []string) (result TestServer, err err } return cloud } - command := app.NewCloudControllerManagerCommand(s, cloudInitializer, app.DefaultInitFuncConstructors, stopCh) + command := app.NewCloudControllerManagerCommand(s, cloudInitializer, app.DefaultInitFuncConstructors, nil, stopCh) pflag.CommandLine.SetNormalizeFunc(flag.WordSepNormalizeFunc) commandArgs := []string{} diff --git a/staging/src/k8s.io/cloud-provider/sample/basic_main.go b/staging/src/k8s.io/cloud-provider/sample/basic_main.go index e5749ae8121..ee9f41ae686 100644 --- a/staging/src/k8s.io/cloud-provider/sample/basic_main.go +++ b/staging/src/k8s.io/cloud-provider/sample/basic_main.go @@ -48,7 +48,7 @@ func main() { klog.Fatalf("unable to initialize command options: %v", err) } - command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, app.DefaultInitFuncConstructors, wait.NeverStop) + command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, app.DefaultInitFuncConstructors, nil, wait.NeverStop) // TODO: once we switch everything over to Cobra commands, we can go back to calling // utilflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the From f098e66cee5eefd0ecfef5af71e1fc3a52402de5 Mon Sep 17 00:00:00 2001 From: cici37 Date: Mon, 1 Mar 2021 12:24:37 -0800 Subject: [PATCH 13/16] Delete build file based on latest changes. --- staging/src/k8s.io/cloud-provider/sample/BUILD | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 staging/src/k8s.io/cloud-provider/sample/BUILD diff --git a/staging/src/k8s.io/cloud-provider/sample/BUILD b/staging/src/k8s.io/cloud-provider/sample/BUILD deleted file mode 100644 index e69de29bb2d..00000000000 From 716122ccecb53bb719ba8b6f4d2376fdba820536 Mon Sep 17 00:00:00 2001 From: cici37 Date: Mon, 1 Mar 2021 13:19:27 -0800 Subject: [PATCH 14/16] Update NodeIPAM wrapper --- cmd/cloud-controller-manager/main.go | 5 ----- .../nodeipamcontroller.go | 15 ++++++--------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/cmd/cloud-controller-manager/main.go b/cmd/cloud-controller-manager/main.go index cf366935dad..4a311399192 100644 --- a/cmd/cloud-controller-manager/main.go +++ b/cmd/cloud-controller-manager/main.go @@ -41,15 +41,10 @@ import ( _ "k8s.io/component-base/metrics/prometheus/clientgo" // load all the prometheus client-go plugins _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration "k8s.io/klog/v2" - nodeipamcontrolleroptions "k8s.io/kubernetes/cmd/kube-controller-manager/app/options" - nodeipamconfig "k8s.io/kubernetes/pkg/controller/nodeipam/config" // For existing cloud providers, the option to import legacy providers is still available. // e.g. _"k8s.io/legacy-cloud-providers/" ) -var nodeIPAMControllerConfiguration nodeipamconfig.NodeIPAMControllerConfiguration -var nodeIPAMControllerOptions nodeipamcontrolleroptions.NodeIPAMControllerOptions - func main() { rand.Seed(time.Now().UnixNano()) diff --git a/cmd/cloud-controller-manager/nodeipamcontroller.go b/cmd/cloud-controller-manager/nodeipamcontroller.go index a39c40458cd..a99a47672c9 100644 --- a/cmd/cloud-controller-manager/nodeipamcontroller.go +++ b/cmd/cloud-controller-manager/nodeipamcontroller.go @@ -22,8 +22,7 @@ package main import ( "errors" "fmt" - "github.com/spf13/pflag" - + nodeipamcontrolleroptions "k8s.io/kubernetes/cmd/kube-controller-manager/app/options" "net" "net/http" "strings" @@ -35,7 +34,6 @@ import ( genericcontrollermanager "k8s.io/controller-manager/app" "k8s.io/controller-manager/pkg/features" "k8s.io/klog/v2" - nodeipamcontrolleroptions "k8s.io/kubernetes/cmd/kube-controller-manager/app/options" nodeipamcontroller "k8s.io/kubernetes/pkg/controller/nodeipam" nodeipamconfig "k8s.io/kubernetes/pkg/controller/nodeipam/config" "k8s.io/kubernetes/pkg/controller/nodeipam/ipam" @@ -49,19 +47,18 @@ const ( defaultNodeMaskCIDRIPv6 = 64 ) +var nodeIPAMControllerConfiguration nodeipamconfig.NodeIPAMControllerConfiguration +var nodeIPAMControllerOptions nodeipamcontrolleroptions.NodeIPAMControllerOptions + func startNodeIpamControllerWrapper(completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) app.InitFunc { - fs := pflag.NewFlagSet("fs", pflag.ContinueOnError) - var nodeIPAMControllerOptions nodeipamcontrolleroptions.NodeIPAMControllerOptions - nodeIPAMControllerOptions.AddFlags(fs) errors := nodeIPAMControllerOptions.Validate() if len(errors) > 0 { klog.Fatal("NodeIPAM controller values are not properly set.") } - var nodeIPAMConfig nodeipamconfig.NodeIPAMControllerConfiguration - nodeIPAMControllerOptions.ApplyTo(&nodeIPAMConfig) + nodeIPAMControllerOptions.ApplyTo(&nodeIPAMControllerConfiguration) return func(ctx genericcontrollermanager.ControllerContext) (http.Handler, bool, error) { - return startNodeIpamController(completedConfig, nodeIPAMConfig, ctx, cloud) + return startNodeIpamController(completedConfig, nodeIPAMControllerConfiguration, ctx, cloud) } } From 9b3d42f20af74a1e664aad80595998bdee275bd6 Mon Sep 17 00:00:00 2001 From: cici37 Date: Mon, 1 Mar 2021 16:14:24 -0800 Subject: [PATCH 15/16] Address comments. --- cmd/cloud-controller-manager/main.go | 8 +++++--- .../nodeipamcontroller.go | 16 +++++++++------- .../cloud-provider/app/controllermanager.go | 1 + 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/cmd/cloud-controller-manager/main.go b/cmd/cloud-controller-manager/main.go index 4a311399192..f4950450728 100644 --- a/cmd/cloud-controller-manager/main.go +++ b/cmd/cloud-controller-manager/main.go @@ -61,10 +61,12 @@ func main() { // Here is an example to add an controller(NodeIpamController) which will be used by cloud provider // generate nodeIPAMConfig. Here is an sample code. // If you do not need additional controller, please ignore. - nodeIPAMControllerOptions.NodeIPAMControllerConfiguration = &nodeIPAMControllerConfiguration + + nodeIpamController := nodeIPAMController{} + nodeIpamController.nodeIPAMControllerOptions.NodeIPAMControllerConfiguration = &nodeIpamController.nodeIPAMControllerConfiguration fs := pflag.NewFlagSet("fs", pflag.ContinueOnError) - nodeIPAMControllerOptions.AddFlags(fs) - controllerInitializers["nodeipam"] = startNodeIpamControllerWrapper + nodeIpamController.nodeIPAMControllerOptions.AddFlags(fs) + controllerInitializers["nodeipam"] = nodeIpamController.startNodeIpamControllerWrapper command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, controllerInitializers, fs, wait.NeverStop) diff --git a/cmd/cloud-controller-manager/nodeipamcontroller.go b/cmd/cloud-controller-manager/nodeipamcontroller.go index a99a47672c9..ebceef0629f 100644 --- a/cmd/cloud-controller-manager/nodeipamcontroller.go +++ b/cmd/cloud-controller-manager/nodeipamcontroller.go @@ -22,7 +22,6 @@ package main import ( "errors" "fmt" - nodeipamcontrolleroptions "k8s.io/kubernetes/cmd/kube-controller-manager/app/options" "net" "net/http" "strings" @@ -34,6 +33,7 @@ import ( genericcontrollermanager "k8s.io/controller-manager/app" "k8s.io/controller-manager/pkg/features" "k8s.io/klog/v2" + nodeipamcontrolleroptions "k8s.io/kubernetes/cmd/kube-controller-manager/app/options" nodeipamcontroller "k8s.io/kubernetes/pkg/controller/nodeipam" nodeipamconfig "k8s.io/kubernetes/pkg/controller/nodeipam/config" "k8s.io/kubernetes/pkg/controller/nodeipam/ipam" @@ -47,18 +47,20 @@ const ( defaultNodeMaskCIDRIPv6 = 64 ) -var nodeIPAMControllerConfiguration nodeipamconfig.NodeIPAMControllerConfiguration -var nodeIPAMControllerOptions nodeipamcontrolleroptions.NodeIPAMControllerOptions +type nodeIPAMController struct { + nodeIPAMControllerConfiguration nodeipamconfig.NodeIPAMControllerConfiguration + nodeIPAMControllerOptions nodeipamcontrolleroptions.NodeIPAMControllerOptions +} -func startNodeIpamControllerWrapper(completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) app.InitFunc { - errors := nodeIPAMControllerOptions.Validate() +func (nodeIpamController *nodeIPAMController) startNodeIpamControllerWrapper(completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) app.InitFunc { + errors := nodeIpamController.nodeIPAMControllerOptions.Validate() if len(errors) > 0 { klog.Fatal("NodeIPAM controller values are not properly set.") } - nodeIPAMControllerOptions.ApplyTo(&nodeIPAMControllerConfiguration) + nodeIpamController.nodeIPAMControllerOptions.ApplyTo(&nodeIpamController.nodeIPAMControllerConfiguration) return func(ctx genericcontrollermanager.ControllerContext) (http.Handler, bool, error) { - return startNodeIpamController(completedConfig, nodeIPAMControllerConfiguration, ctx, cloud) + return startNodeIpamController(completedConfig, nodeIpamController.nodeIPAMControllerConfiguration, ctx, cloud) } } diff --git a/staging/src/k8s.io/cloud-provider/app/controllermanager.go b/staging/src/k8s.io/cloud-provider/app/controllermanager.go index 42c0cc94353..d7619117fb6 100644 --- a/staging/src/k8s.io/cloud-provider/app/controllermanager.go +++ b/staging/src/k8s.io/cloud-provider/app/controllermanager.go @@ -66,6 +66,7 @@ const ( // NewCloudControllerManagerCommand creates a *cobra.Command object with default parameters // initFuncConstructor is a map of named controller groups (you can start more than one in an init func) paired to their InitFuncConstructor. +// additionalFlags provides controller specific flags to be included in the complete set of controller manager flags func NewCloudControllerManagerCommand(s *options.CloudControllerManagerOptions, cloudInitializer InitCloudFunc, initFuncConstructor map[string]InitFuncConstructor, additionalFlags *pflag.FlagSet, stopCh <-chan struct{}) *cobra.Command { cmd := &cobra.Command{ From 408258c6d5ae07610eca39f991ee81853350c1b8 Mon Sep 17 00:00:00 2001 From: cici37 Date: Tue, 2 Mar 2021 10:26:51 -0800 Subject: [PATCH 16/16] Update to use cliflag.NamedFlagSets --- cmd/cloud-controller-manager/main.go | 10 +++---- .../cloud-provider/app/controllermanager.go | 30 +++++++++---------- .../cloud-provider/app/testing/testserver.go | 7 +++-- .../cloud-provider/sample/basic_main.go | 9 +++--- 4 files changed, 28 insertions(+), 28 deletions(-) diff --git a/cmd/cloud-controller-manager/main.go b/cmd/cloud-controller-manager/main.go index f4950450728..23ec3eb65f6 100644 --- a/cmd/cloud-controller-manager/main.go +++ b/cmd/cloud-controller-manager/main.go @@ -36,7 +36,7 @@ import ( "k8s.io/cloud-provider/app" cloudcontrollerconfig "k8s.io/cloud-provider/app/config" "k8s.io/cloud-provider/options" - "k8s.io/component-base/cli/flag" + cliflag "k8s.io/component-base/cli/flag" "k8s.io/component-base/logs" _ "k8s.io/component-base/metrics/prometheus/clientgo" // load all the prometheus client-go plugins _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration @@ -64,17 +64,17 @@ func main() { nodeIpamController := nodeIPAMController{} nodeIpamController.nodeIPAMControllerOptions.NodeIPAMControllerConfiguration = &nodeIpamController.nodeIPAMControllerConfiguration - fs := pflag.NewFlagSet("fs", pflag.ContinueOnError) - nodeIpamController.nodeIPAMControllerOptions.AddFlags(fs) + fss := cliflag.NamedFlagSets{} + nodeIpamController.nodeIPAMControllerOptions.AddFlags(fss.FlagSet("nodeipam controller")) controllerInitializers["nodeipam"] = nodeIpamController.startNodeIpamControllerWrapper - command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, controllerInitializers, fs, wait.NeverStop) + command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, controllerInitializers, fss, wait.NeverStop) // TODO: once we switch everything over to Cobra commands, we can go back to calling // utilflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the // normalize func and add the go flag set by hand. // Here is an sample - pflag.CommandLine.SetNormalizeFunc(flag.WordSepNormalizeFunc) + pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) // utilflag.InitFlags() logs.InitLogs() defer logs.FlushLogs() diff --git a/staging/src/k8s.io/cloud-provider/app/controllermanager.go b/staging/src/k8s.io/cloud-provider/app/controllermanager.go index d7619117fb6..4acefc386a8 100644 --- a/staging/src/k8s.io/cloud-provider/app/controllermanager.go +++ b/staging/src/k8s.io/cloud-provider/app/controllermanager.go @@ -26,27 +26,23 @@ import ( "time" "github.com/spf13/cobra" - "github.com/spf13/pflag" "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" - cacheddiscovery "k8s.io/client-go/discovery/cached" - "k8s.io/client-go/informers" - "k8s.io/client-go/metadata" - "k8s.io/client-go/metadata/metadatainformer" - "k8s.io/client-go/restmapper" - cloudprovider "k8s.io/cloud-provider" - cloudcontrollerconfig "k8s.io/cloud-provider/app/config" - "k8s.io/cloud-provider/options" - "k8s.io/controller-manager/pkg/clientbuilder" - "k8s.io/controller-manager/pkg/informerfactory" - "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/uuid" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/server" "k8s.io/apiserver/pkg/server/healthz" + cacheddiscovery "k8s.io/client-go/discovery/cached" + "k8s.io/client-go/informers" + "k8s.io/client-go/metadata" + "k8s.io/client-go/metadata/metadatainformer" + "k8s.io/client-go/restmapper" "k8s.io/client-go/tools/leaderelection" "k8s.io/client-go/tools/leaderelection/resourcelock" + cloudprovider "k8s.io/cloud-provider" + cloudcontrollerconfig "k8s.io/cloud-provider/app/config" + "k8s.io/cloud-provider/options" cliflag "k8s.io/component-base/cli/flag" "k8s.io/component-base/cli/globalflag" "k8s.io/component-base/configz" @@ -54,6 +50,8 @@ import ( "k8s.io/component-base/version" "k8s.io/component-base/version/verflag" genericcontrollermanager "k8s.io/controller-manager/app" + "k8s.io/controller-manager/pkg/clientbuilder" + "k8s.io/controller-manager/pkg/informerfactory" "k8s.io/klog/v2" ) @@ -67,8 +65,7 @@ const ( // NewCloudControllerManagerCommand creates a *cobra.Command object with default parameters // initFuncConstructor is a map of named controller groups (you can start more than one in an init func) paired to their InitFuncConstructor. // additionalFlags provides controller specific flags to be included in the complete set of controller manager flags -func NewCloudControllerManagerCommand(s *options.CloudControllerManagerOptions, cloudInitializer InitCloudFunc, initFuncConstructor map[string]InitFuncConstructor, additionalFlags *pflag.FlagSet, stopCh <-chan struct{}) *cobra.Command { - +func NewCloudControllerManagerCommand(s *options.CloudControllerManagerOptions, cloudInitializer InitCloudFunc, initFuncConstructor map[string]InitFuncConstructor, additionalFlags cliflag.NamedFlagSets, stopCh <-chan struct{}) *cobra.Command { cmd := &cobra.Command{ Use: "cloud-controller-manager", Long: `The Cloud controller manager is a daemon that embeds @@ -119,9 +116,10 @@ the cloud specific control loops shipped with Kubernetes.`, for _, f := range namedFlagSets.FlagSets { fs.AddFlagSet(f) } - if additionalFlags != nil { - fs.AddFlagSet(additionalFlags) + for _, f := range additionalFlags.FlagSets { + fs.AddFlagSet(f) } + usageFmt := "Usage:\n %s\n" cols, _, _ := term.TerminalSize(cmd.OutOrStdout()) cmd.SetUsageFunc(func(cmd *cobra.Command) error { diff --git a/staging/src/k8s.io/cloud-provider/app/testing/testserver.go b/staging/src/k8s.io/cloud-provider/app/testing/testserver.go index 3f7a53656c5..9e334bf5524 100644 --- a/staging/src/k8s.io/cloud-provider/app/testing/testserver.go +++ b/staging/src/k8s.io/cloud-provider/app/testing/testserver.go @@ -33,7 +33,7 @@ import ( "k8s.io/cloud-provider/app" "k8s.io/cloud-provider/app/config" "k8s.io/cloud-provider/options" - "k8s.io/component-base/cli/flag" + cliflag "k8s.io/component-base/cli/flag" ) // TearDownFunc is to be called to tear down a test server. @@ -102,8 +102,9 @@ func StartTestServer(t Logger, customFlags []string) (result TestServer, err err } return cloud } - command := app.NewCloudControllerManagerCommand(s, cloudInitializer, app.DefaultInitFuncConstructors, nil, stopCh) - pflag.CommandLine.SetNormalizeFunc(flag.WordSepNormalizeFunc) + fss := cliflag.NamedFlagSets{} + command := app.NewCloudControllerManagerCommand(s, cloudInitializer, app.DefaultInitFuncConstructors, fss, stopCh) + pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) commandArgs := []string{} listeners := []net.Listener{} diff --git a/staging/src/k8s.io/cloud-provider/sample/basic_main.go b/staging/src/k8s.io/cloud-provider/sample/basic_main.go index ee9f41ae686..9f6386c5430 100644 --- a/staging/src/k8s.io/cloud-provider/sample/basic_main.go +++ b/staging/src/k8s.io/cloud-provider/sample/basic_main.go @@ -21,17 +21,17 @@ limitations under the License. package main import ( - "k8s.io/apimachinery/pkg/util/wait" "math/rand" "os" "time" "github.com/spf13/pflag" + "k8s.io/apimachinery/pkg/util/wait" "k8s.io/cloud-provider" "k8s.io/cloud-provider/app" "k8s.io/cloud-provider/app/config" "k8s.io/cloud-provider/options" - "k8s.io/component-base/cli/flag" + cliflag "k8s.io/component-base/cli/flag" "k8s.io/component-base/logs" _ "k8s.io/component-base/metrics/prometheus/clientgo" // load all the prometheus client-go plugins _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration @@ -48,13 +48,14 @@ func main() { klog.Fatalf("unable to initialize command options: %v", err) } - command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, app.DefaultInitFuncConstructors, nil, wait.NeverStop) + fss := cliflag.NamedFlagSets{} + command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, app.DefaultInitFuncConstructors, fss, wait.NeverStop) // TODO: once we switch everything over to Cobra commands, we can go back to calling // utilflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the // normalize func and add the go flag set by hand. // Here is an sample - pflag.CommandLine.SetNormalizeFunc(flag.WordSepNormalizeFunc) + pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) // utilflag.InitFlags() logs.InitLogs() defer logs.FlushLogs()