Support timestamp filter in antctl supportbundle command #2389

hangyan · 2021-07-13T04:28:06Z

This PR is based on #1560, besides conflict fixes, the additional
changes are:

fix a command args help string error
revert the interface changes and use struct fields
create a func to do timefilter cal

Co-authored-by: Weiqiang Tang weiqiangt@vmware.com
Signed-off-by: Hang Yan yhang@vmware.com

codecov-commenter · 2021-07-13T04:33:09Z

Codecov Report

Merging #2389 (0ced809) into main (9e112d4) will increase coverage by 4.74%.
The diff coverage is 57.37%.

@@            Coverage Diff             @@
##             main    #2389      +/-   ##
==========================================
+ Coverage   60.57%   65.31%   +4.74%     
==========================================
  Files         286      286              
  Lines       23053    26506    +3453     
==========================================
+ Hits        13964    17312    +3348     
+ Misses       7624     7598      -26     
- Partials     1465     1596     +131

Flag	Coverage Δ
e2e-tests	`56.26% <45.90%> (?)`
kind-e2e-tests	`47.52% <40.38%> (-0.11%)`	⬇️
unit-tests	`41.40% <23.07%> (-0.32%)`	⬇️

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

Impacted Files	Coverage Δ
pkg/log/log_file.go	`72.41% <50.00%> (+1.36%)`	⬆️
pkg/support/dump.go	`62.50% <50.00%> (-2.62%)`	⬇️
pkg/support/dump_others.go	`58.82% <66.66%> (+8.82%)`	⬆️
...kg/apiserver/registry/system/supportbundle/rest.go	`75.75% <100.00%> (+1.19%)`	⬆️
pkg/controller/egress/ipallocator/allocator.go	`65.00% <0.00%> (-15.42%)`	⬇️
pkg/controller/networkpolicy/endpoint_querier.go	`77.64% <0.00%> (-13.79%)`	⬇️
pkg/apis/controlplane/v1beta1/conversion.go	`72.51% <0.00%> (-12.19%)`	⬇️
pkg/legacyapis/core/v1alpha2/register.go	`69.23% <0.00%> (-10.77%)`	⬇️
pkg/apis/stats/register.go	`71.42% <0.00%> (-10.39%)`	⬇️
pkg/legacyapis/stats/register.go	`71.42% <0.00%> (-10.39%)`	⬇️
... and 274 more

hangyan · 2021-07-13T06:18:15Z

@mengdie-song @tnqn

pkg/antctl/raw/supportbundle/command.go

tnqn

LGTM, a minor comment

pkg/antctl/raw/supportbundle/command.go

pkg/log/log_file.go

mengdie-song

LGTM

antoninbas

Sorry for the late review.
Have you given any thought to using an arbitrary time stamp instead?

BTW, some commits are not signed.

pkg/antctl/raw/supportbundle/command.go

abhiraut

Sorry for the late review.
Have you given any thought to using an arbitrary time stamp instead?

BTW, some commits are not signed.

I have seen usages of timestamp in log filters and can perhaps perform more advanced queries like last 5 hours or between certain timestamps.

pkg/antctl/raw/supportbundle/command.go

hangyan · 2021-07-21T01:45:52Z

Sorry for the late review.
Have you given any thought to using an arbitrary time stamp instead?
BTW, some commits are not signed.

I have seen usages of timestamp in log filters and can perhaps perform more advanced queries like last 5 hours or between certain timestamps.

Use timestamp certainly is better, only require some extra work(reading logs file and extract the required parts).

abhiraut

1 comment otherwise lgtm, but perhaps others may want to chime in on whether to support timestamps or days is fine as a filter

pkg/antctl/raw/supportbundle/command.go

mengdie-song

Thanks for the change!

pkg/support/dump.go

pkg/antctl/raw/supportbundle/command.go

pkg/support/dump.go

tnqn · 2021-08-19T16:13:28Z

pkg/support/dump.go

@@ -184,6 +256,8 @@ type agentDumper struct {
 	ovsCtlClient ovsctl.OVSCtlClient
 	aq           agentquerier.AgentQuerier
 	npq          querier.AgentNetworkPolicyInfoQuerier
+	// days is for log filter


wrong comment

tnqn · 2021-08-19T16:19:06Z

pkg/support/dump_others.go

 		return err
 	}
