Convert outstanding commands to cobra (#2384)

* magnum-auto-healer: Stop registering klog options

Per [1], this is no longer desirable. We are already registering the
minimal options that the KEP suggests so we can simply stop registering
the others.

[1] https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/2845-deprecate-klog-specific-flags-in-k8s-components

Signed-off-by: Stephen Finucane <[email protected]>

* client-keystone-auth: Stop registering klog options

As with magnum-auto-healer, we don't need/want to do this anymore.
Don't.

Signed-off-by: Stephen Finucane <[email protected]>

* cinder-csi-plugin: Use binary name in help page

Signed-off-by: Stephen Finucane <[email protected]>

* client-keystone-auth: Migrate to cobra

This one is relatively trivial since the 'Run' function in
'k8s.io/component-base/cli' does most of the heavy lifting for us now,
including registering logging arguments.

Signed-off-by: Stephen Finucane <[email protected]>

* k8s-keystone-auth: Migrate to cobra

This one is slightly trickier due to how we're doing configuration but
there's still nothing crazy confusing here.

Signed-off-by: Stephen Finucane <[email protected]>

* occm: Remove unnecessary flag handling code

k8s.io/cloud-provider switched to cobra some time back [1] and cobra
uses 'pflag' rather than 'flag' under the hood. As such, there's no
reason to keep the handling code for 'flag' options around. Remove it.

[1] https://github.com/kubernetes/cloud-provider/blob/v0.28.0/app/controllermanager.go#L87-L124

Signed-off-by: Stephen Finucane <[email protected]>

* occm: Register additional options correctly

The 'NewCloudControllerManagerCommand' function, which generates the cobra
Command that forms the basis of a cloud provider binary, accepts an
'additionalFlags' argument that allows us to (surprise) pass in
additional provider-specific arguments. We were not making use of this,
which means our options were not showing in the usage string shown on
e.g. '--help'. Correct this on our end, while we wait for the fix in
k8s.io/cloud-provider [1] to close the loop fully.

In additional, move our 'InitLogs' call higher up the function to before
our first logging call so that everything is initialised correctly.

[1] kubernetes/kubernetes#120522

Signed-off-by: Stephen Finucane <[email protected]>

* magnum-auto-healer: Remove use of 'init' methods

Instead, register the health checks and cloud provider plugins on
controller start up. This avoids side-effects from merely importing the
modules - which are polluting the output of '--help' - and is generally
"less weird".

To do this, we must make the registration methods part of the public API
and remove the 'pkg/autohealing/cloudprovider/register' package in
favour of a public registration method in the
'pkg/autohealing/cloudprovider/openstack' package.

Signed-off-by: Stephen Finucane <[email protected]>

---------

Signed-off-by: Stephen Finucane <[email protected]>

Author: stephenfin

cmd/cinder-csi-plugin/main.go

@@ -42,7 +42,7 @@ var (
 
 func main() {
 	cmd := &cobra.Command{
-		Use:   "Cinder",
+		Use:   "cinder-csi-plugin",
 		Short: "CSI based Cinder driver",
 		Run: func(cmd *cobra.Command, args []string) {
 			handle()

cmd/client-keystone-auth/main.go

@@ -17,23 +17,20 @@ limitations under the License.
 package main
 
 import (
-	"flag"
 	"fmt"
 	"io"
 	"os"
 	"time"
 
 	"github.com/gophercloud/gophercloud"
 	"github.com/gophercloud/utils/openstack/clientconfig"
-	"github.com/spf13/pflag"
-	"k8s.io/component-base/logs"
+	"github.com/spf13/cobra"
+	"k8s.io/component-base/cli"
 
 	"golang.org/x/term"
 
 	"k8s.io/cloud-provider-openstack/pkg/identity/keystone"
 	"k8s.io/cloud-provider-openstack/pkg/version"
-	kflag "k8s.io/component-base/cli/flag"
-	"k8s.io/klog/v2"
 )
 
 const errRespTemplate string = `{
@@ -137,51 +134,49 @@ func argumentsAreSet(url, user, project, password, domain, applicationCredential
 	return false
 }
 
+var (
+	url                         string
+	domain                      string
+	user                        string
+	project                     string
+	password                    string
+	clientCertPath              string
+	clientKeyPath               string
+	clientCAPath                string
+	options                     keystone.Options
+	err                         error
+	applicationCredentialID     string
+	applicationCredentialName   string
+	applicationCredentialSecret string
+)
+
 func main() {
-	var url string
-	var domain string
-	var user string
-	var project string
-	var password string
-	var clientCertPath string
-	var clientKeyPath string
-	var clientCAPath string
-	var options keystone.Options
-	var err error
-	var applicationCredentialID string
-	var applicationCredentialName string
-	var applicationCredentialSecret string
-	var showVersion bool
-
-	pflag.StringVar(&url, "keystone-url", os.Getenv("OS_AUTH_URL"), "URL for the OpenStack Keystone API")
-	pflag.StringVar(&domain, "domain-name", os.Getenv("OS_DOMAIN_NAME"), "Keystone domain name")
-	pflag.StringVar(&user, "user-name", os.Getenv("OS_USERNAME"), "User name")
-	pflag.StringVar(&project, "project-name", os.Getenv("OS_PROJECT_NAME"), "Keystone project name")
-	pflag.StringVar(&password, "password", os.Getenv("OS_PASSWORD"), "Password")
-	pflag.StringVar(&clientCertPath, "cert", os.Getenv("OS_CERT"), "Client certificate bundle file")
-	pflag.StringVar(&clientKeyPath, "key", os.Getenv("OS_KEY"), "Client certificate key file")
-	pflag.StringVar(&clientCAPath, "cacert", os.Getenv("OS_CACERT"), "Certificate authority file")
-	pflag.StringVar(&applicationCredentialID, "application-credential-id", os.Getenv("OS_APPLICATION_CREDENTIAL_ID"), "Application Credential ID")
-	pflag.StringVar(&applicationCredentialName, "application-credential-name", os.Getenv("OS_APPLICATION_CREDENTIAL_NAME"), "Application Credential Name")
-	pflag.StringVar(&applicationCredentialSecret, "application-credential-secret", os.Getenv("OS_APPLICATION_CREDENTIAL_SECRET"), "Application Credential Secret")
-	pflag.BoolVar(&showVersion, "version", false, "Show current version and exit")
-
-	logs.AddFlags(pflag.CommandLine)
-
-	klogFlags := flag.NewFlagSet("klog", flag.ExitOnError)
-	klog.InitFlags(klogFlags)
-	pflag.CommandLine.AddGoFlagSet(klogFlags)
-
-	kflag.InitFlags()
-
-	if showVersion {
-		fmt.Println(version.Version)
-		os.Exit(0)
+	cmd := &cobra.Command{
+		Use:   "client-keystone-auth",
+		Short: "Keystone client credential plugin for Kubernetes",
+		Run: func(cmd *cobra.Command, args []string) {
+			handle()
+		},
+		Version: version.Version,
 	}
 
-	logs.InitLogs()
-	defer logs.FlushLogs()
+	cmd.PersistentFlags().StringVar(&url, "keystone-url", os.Getenv("OS_AUTH_URL"), "URL for the OpenStack Keystone API")
+	cmd.PersistentFlags().StringVar(&domain, "domain-name", os.Getenv("OS_DOMAIN_NAME"), "Keystone domain name")
+	cmd.PersistentFlags().StringVar(&user, "user-name", os.Getenv("OS_USERNAME"), "User name")
+	cmd.PersistentFlags().StringVar(&project, "project-name", os.Getenv("OS_PROJECT_NAME"), "Keystone project name")
+	cmd.PersistentFlags().StringVar(&password, "password", os.Getenv("OS_PASSWORD"), "Password")
+	cmd.PersistentFlags().StringVar(&clientCertPath, "cert", os.Getenv("OS_CERT"), "Client certificate bundle file")
+	cmd.PersistentFlags().StringVar(&clientKeyPath, "key", os.Getenv("OS_KEY"), "Client certificate key file")
+	cmd.PersistentFlags().StringVar(&clientCAPath, "cacert", os.Getenv("OS_CACERT"), "Certificate authority file")
+	cmd.PersistentFlags().StringVar(&applicationCredentialID, "application-credential-id", os.Getenv("OS_APPLICATION_CREDENTIAL_ID"), "Application Credential ID")
+	cmd.PersistentFlags().StringVar(&applicationCredentialName, "application-credential-name", os.Getenv("OS_APPLICATION_CREDENTIAL_NAME"), "Application Credential Name")
+	cmd.PersistentFlags().StringVar(&applicationCredentialSecret, "application-credential-secret", os.Getenv("OS_APPLICATION_CREDENTIAL_SECRET"), "Application Credential Secret")
+
+	code := cli.Run(cmd)
+	os.Exit(code)
+}
 
+func handle() {
 	// Generate Gophercloud Auth Options based on input data from stdin
 	// if IsTerminal returns "true", or from env variables otherwise.
 	if !term.IsTerminal(int(os.Stdin.Fd())) {

cmd/k8s-keystone-auth/main.go

@@ -15,47 +15,45 @@ limitations under the License.
 package main
 
 import (
-	"fmt"
 	"os"
 
+	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
+	"k8s.io/component-base/cli"
 	"k8s.io/klog/v2"
 
 	"k8s.io/cloud-provider-openstack/pkg/identity/keystone"
 	"k8s.io/cloud-provider-openstack/pkg/version"
-	kflag "k8s.io/component-base/cli/flag"
-	"k8s.io/component-base/logs"
 )
 
+var config = keystone.NewConfig()
+
 func main() {
-	var showVersion bool
-	pflag.BoolVar(&showVersion, "version", false, "Show current version and exit")
+	cmd := &cobra.Command{
+		Use:   "k8s-keystone-auth",
+		Short: "Keystone authentication webhook plugin for Kubernetes",
+		Run: func(cmd *cobra.Command, args []string) {
+			if err := config.ValidateFlags(); err != nil {
+				klog.Errorf("%v", err)
+				os.Exit(1)
+			}
+
+			keystoneAuth, err := keystone.NewKeystoneAuth(config)
+			if err != nil {
+				klog.Errorf("%v", err)
+				os.Exit(1)
+			}
+			keystoneAuth.Run()
+
+		},
+		Version: version.Version,
+	}
 
-	logs.AddFlags(pflag.CommandLine)
 	keystone.AddExtraFlags(pflag.CommandLine)
 
-	logs.InitLogs()
-	defer logs.FlushLogs()
-
-	config := keystone.NewConfig()
 	config.AddFlags(pflag.CommandLine)
 
-	kflag.InitFlags()
-
-	if showVersion {
-		fmt.Println(version.Version)
-		os.Exit(0)
-	}
-
-	if err := config.ValidateFlags(); err != nil {
-		klog.Errorf("%v", err)
-		os.Exit(1)
-	}
+	code := cli.Run(cmd)
+	os.Exit(code)
 
-	keystoneAuth, err := keystone.NewKeystoneAuth(config)
-	if err != nil {
-		klog.Errorf("%v", err)
-		os.Exit(1)
-	}
-	keystoneAuth.Run()
 }

cmd/openstack-cloud-controller-manager/main.go

@@ -20,11 +20,9 @@ limitations under the License.
 package main
 
 import (
-	goflag "flag"
 	"fmt"
 	"os"
 
-	"github.com/spf13/pflag"
 	"k8s.io/apimachinery/pkg/util/wait"
 	cloudprovider "k8s.io/cloud-provider"
 	"k8s.io/cloud-provider/app"
@@ -43,26 +41,18 @@ import (
 )
 
 func main() {
+	logs.InitLogs()
+	defer logs.FlushLogs()
+
 	ccmOptions, err := options.NewCloudControllerManagerOptions()
 	if err != nil {
 		klog.Fatalf("unable to initialize command options: %v", err)
 	}
 
 	fss := cliflag.NamedFlagSets{}
-	command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, app.DefaultInitFuncConstructors, names.CCMControllerAliases(), fss, wait.NeverStop)
-
-	openstack.AddExtraFlags(pflag.CommandLine)
+	openstack.AddExtraFlags(fss.FlagSet("OpenStack Client"))
 
-	// 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(cliflag.WordSepNormalizeFunc)
-	pflag.CommandLine.AddGoFlagSet(goflag.CommandLine)
-
-	// utilflag.InitFlags()
-	logs.InitLogs()
-	defer logs.FlushLogs()
+	command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, app.DefaultInitFuncConstructors, names.CCMControllerAliases(), fss, wait.NeverStop)
 
 	klog.V(1).Infof("openstack-cloud-controller-manager version: %s", version.Version)
 

pkg/autohealing/cloudprovider/openstack/provider.go

@@ -25,6 +25,7 @@ import (
 	"time"
 
 	"github.com/gophercloud/gophercloud"
+	gopenstack "github.com/gophercloud/gophercloud/openstack"
 	"github.com/gophercloud/gophercloud/openstack/blockstorage/v2/volumes"
 	"github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/startstop"
 	"github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/volumeattach"
@@ -43,8 +44,10 @@ import (
 	"k8s.io/client-go/util/retry"
 	log "k8s.io/klog/v2"
 
+	"k8s.io/cloud-provider-openstack/pkg/autohealing/cloudprovider"
 	"k8s.io/cloud-provider-openstack/pkg/autohealing/config"
 	"k8s.io/cloud-provider-openstack/pkg/autohealing/healthcheck"
+	"k8s.io/cloud-provider-openstack/pkg/client"
 )
 
 const (
@@ -671,3 +674,55 @@ func CheckNodeCondition(node *apiv1.Node, conditionType apiv1.NodeConditionType,
 	}
 	return false
 }
+
+func NewOpenStackCloudProvider(cfg config.Config, kubeClient kubernetes.Interface) (cloudprovider.CloudProvider, error) {
+	client, err := client.NewOpenStackClient(&cfg.OpenStack, "magnum-auto-healer")
+	if err != nil {
+		return nil, err
+	}
+
+	eoOpts := gophercloud.EndpointOpts{
+		Region:       cfg.OpenStack.Region,
+		Availability: cfg.OpenStack.EndpointType,
+	}
+
+	// get nova service client
+	var novaClient *gophercloud.ServiceClient
+	novaClient, err = gopenstack.NewComputeV2(client, eoOpts)
+	if err != nil {
+		return nil, fmt.Errorf("failed to find Nova service endpoint in the region %s: %v", cfg.OpenStack.Region, err)
+	}
+
+	// get heat service client
+	var heatClient *gophercloud.ServiceClient
+	heatClient, err = gopenstack.NewOrchestrationV1(client, eoOpts)
+	if err != nil {
+		return nil, fmt.Errorf("failed to find Heat service endpoint in the region %s: %v", cfg.OpenStack.Region, err)
+	}
+
+	// get magnum service client
+	var magnumClient *gophercloud.ServiceClient
+	magnumClient, err = gopenstack.NewContainerInfraV1(client, eoOpts)
+	if err != nil {
+		return nil, fmt.Errorf("failed to find Magnum service endpoint in the region %s: %v", cfg.OpenStack.Region, err)
+	}
+	magnumClient.Microversion = "latest"
+
+	// get cinder service client
+	var cinderClient *gophercloud.ServiceClient
+	cinderClient, err = gopenstack.NewBlockStorageV3(client, eoOpts)
+	if err != nil {
+		return nil, fmt.Errorf("failed to find Cinder service endpoint in the region %s: %v", cfg.OpenStack.Region, err)
+	}
+
+	p := CloudProvider{
+		KubeClient: kubeClient,
+		Nova:       novaClient,
+		Heat:       heatClient,
+		Magnum:     magnumClient,
+		Cinder:     cinderClient,
+		Config:     cfg,
+	}
+
+	return p, nil
+}