-
Notifications
You must be signed in to change notification settings - Fork 597
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove handling code for glog #2325
Changes from all commits
d0fc449
de63646
6413e2c
c5a1830
4b92df2
eb7417f
c57e0ad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,8 +17,6 @@ limitations under the License. | |
package main | ||
|
||
import ( | ||
"flag" | ||
"fmt" | ||
"os" | ||
|
||
"github.com/spf13/cobra" | ||
|
@@ -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) | ||
} | ||
}) | ||
Comment on lines
-63
to
-69
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This iterates over the cobra flags. For each one it looks up a correspondingly named klog flag and sets its value to the value of the cobra flag. IOW it has no effect on the cobra flags, which are the only flags now in use. |
||
return nil | ||
}, | ||
Run: func(cmd *cobra.Command, args []string) { | ||
handle() | ||
}, | ||
|
@@ -99,7 +72,6 @@ func main() { | |
} | ||
|
||
func handle() { | ||
|
||
// Initialize cloud | ||
d := cinder.NewDriver(endpoint, cluster) | ||
openstack.InitOpenStackProvider(cloudConfig, httpEndpoint) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now done below, so we keep the same flags. |
||
|
||
// 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) | ||
} | ||
}) | ||
Comment on lines
-151
to
-157
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sets flags in flag.CommandLine with a corresponding klogflag to the value of the klog flag. Nothing is now using these flags, so this is redundant. |
||
|
||
var url string | ||
var domain string | ||
var user string | ||
|
@@ -186,10 +168,11 @@ func main() { | |
|
||
logs.AddFlags(pflag.CommandLine) | ||
|
||
klogFlags := flag.NewFlagSet("klog", flag.ExitOnError) | ||
klog.InitFlags(klogFlags) | ||
pflag.CommandLine.AddGoFlagSet(klogFlags) | ||
kflag.InitFlags() | ||
|
||
pflag.Parse() | ||
kflag.InitFlags() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
||
if showVersion { | ||
fmt.Println(version.Version) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,6 @@ limitations under the License. | |
package main | ||
|
||
import ( | ||
"flag" | ||
"fmt" | ||
"os" | ||
|
||
|
@@ -29,46 +28,25 @@ 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) | ||
} | ||
}) | ||
Comment on lines
-49
to
-56
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This appears to set values in flag.CommandLine, although nothing is using it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh it's used alright: the |
||
|
||
pflag.Parse() | ||
|
||
if showVersion { | ||
fmt.Println(version.Version) | ||
os.Exit(0) | ||
} | ||
|
||
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) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewer note: This copies all the options from the
commandLine
flagset defined here to klogFlags.This includes --log_dir, --log_file, et al.