Skip to content
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

Jump to bottom

Support return partial results for supportbundle when some nodes fails #2399

Merged
merged 3 commits into from Aug 19, 2021
Merged

Support return partial results for supportbundle when some nodes fails #2399

merged 3 commits into from Aug 19, 2021

Conversation

hangyan
Copy link
Member

@hangyan hangyan commented Jul 14, 2021

Signed-off-by: Hang Yan yhang@vmware.com

A sample outpout:
image

@hangyan
Copy link
Member Author

hangyan commented Jul 14, 2021

@mengdie-song @tnqn

@codecov-commenter
Copy link

codecov-commenter commented Jul 14, 2021 •
edited

Codecov Report

Merging #2399 (ce67f45) into main (bdddc01) will increase coverage by 4.91%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2399      +/-   ##
==========================================
+ Coverage   60.27%   65.19%   +4.91%     
==========================================
  Files         282      283       +1     
  Lines       22345    25774    +3429     
==========================================
+ Hits        13469    16803    +3334     
+ Misses       7452     7419      -33     
- Partials     1424     1552     +128
Flag Coverage Δ
e2e-tests 56.06% <ø> (?)
kind-e2e-tests 47.40% <ø> (+0.21%) ⬆️
unit-tests 41.97% <ø> (-0.23%) ⬇️

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

Impacted Files Coverage Δ
pkg/controller/egress/ipallocator/allocator.go 67.82% <0.00%> (-15.16%) ⬇️
pkg/controller/networkpolicy/endpoint_querier.go 77.64% <0.00%> (-13.79%) ⬇️
pkg/apis/controlplane/v1beta1/conversion.go 72.44% <0.00%> (-11.89%) ⬇️
pkg/legacyapis/core/v1alpha2/register.go 69.23% <0.00%> (-10.77%) ⬇️
pkg/controller/egress/controller.go 76.76% <0.00%> (-10.44%) ⬇️
pkg/apis/stats/register.go 71.42% <0.00%> (-10.39%) ⬇️
pkg/legacyapis/stats/register.go 71.42% <0.00%> (-10.39%) ⬇️
pkg/ovs/openflow/ofctrl_meter.go 33.84% <0.00%> (-10.16%) ⬇️
pkg/legacyapis/security/v1alpha1/register.go 73.33% <0.00%> (-10.00%) ⬇️
pkg/agent/config/traffic_encap_mode.go 91.30% <0.00%> (-8.70%) ⬇️
... and 268 more

mengdie-song
mengdie-song reviewed Jul 14, 2021
View reviewed changes
Copy link
Contributor

@mengdie-song mengdie-song left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the change. Besides standard output, we can also output the failed nodes to a file into the support bundle if there is any failed node.

@hangyan
Copy link
Member Author

hangyan commented Jul 15, 2021

Thanks for the change. Besides standard output, we can also output the failed nodes to a file into the support bundle if there is any failed node.

Updated result:
image

mengdie-song
mengdie-song reviewed Jul 16, 2021
View reviewed changes
Copy link
Contributor

@mengdie-song mengdie-song left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the change.

pkg/antctl/raw/supportbundle/command.go Show resolved Hide resolved
pkg/antctl/raw/supportbundle/command.go Outdated Show resolved Hide resolved
@hangyan hangyan force-pushed the supportbundle-ignore-failed branch from 26af354 to 3d1d079 Compare August 16, 2021 02:46
@hangyan
Copy link
Member Author

hangyan commented Aug 16, 2021

@mengdie-song I have rebased this branch with master and please take another look if this PR is ready to merge.

mengdie-song
mengdie-song reviewed Aug 16, 2021
View reviewed changes
Copy link
Contributor

@mengdie-song mengdie-song left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing my previous comments. I still have some minor ones, please take a look.

pkg/antctl/raw/supportbundle/command.go Outdated Show resolved Hide resolved
pkg/antctl/raw/supportbundle/command.go Outdated Show resolved Hide resolved
pkg/antctl/raw/supportbundle/command.go Show resolved Hide resolved
pkg/antctl/raw/supportbundle/command.go Show resolved Hide resolved
pkg/antctl/raw/supportbundle/command.go Show resolved Hide resolved
@hangyan
Update function comments
5ab3eff 
Signed-off-by: Hang Yan <yhang@vmware.com>
mengdie-song
mengdie-song previously approved these changes Aug 18, 2021
View reviewed changes
Copy link
Contributor

@mengdie-song mengdie-song left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

tnqn
tnqn reviewed Aug 18, 2021
View reviewed changes
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two minor comments

pkg/antctl/raw/supportbundle/command.go Outdated Show resolved Hide resolved
pkg/antctl/raw/supportbundle/command.go
}
return nil
return results
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could it return map[string]error so requestAll and downloadAll looks neater?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

@hangyan
Refactor code
ce67f45 
Signed-off-by: Hang Yan <yhang@vmware.com>
tnqn
tnqn approved these changes Aug 19, 2021
View reviewed changes
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tnqn
Copy link
Member

tnqn commented Aug 19, 2021

/test-all

@tnqn tnqn merged commit b10a794 into antrea-io:main Aug 19, 2021
@hangyan hangyan mentioned this pull request Sep 16, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tnqn tnqn tnqn approved these changes

@mengdie-song mengdie-song mengdie-song approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@hangyan @codecov-commenter @tnqn @mengdie-song