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

Incorporate review comments #672

Merged
merged 2 commits into from
Jan 4, 2022
Merged

Conversation

sayboras
Copy link
Member

@sayboras sayboras commented Jan 4, 2022

Description

To incorporate review comments in #671

Testing

Sanity check was done locally with kind cluster to make sure there is no silly mistake :)

Sanity check
$ ./cilium sysdump         
🔍 Collecting Kubernetes nodes
🔍 Collect Kubernetes nodes
🔍 Collect Kubernetes version
🔍 Collecting Kubernetes pods
...
🔍 Collecting 'cilium-bugtool' output from Cilium pods
🔍 Collecting logs from Hubble pods
🔍 Collecting logs from Hubble Relay pods
🔍 Collecting platform-specific data
🔍 Collecting Hubble flows from Cilium pods
⚠️ The following tasks failed, the sysdump may be incomplete:
⚠️ [10] Collecting Cilium egress NAT policies: failed to collect Cilium egress NAT policies: the server could not find the requested resource (get ciliumegressnatpolicies.cilium.io)
⚠️ [11] Collecting Cilium local redirect policies: failed to collect Cilium local redirect policies: the server could not find the requested resource (get ciliumlocalredirectpolicies.cilium.io)
⚠️ [19] Collecting the Hubble Relay configuration: failed to collect the Hubble Relay configuration: configmaps "hubble-relay-config" not found
⚠️ hubble-flows-cilium-gnvqz: failed to collect hubble flows for "cilium-gnvqz" in namespace "kube-system": command terminated with exit code 1: failed to connect to 'unix:///var/run/cilium/hubble.sock': connection error: desc = "transport: error while dialing: dial unix /var/run/cilium/hubble.sock: connect: no such file or directory"

⚠️ Please note that depending on your Cilium version and installation options, this may be expected
🗳 Compiling sysdump
✅ The sysdump has been saved to /home/tammach/go/src/github.com/cilium/cilium-cli/cilium-sysdump-20220105-001818.zip

$ ls -lrht cilium-sysdump-20220105-001818.zip 
-rw-rw-r-- 1 tammach tammach 5.0M Jan  5 00:18 cilium-sysdump-20220105-001818.zip

@maintainer-s-little-helper
Copy link

Commits 33dccf4, ee61e82 do not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

This commit is to correct/add header with year 2022 for new files.

Relates cilium#671 (comment)

Signed-off-by: Tam Mach <[email protected]>
Replace os.Create by os.OpenFile for more granular permissions and flags.
Also, adding defer function to close the file once done as per review
comment.

Signed-off-by: Tam Mach <[email protected]>
@sayboras sayboras force-pushed the tam/review-comments branch from ee61e82 to cb34a69 Compare January 4, 2022 13:23
@sayboras sayboras temporarily deployed to ci January 4, 2022 13:23 Inactive
@sayboras sayboras changed the title Tam/review comments Incorporate review comments Jan 4, 2022
@sayboras sayboras marked this pull request as ready for review January 4, 2022 13:30
@sayboras sayboras requested review from a team and aanm January 4, 2022 13:30
@christarazi christarazi merged commit c533fa3 into cilium:master Jan 4, 2022
@sayboras sayboras deleted the tam/review-comments branch January 4, 2022 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants