Enable verbose logging through generate-manifest.sh #1142

srikartati · 2020-08-24T19:21:31Z

This patch enables verbose logging for antrea-agent and
antrea-controller when generate manifest. This will help in
troubleshooting with increased log level.

I felt troubleshooting info for incrementing log-level for antrea-agent and antrea-controller is lacking right now. I thought providing an antrea manifest through generate-manifest.sh with increased log-level is one way to help troubleshooting for developers/users. There could be an efficient approach instead of using antrea-conf configMap. Please comment on what you think.

If this approach looks ok, I will update the troubleshooting doc as part of this PR.

antrea-bot · 2020-08-24T19:21:47Z

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).

tnqn · 2020-08-25T16:10:54Z

build/yamls/patches/dev/agentVerboseLog.yml

+              valueFrom:
+                configMapKeyRef:
+                  name: antrea-config
+                  key: verbose-level


Could it use a patch operation to add the loglevel to args directly without involving configmap?

thought it over, maybe exposing it as a key of configmap indeed makes it easier for users to set it. would hear others' opinions too.

but why is this a patch of dev model? I see the commit message says the ability is for users too?
And could you add ending newline for the last line?

Thanks for the comment.
I used configmap to avoid doing sed on *VerboseLog.yml to update the log level. I could not think of another cleaner way. May be possible with json patching.. I haven't tried that.
Yes, an advantage of having in configmap will make it easier for users to change it.

I saw dev troubleshooting as the main use case. Yes, it is possible that it can be helpful with debugging in production environment with release manifests. Do you think it makes sense to have this both for dev and release manifests?

I am thinking something simpler, add "--v=0" to the base yaml so that user know where to increase the log level for agent and controller separately, given that we already specify other log related options there directly, then override the command in dev mode patch to specify a higher level like 5. What do you think?

Thanks for the suggestion @tnqn
If I understand correctly, I think you want me to add a patch like below to replace the args with new log level:

apiVersion: apps/v1 kind: DaemonSet metadata: name: antrea-agent spec: template: spec: containers: - name: antrea-agent args: ["--config", "/etc/antrea/antrea-agent.conf", "--logtostderr=false", "--log_dir=/var/log/antrea", "--alsologtostderr", "--log_file_max_size=100", "--log_file_max_num=4", "--v=<NEW_LOG_LEVEL>"]

Is that correct? Do you have an alternative approach in mind to change the log level in above patch.yml instead of sed?

Yes, I don't know an alternative approach to set log level, but I thought we could hard code it to 5 in dev mode because we just need the logs to be more verbose for development.

Yes, I did not find any other approach than changing through configMap. As this is log-level, we can definitely avoid it. I like the suggested approach of adding '--v=0' in the base YAML file as it will be intuitive for users to change it. As an additional step, we can provide a way to generate the YAML with increased log-level through generate_manifest.sh script in dev mode. Updated the patch accordingly.

I picked log level of 4 as 0-4 considered as debug-level logs, and 5-10 as trace-level logs. Let me know if you think 5 would be good.

codecov-commenter · 2020-09-03T18:30:03Z

Codecov Report

Merging #1142 into master will decrease coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1142      +/-   ##
==========================================
- Coverage   56.33%   56.27%   -0.06%     
==========================================
  Files         108      108              
  Lines       12027    12027              
==========================================
- Hits         6775     6768       -7     
- Misses       4666     4670       +4     
- Partials      586      589       +3

Flag	Coverage Δ
#integration-tests	`46.09% <ø> (-0.04%)`	⬇️
#unit-tests	`42.53% <ø> (-0.05%)`	⬇️

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

Impacted Files	Coverage Δ
pkg/apiserver/certificate/certificate.go	`72.83% <0.00%> (-6.18%)`	⬇️
pkg/ovs/openflow/ofctrl_bridge.go	`71.57% <0.00%> (-0.67%)`	⬇️
...ntroller/networkpolicy/networkpolicy_controller.go	`73.04% <0.00%> (-0.25%)`	⬇️
pkg/apiserver/storage/ram/store.go	`81.69% <0.00%> (+1.30%)`	⬆️

tnqn

a typo in doc, otherwise LGTM

