pass log level to KCM #1155
pass log level to KCM #1155
Conversation
| @@ -96,6 +96,8 @@ func configure(cfg *config.MicroshiftConfig) (args []string, applyFn func() erro | |||
| } | |||
|
|
|||
| args, err = mergeAndConvertToArgs(overrides) | |||
| args = append(args, fmt.Sprintf("-v=%d", cfg.LogVLevel)) | |||
There was a problem hiding this comment.
Does including
"v": {fmt.Sprintf("%d", cfg.LogVLevel)},
To ExtendedArguments in overrides do the same? You wouldnt need this line.
There was a problem hiding this comment.
@pacevedom Yup, looks better to use the same approach for both short and long arguments. I have modified GetKubeControllerManagerArgs to support that.
atiratree
force-pushed
the
fix-kcm-logs
branch
from
9e87955
to
e0fd9f4
Compare
atiratree
force-pushed
the
fix-kcm-logs
branch
from
e0fd9f4
to
626766c
Compare
| @@ -171,7 +173,11 @@ func GetKubeControllerManagerArgs(config map[string]interface{}) []string { | |||
| args := []string{} | |||
| for key, value := range extendedArguments.(map[string]interface{}) { | |||
| for _, arrayValue := range value.([]interface{}) { | |||
| args = append(args, fmt.Sprintf("--%s=%s", key, arrayValue.(string))) | |||
| if len(key) == 1 { | |||
| args = append(args, fmt.Sprintf("-%s=%s", key, arrayValue.(string))) | |||
There was a problem hiding this comment.
This should work with double dash too.
There was a problem hiding this comment.
yes, you are right. But for example -h doesn't, so it shouldn't hurt anything to keep this here.
There was a problem hiding this comment.
also a minor plus for consistency is that we are using -v in KCM operator as well when generating the args.
Nevertheless, I don't have a strong preference if you want to remove this.
There was a problem hiding this comment.
log level verbosity flag in KCM supports simple or double dash. So this sounds good to me.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: atiratree, oglok The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
@atiratree: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
needed for testing the KCM https://issues.redhat.com/browse/WRKLDS-604
It seems to me that log level is passed correctly to the other controllers