-
Notifications
You must be signed in to change notification settings - Fork 37
Set logging level in ovn-controller #510
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
base: main
Are you sure you want to change the base?
Conversation
karelyatin
left a comment
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.
Since this config change will trigger a pod restarts and trigger some downtime for centralized traffic, wondering now if we really need this change as when needed temporary debug can just change with vlog/set in the required pods?
If we still need to enable these flags then i think we should consider avoiding any restarts with loglevel change.
|
Good point @karelyatin , A way to work arround that could be create a job that spawns a new pod that connects to the desired pod (ovn-controller or ovs-db-server) and modify the debug level using vlog/set? |
Yes, Can consider to utilized existing config job for this too |
183d5f3 to
ba26b60
Compare
|
I changed it to work now without the need of restarting any pods, using the config job. Let me know if this is better. Thanks for all the reviews! |
fe52f7b to
79a131e
Compare
|
I improved the ovn-appctl commands so as to not hardcode the name of the unixctl socket. |
slawqo
left a comment
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.
LGTM for me, just please remove that empty line from start-vswitchd.sh file and that way diff will be one file smaller :)
| # under the License. | ||
|
|
||
| source $(dirname $0)/functions | ||
|
|
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.
nit: I think that this is not really needed
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.
100% agree, I did not realize I left that :)
OVNLogLevel affects ovncontroller logs and OVSLogLevel affects ovs-vswitchd and ovsdb-server. Both OVNLogLevel and OVSLogLevel can take values from the vlog range: off, emer, err, warn, info, or dbg See ovs-appctl(8) for a definition of each log level. The change of the log level will be modified from the config pod and not on the pod itself to avoid restarting ovn-controller and ovn-controller-ovs pods. Resolves: OSPRH-6429 Signed-off-by: Elvira Garcia <[email protected]>
79a131e to
0dbc84c
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: elvgarrui, slawqo 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 |
|
/retest failed unrelated because of fips
|
|
@elvgarrui: The The following commands are available to trigger optional jobs: Use In response to this:
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-sigs/prow repository. |
|
/retest |
|
@elvgarrui: The following test failed, say
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-sigs/prow repository. I understand the commands that are listed here. |
|
Waiting for PR #513 to merge and unblock the test |
OVNLogLevel affects ovncontroller logs and OVSLogLevel affects
ovs-vswitchd and ovsdb-server. Both OVNLogLevel and OVSLogLevel can take
values from the vlog range: off, emer, err, warn, info, or dbg See
ovs-appctl(8) for a definition of each log level. The change of the log
level will be modified from the config pod and not on the pod itself to
avoid restarting ovn-controller and ovn-controller-ovs pods.
Resolves: OSPRH-6429