tnqn · 2020-09-09T07:13:02Z

docs/troubleshooting.md

@@ -38,6 +38,11 @@ The Open vSwitch daemon logs for each `antrea-agent` Pod are also stored on the
 persistent storage of the corresponding node (i.e. the node on which the Pod is
 scheduled), under `/var/log/antrea/openvswitch`.

+To increase the log level for the `antrea-agent` and the `antrea-controller`, you can edit the `-v==0` arg in the Antrea manifest to a desired level. Alternatively, you can generate an Antrea manifest with increased log level of 4 (maximum debug level) using `generate_manifest.sh`:


Suggested change

To increase the log level for the `antrea-agent` and the `antrea-controller`, you can edit the `-v==0` arg in the Antrea manifest to a desired level. Alternatively, you can generate an Antrea manifest with increased log level of 4 (maximum debug level) using `generate_manifest.sh`:

To increase the log level for the `antrea-agent` and the `antrea-controller`, you can edit the `-v=0` arg in the Antrea manifest to a desired level. Alternatively, you can generate an Antrea manifest with increased log level of 4 (maximum debug level) using `generate_manifest.sh`:

two "=" is illegal

Fixed it. Thanks.

srikartati · 2020-09-09T19:06:29Z

/test-all

srikartati · 2020-09-09T19:06:53Z

/skip-hw-offload

antoninbas · 2020-09-10T00:44:24Z

hack/generate-manifest.sh

@@ -31,6 +31,8 @@ Generate a YAML manifest for Antrea using Kustomize and print it to stdout.
        --np                          Generate a manifest with ClusterNetworkPolicy and Antrea NetworkPolicy features enabled
        --keep                        Debug flag which will preserve the generated kustomization.yml
        --tun (geneve|vxlan|gre|stt)  Choose encap tunnel type from geneve, gre, stt and vxlan (default is geneve)
+        --verbose-log                 Generate a manifest with increased log-level (level 4) for Antrea agent and controller.


curious about why you didn't make this --verbosity <N>?

Could not find a way to change log levels in manifest through kustomize other than using configmap. Or using sed, which I felt not very neat.
In addition, '--v=0' in base yaml is good enough to make it intuitive for devs to change log-level through command args.
#1142 (comment)

makes sense

antoninbas

just one more nit, otherwise LGTM

antoninbas · 2020-09-11T23:08:10Z

docs/troubleshooting.md

@@ -38,6 +38,11 @@ The Open vSwitch daemon logs for each `antrea-agent` Pod are also stored on the
 persistent storage of the corresponding node (i.e. the node on which the Pod is
 scheduled), under `/var/log/antrea/openvswitch`.

+To increase the log level for the `antrea-agent` and the `antrea-controller`, you can edit the `--v=0` arg in the Antrea manifest to a desired level. Alternatively, you can generate an Antrea manifest with increased log level of 4 (maximum debug level) using `generate_manifest.sh`:


please wrap that line

antoninbas · 2020-09-11T23:08:28Z

hack/generate-manifest.sh

@@ -31,6 +31,8 @@ Generate a YAML manifest for Antrea using Kustomize and print it to stdout.
        --np                          Generate a manifest with ClusterNetworkPolicy and Antrea NetworkPolicy features enabled
        --keep                        Debug flag which will preserve the generated kustomization.yml
        --tun (geneve|vxlan|gre|stt)  Choose encap tunnel type from geneve, gre, stt and vxlan (default is geneve)
+        --verbose-log                 Generate a manifest with increased log-level (level 4) for Antrea agent and controller.


makes sense

This patch enables verbose logging for antrea-agent and antrea-controller when generate manifest. This will help in troubleshooting with increased log level.

srikartati · 2020-09-12T01:14:03Z

/test-all

srikartati · 2020-09-14T13:52:49Z

/skip-hw-offload

tnqn

LGTM

srikartati · 2020-09-14T16:17:33Z

/skip-hw-offload

srikartati · 2020-09-14T16:18:29Z

As hw-offload test is not applicable, merging this patch.

This patch enables verbose logging for antrea-agent and antrea-controller when generate manifest. This will help in troubleshooting with increased log level.

