-
Notifications
You must be signed in to change notification settings - Fork 363
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
Enhancement in support bundle #1145
Conversation
Thanks for your PR. The following commands are available:
|
/test-all |
e630342
to
00602a5
Compare
/test-all |
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.
Some nits, overall LGTM.
pkg/ovs/ovsctl/ofctl.go
Outdated
if len(portItem) > 0 { | ||
rawPortDescItems = append(rawPortDescItems, portItem) | ||
} | ||
portItem = make([]string, 0) |
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.
Is portItem = nil better here?
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.
Sure, I will change.
pkg/support/dump.go
Outdated
for idx := range portsDesc { | ||
portData[idx] = strings.Join(portsDesc[idx], "\n") | ||
} | ||
err = afero.WriteFile(d.fs, filepath.Join(basedir, "ovsports"), []byte(strings.Join(portData, "\n")), 0644) |
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.
Does it make sense to make a separate func for line 237 to 241?
Then you can do:
return d.writeFile("ovsports", portData)
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.
Make sense, I could try to change.
1. Add support to collect ovs/kubelet logs and host network info on Windows Node. 2. Add support to dump OpenFlow ports desc.
/test-all |
Codecov Report
@@ Coverage Diff @@
## master #1145 +/- ##
==========================================
+ Coverage 56.13% 56.19% +0.06%
==========================================
Files 105 105
Lines 11528 11550 +22
==========================================
+ Hits 6471 6491 +20
- Misses 4489 4490 +1
- Partials 568 569 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
/test-networkpolicy |
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
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.
1. Add support to collect ovs/kubelet logs and host network info on Windows Node. 2. Add support to dump OpenFlow ports desc.
Fixes #1144