Skip to content

Commit

Permalink
Remove handling code for glog (kubernetes#2325)
Browse files Browse the repository at this point in the history
* client-keystone-auth: Remove duplicate pflag.Parse call

This is already handled by the earlier call to the (confusingly named)
'InitFlags' function provided by 'k8s.io/component-base/cli/flag' [1].

[1] https://github.com/kubernetes/component-base/blob/v0.28.1/cli/flag/flags.go#L51-L59

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

* k8s-keystone-auth: Remove duplicate pflag.Parse call

This is already handled by the earlier call to the (confusingly named)
'InitFlags' function provided by 'k8s.io/component-base/cli/flag' [1].
We need to move the handling for the '--version' flag to after this to
ensure we parse the flags first.

[1] https://github.com/kubernetes/component-base/blob/v0.28.1/cli/flag/flags.go#L51-L59

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

* barbican-kms-plugin: Remove unnecessary klog flag calls

The comment here seems to have been left over when this module was
extensively reworked in commit 7f1e9ed (kubernetes#2278). In any case, it's
unnecessary: the call to 'klog.InitFlags' with a 'nil' argument results
in the flags being registered against the global 'flag.CommandLine'
flagset [1], but since cobra uses pflag rather than flag this doesn't
do anything useful. You can validate this by simply building the binary
without this change: you'll note that we only have '-v' and '-vmodule'
arguments, which are actually added by 'cli.Run' [2][3][4]. As such, we
can simply remove the calls.

[1] https://github.com/kubernetes/klog/blob/v2.100.1/klog.go#L432-L434
[2] https://github.com/kubernetes/component-base/blob/v0.28.1/cli/run.go#L46
[3] https://github.com/kubernetes/component-base/blob/v0.28.1/cli/run.go#L120
[4] https://github.com/kubernetes/component-base/blob/v0.28.1/logs/logs.go#L73-L105

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

* cinder-csi-plugin: Remove handling for glog

The kubernetes ecosystem has migrated to klog now and the flags
registered are for klog [1]. As such, there is no need to continue
translating from glog to klog.

[1] https://github.com/kubernetes/component-base/blob/v0.28.1/logs/logs.go#L73-L105

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

* manila-csi-plugin: Remove handling for glog

Remove the code to handle translation of legacy glog options to klog
options since glog is no longer a thing in kubernetes.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

* client-keystone-auth: Remove handling for glog

As with our earlier change to cinder-csi-plugin, the handling code for
glog is no longer necessary now that the kubernetes ecosystem has
migrated to klog. However, unlike that change, client-keystone-auth is
not using cobra but rather plain old pflag. As a result, it is still
actually registering all the klog options. We preserve this behavior.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

* k8s-keystone-auth: Remove handling for glog

This is quite similar but not identical to the earlier removal of glog
handling in client-keystone-auth. As with that utility, we are using
pflag rather than cobra here, but unlike that utility the klog options
are not being registered.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

---------

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
  • Loading branch information
stephenfin authored and mandre committed Feb 7, 2024
1 parent 0ea41fd commit a9d622c
Show file tree
Hide file tree
Showing 5 changed files with 2 additions and 103 deletions.
7 changes: 0 additions & 7 deletions cmd/barbican-kms-plugin/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package main

import (
"flag"
"os"
"os/signal"

Expand All @@ -35,12 +34,6 @@ var (
)

func main() {
flag.Parse()

// This is a temporary hack to enable proper logging until upstream dependencies
// are migrated to fully utilize klog instead of glog.
klog.InitFlags(nil)

cmd := &cobra.Command{
Use: "barbican-kms-plugin",
Short: "Barbican KMS plugin for Kubernetes",
Expand Down
28 changes: 0 additions & 28 deletions cmd/cinder-csi-plugin/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ limitations under the License.
package main

import (
"flag"
"fmt"
"os"

"github.com/spf13/cobra"
Expand All @@ -41,34 +39,9 @@ var (
)

func main() {
if err := flag.CommandLine.Parse([]string{}); err != nil {
klog.Fatalf("Unable to parse flags: %v", err)
}

cmd := &cobra.Command{
Use: "Cinder",
Short: "CSI based Cinder driver",
PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
// Glog requires this otherwise it complains.
if err := flag.CommandLine.Parse(nil); err != nil {
return fmt.Errorf("unable to parse flags: %w", err)
}

// This is a temporary hack to enable proper logging until upstream dependencies
// are migrated to fully utilize klog instead of glog.
klogFlags := flag.NewFlagSet("klog", flag.ExitOnError)
klog.InitFlags(klogFlags)

// Sync the glog and klog flags.
cmd.Flags().VisitAll(func(f1 *pflag.Flag) {
f2 := klogFlags.Lookup(f1.Name)
if f2 != nil {
value := f1.Value.String()
_ = f2.Value.Set(value)
}
})
return nil
},
Run: func(cmd *cobra.Command, args []string) {
handle()
},
Expand Down Expand Up @@ -99,7 +72,6 @@ func main() {
}

func handle() {

// Initialize cloud
d := cinder.NewDriver(endpoint, cluster)
openstack.InitOpenStackProvider(cloudConfig, httpEndpoint)
Expand Down
20 changes: 2 additions & 18 deletions cmd/client-keystone-auth/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,24 +138,6 @@ func argumentsAreSet(url, user, project, password, domain, applicationCredential
}

func main() {
// Glog requires this otherwise it complains.
if err := flag.CommandLine.Parse(nil); err != nil {
klog.Fatalf("Unable to parse flags: %v", err)
}
// This is a temporary hack to enable proper logging until upstream dependencies
// are migrated to fully utilize klog instead of glog.
klogFlags := flag.NewFlagSet("klog", flag.ExitOnError)
klog.InitFlags(klogFlags)

// Sync the glog and klog flags.
flag.CommandLine.VisitAll(func(f1 *flag.Flag) {
f2 := klogFlags.Lookup(f1.Name)
if f2 != nil {
value := f1.Value.String()
_ = f2.Value.Set(value)
}
})

var url string
var domain string
var user string
Expand Down Expand Up @@ -186,6 +168,8 @@ func main() {

logs.AddFlags(pflag.CommandLine)

klogFlags := flag.NewFlagSet("klog", flag.ExitOnError)
klog.InitFlags(klogFlags)
pflag.CommandLine.AddGoFlagSet(klogFlags)

kflag.InitFlags()
Expand Down
21 changes: 0 additions & 21 deletions cmd/k8s-keystone-auth/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ limitations under the License.
package main

import (
"flag"
"fmt"
"os"

Expand All @@ -29,32 +28,12 @@ import (
)

func main() {
// Glog requires this otherwise it complains.
err := flag.CommandLine.Parse(nil)
if err != nil {
klog.Fatalf("Unable to parse flags: %v", err)
}

var showVersion bool
pflag.BoolVar(&showVersion, "version", false, "Show current version and exit")

// This is a temporary hack to enable proper logging until upstream dependencies
// are migrated to fully utilize klog instead of glog.
klogFlags := flag.NewFlagSet("klog", flag.ExitOnError)
klog.InitFlags(klogFlags)

logs.AddFlags(pflag.CommandLine)
keystone.AddExtraFlags(pflag.CommandLine)

// Sync the glog and klog flags.
flag.CommandLine.VisitAll(func(f1 *flag.Flag) {
f2 := klogFlags.Lookup(f1.Name)
if f2 != nil {
value := f1.Value.String()
_ = f2.Value.Set(value)
}
})

logs.InitLogs()
defer logs.FlushLogs()

Expand Down
29 changes: 0 additions & 29 deletions cmd/manila-csi-plugin/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,11 @@ limitations under the License.
package main

import (
"flag"
"fmt"
"os"
"strings"

"github.com/spf13/cobra"
"github.com/spf13/pflag"
"k8s.io/cloud-provider-openstack/pkg/csi/manila"
"k8s.io/cloud-provider-openstack/pkg/csi/manila/csiclient"
"k8s.io/cloud-provider-openstack/pkg/csi/manila/manilaclient"
Expand Down Expand Up @@ -66,34 +64,9 @@ func validateShareProtocolSelector(v string) error {
}

func main() {
if err := flag.CommandLine.Parse([]string{}); err != nil {
klog.Fatalf("Unable to parse flags: %v", err)
}

cmd := &cobra.Command{
Use: os.Args[0],
Short: "CSI Manila driver",
PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
// Glog requires this otherwise it complains.
if err := flag.CommandLine.Parse(nil); err != nil {
return fmt.Errorf("unable to parse flags: %w", err)
}

// This is a temporary hack to enable proper logging until upstream dependencies
// are migrated to fully utilize klog instead of glog.
klogFlags := flag.NewFlagSet("klog", flag.ExitOnError)
klog.InitFlags(klogFlags)

// Sync the glog and klog flags.
cmd.Flags().VisitAll(func(f1 *pflag.Flag) {
f2 := klogFlags.Lookup(f1.Name)
if f2 != nil {
value := f1.Value.String()
_ = f2.Value.Set(value)
}
})
return nil
},
Run: func(cmd *cobra.Command, args []string) {
if err := validateShareProtocolSelector(protoSelector); err != nil {
klog.Fatalf(err.Error())
Expand Down Expand Up @@ -128,8 +101,6 @@ func main() {
Version: version.Version,
}

cmd.Flags().AddGoFlagSet(flag.CommandLine)

cmd.PersistentFlags().StringVar(&endpoint, "endpoint", "unix://tmp/csi.sock", "CSI endpoint")

cmd.PersistentFlags().StringVar(&driverName, "drivername", "manila.csi.openstack.org", "name of the driver")
Expand Down

0 comments on commit a9d622c

Please sign in to comment.