* Enable verbose logging through generate-manifest.sh (#1142) This patch enables verbose logging for antrea-agent and antrea-controller when generate manifest. This will help in troubleshooting with increased log level. * Bug in PR#1142 (#1248) manifest generation failing in release mode. Fixed the bug. * [IPv6] Consume Node.Spec.CIDRs to support dual-stack configuration (#971) 1. Consume Node.Spec.CIDRs to support IPv4/IPv6 dual-stack Pod Subnets 2. Change NodeConfig.PodCIDR as a slice 3. Change GatewayConfig.IP as a slice to support multiple addresses for antrea-gw0 4. Change InterfaceConfig.IP as a slice to support multiple address for a Pod * [IPv6] Change openflow pipeline for L2 Pod networking (#1040) 1. Add a new table named IPv6 to handle IPv6 ND Solicitation, ND advertisement and IPv6 Multicast traffic. 2. Add flows in openflow tables (spoofGuardTable, IPv6, conntrackTable, conntrackStateTable, conntrackCommitTable, L2ForwardingOutTable) for handling IPv6 L2 Pod networking. * [IPv6] Change host-local IPAM configuration for IPv6 (#1039) 1. Add new field Ranges in IPAMConfig for allocating both IPv4 and IPv6 addresses. 2. Populate subnet and gateway for both IPv4 range and IPv6 range. * [IPv6] Use separate fields for IPv4 and IPv6 in GatewayConfig (#1111) Replace IP slice in GatewayConfig with separate IPv4 and IPv6 fields. * [IPv6] Implement L3 connectivity for IPv6 traffic (#1011) 1. Use IPv6 in iptables and ipset configuration. 2. Identifiy IPv6 address and configure in OpenFlow. 3. Use Node Internal address for tunnel. * [IPv6] Handle Spec.PodCIDR with IPv6 CIDR (#1151) For IPv6 single stack case, node.Spec.PodCIDR is configured with IPv6 CIDR. This patch handles the case and sets nodeConfig.PodIPv6CIDR with parsed CIDR. * [IPv6] Add support for IPv6 address in antctl and agent's apiserver (#1118) * [IPv6] Add support for IPv6 address in antctl and agent's apiserver 1. Support using IPv6 address in OVS tracing. 2. Support displaying Node's and Pod's IPv6 address in agent apiserver. Co-authored-by: Zhecheng Li <zhechel1@uci.edu> * [IPv6] Support IPv6 in e2e (#1129) * [IPv6] Display dual stack NodeSubnet in antrea-octant-plugin (#1156) NodeSubnet can have two values in dual stack case and this patch enhances octant-plugin to show both subnets. * [IPv6] Handle dual stack NodeSubnet for monitoring CRD (#1182) 1. Rename NodeSubnet to NodeSubnets for AntreaAgentInfo. 2. Make a new string slice for dual stack node subnet instead of appending agentInfo.NodeSubnets directly to avoid duplicate CIDRs. * [IPv6][e2e] Fix testDeletePod (#1193) On a dual-stack cluster, podInterfaces[0].IP returns "[ipv4-address], [ipv6-address]". Current implementation doesn't distingush two. * [IPv6] Collect service CIDR in e2e * [IPv6] Add support for dual-stack when using kube-proxy for Service (#1200) 1. Add a config item for IPv6 Serivce CIDR if using kube-proxy to provide Service functions. 2. Output IPv6 traffic from host gateway if its destination is a Service address. 3. Use ct_mark to identify Service traffic and output the reply packet to the host gateway to ensure the DNAT processing in iptables. * [IPv6] Extend e2e tests for dual-stack (#1192) 1. Extend generic functions "podWaitForIP" to return all assigned IPs of a given Pod. 2. Validate each IP address in the cluster's network CIDR 3. Use each valid IP to check connectivity. 4. Use each valid IP to execute tests. * [IPv6] E2e bug fixes (#1311) 1. No -6 option in busybox nc So, no need to distinguish if it is an IPv6 environment for runNetcatCommandFromTestPod() nc BusyBox v1.31.1 (2019-10-28 18:40:01 UTC) multi-call binary. Usage: nc [OPTIONS] HOST PORT - connect nc [OPTIONS] -l -p PORT [HOST] [PORT] - listen -e PROG Run PROG after connect (must be last) -l Listen mode, for inbound connects -lk With -e, provides persistent server -p PORT Local port -s ADDR Local address -w SEC Timeout for connects and final net reads -i SEC Delay interval for lines sent -n Don't do DNS resolution -u UDP mode -v Verbose -o FILE Hex dump traffic -z Zero-I/O mode (scanning) 2. testCert * IPv6 address should be in "[]" * [IPv6] Fix TestReconcileGatewayRoutesOnStartup failure (#1313) Use "ip -6 route" for IPv6 network. * [IPv6] adjust MTU for IPv6 overhead (#1305) If Antrea MTU is too large in IPv6 environment, large packet with overhead exceeds node MTU cannot be transmitted successfully across nodes. IPv6ExtraOverhead, 20 is from observation of IPv4 and IPv6 packets under same situation. * [IPv6] Fix MTU config (#1317) Use Node's internal address to decide if extra IPv6 overhead is needed. * [IPv6] Skip IPsec e2e test (#1373) * With OVS v2.14.0, IPsec in IPv6 envinronment is not supported. * More user-friendly output for PodIPs. From: Retrieved all Pod IPs: map[test-pod-0-upgp1ung:0xc000708960 test-pod-1-pbva9007:0xc0006ec8a0] To: Retrieved all Pod IPs: map[test-pod-0-mudzj847:IPv6: fd74:ca9b:172:16::4, IP strings: fd74:ca9b:172:16::4 test-pod-1-apcmyd30:IPv6: fd74:ca9b:172:16:1::3c, IP strings: fd74:ca9b:172:16:1::3c] * [IPv6] Add 2 Network Policy tests (#1399) 2 upstream Network Policy tests didn't consider netmask for IPv6, this patch is to add correct tests. When bug is fixed in latest release, these 2 tests can be deleted. Kubernetes PR: kubernetes/kubernetes#93583 2 testcases: https://github.com/kubernetes/kubernetes/blob/v1.20.0-alpha.0/test/e2e/network/network_policy.go#L1365 https://github.com/kubernetes/kubernetes/blob/v1.20.0-alpha.0/test/e2e/network/network_policy.go#L1444 * Skip 2 Network Policy testcases before Network Policy IPv6 is supported (#1460) * [IPv6] Fix after rebasing * format code * fix TestPodTrafficShaping * fix TestIPv6RoutesAndNeighbors * [IPv6] Fix issues (#1496) * unit test * manifest * [IPv6] Skip TestAntctlProxy for IPv6 (#1498) * [IPv6] Add IPv6 support for NetworkPolicy 1. Add enhancement in Antrea Controller and Agent to support NetworkPolicy in IPv6. 2. Optimize test cases to support IPv6 3. Use regex in CRD to validate IPv4 or IPv6 string 4. Add TestEgressToServerInCIDRBlock and TestEgressToServerInCIDRBlockWithException 5. networkpolicy_controller.go: PodIPs includes PodIP * [IPv6] Fix issues * remove Github Actions integration test, Jenkins: jenkins-integration -> Integration tests * go fmt * add FlowProtocl() to interface Flow * remove extra lines when rebasing for an octant commit * TestIPv6RoutesAndNeighbors: routeClient.Initialize Co-authored-by: srikartati <stati@vmware.com> Co-authored-by: Wenying Dong <wenyingd@vmware.com> Co-authored-by: Mengdie Song <songm@vmware.com> Co-authored-by: Zhecheng Li <zhechel1@uci.edu>

vmwclabot added the cla-not-required label Aug 24, 2020

srikartati force-pushed the loglevel_inc branch from f55e989 to d4c556a Compare August 24, 2020 23:07

tnqn reviewed Aug 25, 2020

View reviewed changes

srikartati force-pushed the loglevel_inc branch 2 times, most recently from 3521a12 to 2ff9b01 Compare August 25, 2020 16:59

srikartati requested a review from tnqn August 27, 2020 17:33

srikartati force-pushed the loglevel_inc branch from 2ff9b01 to f35b481 Compare September 3, 2020 18:25

tnqn reviewed Sep 9, 2020

View reviewed changes

srikartati force-pushed the loglevel_inc branch from f35b481 to cff0449 Compare September 9, 2020 17:35

antoninbas reviewed Sep 10, 2020

View reviewed changes

srikartati requested a review from antoninbas September 11, 2020 22:13

antoninbas reviewed Sep 11, 2020

View reviewed changes

srikartati force-pushed the loglevel_inc branch 2 times, most recently from d2d4d1d to 3db2b3e Compare September 12, 2020 01:09

Enable verbose logging through generate-manifest.sh

9c52270

This patch enables verbose logging for antrea-agent and antrea-controller when generate manifest. This will help in troubleshooting with increased log level.

srikartati force-pushed the loglevel_inc branch from 3db2b3e to 9c52270 Compare September 12, 2020 01:11

srikartati requested review from antoninbas and tnqn September 14, 2020 13:54

tnqn approved these changes Sep 14, 2020

View reviewed changes

srikartati merged commit 165c6e4 into antrea-io:master Sep 14, 2020

srikartati deleted the loglevel_inc branch September 14, 2020 16:19

srikartati mentioned this pull request Sep 14, 2020

Bug in enable verbose log patch #1248

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable verbose logging through generate-manifest.sh #1142

Enable verbose logging through generate-manifest.sh #1142

srikartati commented Aug 24, 2020 •

edited

Loading

antrea-bot commented Aug 24, 2020

tnqn Aug 25, 2020

tnqn Aug 25, 2020

tnqn Aug 25, 2020

srikartati Aug 25, 2020

tnqn Aug 28, 2020

srikartati Sep 1, 2020 •

edited

Loading

tnqn Sep 1, 2020

srikartati Sep 3, 2020 •

edited

Loading

codecov-commenter commented Sep 3, 2020 •

edited

Loading

tnqn left a comment

tnqn Sep 9, 2020

srikartati Sep 9, 2020

srikartati commented Sep 9, 2020

srikartati commented Sep 9, 2020

antoninbas Sep 10, 2020

srikartati Sep 10, 2020 •

edited

Loading

antoninbas Sep 11, 2020

antoninbas left a comment

antoninbas Sep 11, 2020

srikartati Sep 12, 2020

antoninbas Sep 11, 2020

srikartati commented Sep 12, 2020

srikartati commented Sep 14, 2020

tnqn left a comment

srikartati commented Sep 14, 2020

srikartati commented Sep 14, 2020

	To increase the log level for the `antrea-agent` and the `antrea-controller`, you can edit the `-v==0` arg in the Antrea manifest to a desired level. Alternatively, you can generate an Antrea manifest with increased log level of 4 (maximum debug level) using `generate_manifest.sh`:
	To increase the log level for the `antrea-agent` and the `antrea-controller`, you can edit the `-v=0` arg in the Antrea manifest to a desired level. Alternatively, you can generate an Antrea manifest with increased log level of 4 (maximum debug level) using `generate_manifest.sh`:

Enable verbose logging through generate-manifest.sh #1142

Enable verbose logging through generate-manifest.sh #1142

Conversation

srikartati commented Aug 24, 2020 • edited Loading

antrea-bot commented Aug 24, 2020

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

srikartati Sep 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srikartati Sep 3, 2020 • edited Loading

Choose a reason for hiding this comment

codecov-commenter commented Sep 3, 2020 • edited Loading

Codecov Report

tnqn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srikartati commented Sep 9, 2020

srikartati commented Sep 9, 2020

Choose a reason for hiding this comment

srikartati Sep 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

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

srikartati commented Sep 12, 2020

srikartati commented Sep 14, 2020

tnqn left a comment

Choose a reason for hiding this comment

srikartati commented Sep 14, 2020

srikartati commented Sep 14, 2020

srikartati commented Aug 24, 2020 •

edited

Loading

srikartati Sep 1, 2020 •

edited

Loading

srikartati Sep 3, 2020 •

edited

Loading

codecov-commenter commented Sep 3, 2020 •

edited

Loading

srikartati Sep 10, 2020 •

edited

Loading