Support get/set log verbosity level by antctl and controller/agent API #1340

jianjuns · 2020-10-04T04:38:11Z

Add antctl log-level command that calls controller or agent /loglevel
API to get/set the log verbosity level of controller or agent in the
runtime.
Change antctl to allow an empty response of an controller or agent
call.

antrea-bot · 2020-10-04T04:38:25Z

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

/test-e2e: to trigger e2e tests.
/skip-e2e: to skip e2e tests.
/test-conformance: to trigger conformance tests.
/skip-conformance: to skip conformance tests.
/test-whole-conformance: to trigger all conformance tests on linux.
/skip-whole-conformance: to skip all conformance tests on linux.
/test-networkpolicy: to trigger networkpolicy tests.
/skip-networkpolicy: to skip networkpolicy tests.
/test-windows-conformance: to trigger windows conformance tests.
/skip-windows-conformance: to skip windows conformance tests.
/test-windows-networkpolicy: to trigger windows networkpolicy tests.
/skip-windows-networkpolicy: to skip windows networkpolicy tests.
/test-hw-offload: to trigger ovs hardware offload test.
/skip-hw-offload: to skip ovs hardware offload test.
/test-all: to trigger all tests (except whole conformance).
/skip-all: to skip all tests (except whole conformance).

jianjuns · 2020-10-04T04:38:35Z

I feel very useful to allow changing log level in the runtime.

codecov-commenter · 2020-10-04T04:44:11Z

Codecov Report

Merging #1340 into master will decrease coverage by 0.16%.
The diff coverage is 21.05%.

@@            Coverage Diff             @@
##           master    #1340      +/-   ##
==========================================
- Coverage   64.63%   64.47%   -0.17%     
==========================================
  Files         157      159       +2     
  Lines       12626    12657      +31     
==========================================
- Hits         8161     8160       -1     
- Misses       3620     3645      +25     
- Partials      845      852       +7

Flag	Coverage Δ
#integration-tests	`44.76% <ø> (-0.08%)`	⬇️
#kind-e2e-tests	`50.58% <32.00%> (-0.23%)`	⬇️
#unit-tests	`42.12% <0.00%> (-0.12%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/antctl/antctl.go	`100.00% <ø> (ø)`
pkg/antctl/command_definition.go	`41.96% <0.00%> (-0.44%)`	⬇️
pkg/antctl/command_list.go	`31.48% <0.00%> (-1.22%)`	⬇️
pkg/log/log_level.go	`10.00% <10.00%> (ø)`
pkg/apiserver/handlers/loglevel/handler.go	`38.46% <38.46%> (ø)`
pkg/agent/apiserver/apiserver.go	`66.12% <100.00%> (+0.55%)`	⬆️
pkg/apiserver/apiserver.go	`84.33% <100.00%> (+0.19%)`	⬆️
pkg/controller/networkpolicy/tier.go	`90.00% <0.00%> (-10.00%)`	⬇️
pkg/apiserver/certificate/certificate.go	`71.05% <0.00%> (-6.58%)`	⬇️
pkg/ovs/openflow/ofctrl_bridge.go	`70.35% <0.00%> (-1.59%)`	⬇️
... and 3 more

antoninbas

LGTM, I think we should mention the command in the antctl documentation (with the limitation that it only supports local execution).

antoninbas · 2020-10-05T18:44:34Z

pkg/log/log_level.go

+	if err != nil {
+		return err
+	}
+	klog.Infof("Change log level from %s to %s", oldLevel, level)


nit: "Changed" or "Changing"

Changed to "Changed"

antoninbas · 2020-10-05T18:45:04Z

build/yamls/base/antctl.yml

@@ -50,6 +50,7 @@ rules:
    verbs:
      - get
  - nonResourceURLs:
+      - /loglevel


I thought these were kept in alphabetical order on purpose?

Ok. Changed.

antoninbas · 2020-10-05T18:47:19Z

pkg/apiserver/handlers/loglevel/handler.go

+	"github.com/vmware-tanzu/antrea/pkg/log"
+)
+
+// HandleFunc returns the function which can handle the /log-level API request.


You used /loglevel for the APIU endpoint

I like to follow the kubectl (and other antctl commands) style which use "-" to connect two words in command names, like cluster-info, port-forward, api-resources.

I undertand and I'm not suggesting you changed the antctl command name. But you are referring to the API endpoint here, for which you used /loglevel not /log-level.

I see. I think we want not connectors in API path?

We haven't been using them so far, so I was really just suggesting fixing the comment

Ok. Sorry for being misunderstanding your comments.
Changed to /loglevel.

jianjuns · 2020-10-05T19:50:04Z

LGTM, I think we should mention the command in the antctl documentation (with the limitation that it only supports local execution).

Added the command to antctl.md.

Add antctl log-level command that calls controller or agent /loglevel API to get/set the log verbosity level of controller or agent in the runtime. Change antctl to allow an empty response of an controller or agent call.

antoninbas

LGTM

jianjuns · 2020-10-05T21:39:37Z

/test-all

vmwclabot added the cla-not-required label Oct 4, 2020

jianjuns force-pushed the loglevel-cmd branch from b300401 to ea17647 Compare October 4, 2020 04:40

jianjuns force-pushed the loglevel-cmd branch from ea17647 to 168a597 Compare October 4, 2020 05:38

antoninbas reviewed Oct 5, 2020

View reviewed changes

jianjuns force-pushed the loglevel-cmd branch 2 times, most recently from f14e3c0 to 8f5766b Compare October 5, 2020 19:48

jianjuns force-pushed the loglevel-cmd branch from 8f5766b to 91b1684 Compare October 5, 2020 19:59

jianjuns force-pushed the loglevel-cmd branch from 91b1684 to 417fad6 Compare October 5, 2020 20:01

antoninbas approved these changes Oct 5, 2020

View reviewed changes

jianjuns merged commit 9c106b2 into antrea-io:master Oct 5, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support get/set log verbosity level by antctl and controller/agent API #1340

Support get/set log verbosity level by antctl and controller/agent API #1340

jianjuns commented Oct 4, 2020

antrea-bot commented Oct 4, 2020

jianjuns commented Oct 4, 2020

codecov-commenter commented Oct 4, 2020 •

edited

Loading

antoninbas left a comment

antoninbas Oct 5, 2020

jianjuns Oct 5, 2020

antoninbas Oct 5, 2020

jianjuns Oct 5, 2020

antoninbas Oct 5, 2020

jianjuns Oct 5, 2020

antoninbas Oct 5, 2020

jianjuns Oct 5, 2020

antoninbas Oct 5, 2020

jianjuns Oct 5, 2020

jianjuns commented Oct 5, 2020

antoninbas left a comment

jianjuns commented Oct 5, 2020

Support get/set log verbosity level by antctl and controller/agent API #1340

Support get/set log verbosity level by antctl and controller/agent API #1340

Conversation

jianjuns commented Oct 4, 2020

antrea-bot commented Oct 4, 2020

jianjuns commented Oct 4, 2020

codecov-commenter commented Oct 4, 2020 • edited Loading

Codecov Report

antoninbas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jianjuns commented Oct 5, 2020

antoninbas left a comment

Choose a reason for hiding this comment

jianjuns commented Oct 5, 2020

codecov-commenter commented Oct 4, 2020 •

edited

Loading