-	return fileCopy(d.fs, path.Join(basedir, "logs", "ovs"), logDir, "ovs")
+	return directoryCopy(d.fs, path.Join(basedir, "logs", "ovs"), logDir, "ovs", timeFilter)


I have checked the code again. This was by design, alougth i'm not sure it's a good choice. The timefilter is based on the timestamp records in the log filename, which was generated by klog. If no timestamp found in the filename, the whole log file is returned, and it applied to the ovs logs. I assumed it's much smaller than the agent/controller log and does not need to do timestamp filter.

A bad assumption i guess.

tnqn · 2021-08-19T16:19:23Z

pkg/support/dump_test.go

@@ -0,0 +1,21 @@
+package support


Add license header

tnqn · 2021-08-19T16:20:10Z

pkg/support/dump_windows.go

 		return err
 	}
 	// Dump kubelet logs.
-	if err := fileCopy(d.fs, path.Join(basedir, "logs", "kubelet"), antreaWindowsKubeletLogDir, "kubelet"); err != nil {
+	if err := directoryCopy(d.fs, path.Join(basedir, "logs", "kubelet"), antreaWindowsKubeletLogDir, "kubelet", timeFilter); err != nil {


Does kubelet have same time format?

kubelet has the same format.

hangyan · 2021-08-20T03:30:45Z

@tnqn All updated, please take a look if you got time.

tnqn

LGTM, thanks

hangyan · 2021-08-21T03:19:09Z

Thanks for the review

pkg/support/dump.go

tnqn · 2021-08-23T15:53:45Z

/test-all
Will wait for @mengdie-song's confirmation on her comment.

mengdie-song

LGTM

pkg/support/dump.go

This PR is based on antrea-io#1560, besides conflict fixes, the additional changes are: 1. change days filter to timestamp filter 2. revert the interface changes and use struct fields Co-authored-by: Weiqiang Tang <weiqiangt@vmware.com> Signed-off-by: Hang Yan <yhang@vmware.com>

hangyan · 2021-08-24T02:55:03Z

merge conflict are fixed

tnqn · 2021-08-24T04:02:34Z

/test-all

This PR is based on #1560, besides conflict fixes, the additional changes are: 1. change days filter to timestamp filter 2. revert the interface changes and use struct fields Co-authored-by: Weiqiang Tang <weiqiangt@vmware.com> Signed-off-by: Hang Yan <yhang@vmware.com>

mengdie-song reviewed Jul 13, 2021

View reviewed changes

pkg/antctl/raw/supportbundle/command.go Outdated Show resolved Hide resolved

tnqn reviewed Jul 16, 2021

View reviewed changes

pkg/antctl/raw/supportbundle/command.go Outdated Show resolved Hide resolved

mengdie-song reviewed Jul 19, 2021

View reviewed changes

pkg/log/log_file.go Show resolved Hide resolved

mengdie-song previously approved these changes Jul 19, 2021

View reviewed changes

antoninbas reviewed Jul 19, 2021

View reviewed changes

pkg/antctl/raw/supportbundle/command.go Outdated Show resolved Hide resolved

pkg/antctl/raw/supportbundle/command.go Outdated Show resolved Hide resolved

hangyan dismissed mengdie-song’s stale review via 3c6022e July 20, 2021 01:30

hangyan force-pushed the antctl-supportbundle-date-filter-new branch from 3689b11 to 3c6022e Compare July 20, 2021 01:30

abhiraut reviewed Jul 20, 2021

View reviewed changes

pkg/antctl/raw/supportbundle/command.go Outdated Show resolved Hide resolved

hangyan force-pushed the antctl-supportbundle-date-filter-new branch 2 times, most recently from 99380f2 to 4ee6f22 Compare July 21, 2021 01:37

abhiraut reviewed Jul 21, 2021

View reviewed changes

pkg/antctl/raw/supportbundle/command.go Outdated Show resolved Hide resolved

hangyan force-pushed the antctl-supportbundle-date-filter-new branch 2 times, most recently from 60a358e to 74b4ab6 Compare August 18, 2021 01:06

hangyan requested review from abhiraut, antoninbas and mengdie-song August 18, 2021 01:07

hangyan changed the title ~~Support days filter in antctl supportbundle command~~ Support timestamp filter in antctl supportbundle command Aug 18, 2021

mengdie-song reviewed Aug 18, 2021

View reviewed changes

pkg/support/dump.go Outdated Show resolved Hide resolved

pkg/support/dump.go Show resolved Hide resolved

pkg/support/dump.go Outdated Show resolved Hide resolved

pkg/antctl/raw/supportbundle/command.go Outdated Show resolved Hide resolved

tnqn reviewed Aug 18, 2021

View reviewed changes

pkg/support/dump.go Outdated Show resolved Hide resolved

pkg/support/dump.go Outdated Show resolved Hide resolved

pkg/support/dump.go Outdated Show resolved Hide resolved

tnqn reviewed Aug 19, 2021

View reviewed changes

tnqn previously approved these changes Aug 20, 2021

View reviewed changes

mengdie-song reviewed Aug 23, 2021

View reviewed changes

pkg/support/dump.go Show resolved Hide resolved

mengdie-song previously approved these changes Aug 24, 2021

View reviewed changes

pkg/support/dump.go Show resolved Hide resolved

hangyan dismissed stale reviews from mengdie-song and tnqn via 0ced809 August 24, 2021 02:54

hangyan force-pushed the antctl-supportbundle-date-filter-new branch from 5972d51 to 0ced809 Compare August 24, 2021 02:54

mengdie-song approved these changes Aug 24, 2021

View reviewed changes

tnqn approved these changes Aug 24, 2021

View reviewed changes

tnqn merged commit 1d148ad into antrea-io:main Aug 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support timestamp filter in antctl supportbundle command #2389

Support timestamp filter in antctl supportbundle command #2389

hangyan commented Jul 13, 2021

codecov-commenter commented Jul 13, 2021 •

edited

hangyan commented Jul 13, 2021

tnqn left a comment

mengdie-song left a comment

antoninbas left a comment

abhiraut left a comment

hangyan commented Jul 21, 2021

abhiraut left a comment

mengdie-song left a comment

tnqn Aug 19, 2021

tnqn Aug 19, 2021

hangyan Aug 19, 2021 •

edited

hangyan Aug 19, 2021

tnqn Aug 19, 2021

tnqn Aug 19, 2021

hangyan Aug 19, 2021

hangyan commented Aug 20, 2021

tnqn left a comment

hangyan commented Aug 21, 2021

tnqn commented Aug 23, 2021

mengdie-song left a comment

hangyan commented Aug 24, 2021

tnqn commented Aug 24, 2021

Support timestamp filter in antctl supportbundle command #2389

Support timestamp filter in antctl supportbundle command #2389

Conversation

hangyan commented Jul 13, 2021

codecov-commenter commented Jul 13, 2021 • edited

Codecov Report

hangyan commented Jul 13, 2021

tnqn left a comment

Choose a reason for hiding this comment

mengdie-song left a comment

Choose a reason for hiding this comment

antoninbas left a comment

Choose a reason for hiding this comment

abhiraut left a comment

Choose a reason for hiding this comment

hangyan commented Jul 21, 2021

abhiraut left a comment

Choose a reason for hiding this comment

mengdie-song left a comment

Choose a reason for hiding this comment

tnqn Aug 19, 2021

Choose a reason for hiding this comment

tnqn Aug 19, 2021

Choose a reason for hiding this comment

hangyan Aug 19, 2021 • edited

Choose a reason for hiding this comment

hangyan Aug 19, 2021

Choose a reason for hiding this comment

tnqn Aug 19, 2021

Choose a reason for hiding this comment

tnqn Aug 19, 2021

Choose a reason for hiding this comment

hangyan Aug 19, 2021

Choose a reason for hiding this comment

hangyan commented Aug 20, 2021

tnqn left a comment

Choose a reason for hiding this comment

hangyan commented Aug 21, 2021

tnqn commented Aug 23, 2021

mengdie-song left a comment

Choose a reason for hiding this comment

hangyan commented Aug 24, 2021

tnqn commented Aug 24, 2021

codecov-commenter commented Jul 13, 2021 •

edited

hangyan Aug 19, 2021 •

edited