Add variable for global flags to runlabel#2905
Add variable for global flags to runlabel#2905openshift-merge-robot merged 1 commit intocontainers:masterfrom
Conversation
|
Initial impression is that this looks about right - just needs to pick up the rest of the local flags |
|
do I need to add tests for this? |
|
Tests are always appreciated! |
|
Test added |
|
/approved |
cmd/podman/runlabel.go
Outdated
There was a problem hiding this comment.
@mheon, should this live in a common or util file somewhere?
There was a problem hiding this comment.
I've moved it to package util
|
LGTM, just one question on a final resting place for one of the functions. |
cmd/podman/shared/funcs.go
Outdated
pkg/util/utils.go
Outdated
There was a problem hiding this comment.
I wonder if we can somehow pull the names of the global flags straight out of Cobra - @baude you know if there's a way to do this?
There was a problem hiding this comment.
I found FlagSet.VisitAll() for all flags, not just for global flags...
There was a problem hiding this comment.
Hm. This could be painful if we add more global flags, but no helping that, I suppose.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mheon, QiWang19 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
☔ The latest upstream changes (presumably #3031) made this pull request unmergeable. Please resolve the merge conflicts. |
|
LGTM |
use $GLOBAL_OPTS to pass global flags to the runlabel command. Signed-off-by: Qi Wang <qiwan@redhat.com>
| // GetGlobalOpts checks all global flags and generates the command string | ||
| func GetGlobalOpts(c *cliconfig.RunlabelValues) string { | ||
| globalFlags := map[string]bool{ | ||
| "cgroup-manager": true, "cni-config-dir": true, "conmon": true, "default-mounts-file": true, |
There was a problem hiding this comment.
Separate PR, but could you move this to main_local.go
Add one line after each global flag gets added
rootCmd.PersistentFlags().StringVar(&MainGlobalOpts.CGroupManager, "cgroup-manager", "", "Cgroup manager to use (cgroupfs or systemd, default systemd)")
globalFlags["cgroup-manager"]=true
rootCmd.PersistentFlags().StringVar(&MainGlobalOpts.CpuProfile, "cpu-profile", "", "Path for the cpu profiling results")
globalFlags["cpu-profile"]=true
That way if a user adds a global flag, it is much more likely they will do the right thing.
|
/lgtm |
close #2399
use $GLOBAL_OPTS to pass global flags to the runlabel command.
Signed-off-by: Qi Wang qiwan@redhat.com