Minor fixes for gather_network_logs #243
Conversation
… than service's logs) Fix ip route gathering (command should be evaluated inside OVNKUBE_NODE_POD) Fix evaluating if ipsec is enabled (condition evaluation previously failed, always returning False condition result) Add How to run section to README.md
openshift-ci
bot
added
bugzilla/severity-high
|
@npinaeva: This pull request references Bugzilla bug 1970129, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Bugzilla (anusaxen@redhat.com), skipping review request. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
| # Call format: NODE [file_paths] | ||
| # Collects log files from given paths, | ||
| # path should be relative to the node's /var/logs/ folder, | ||
| # e.g. openvswitch/ovs-vswitchd.log will be collected to "$NETWORK_LOG_PATH"/"$NODE"_ovs-vswitchd.log |
There was a problem hiding this comment.
we already capture ovs-vswitchd.log as part of must-gather now:
The issue is we need to set the logging levels in it, by doing ovs-vsctl vlog/set INFO for example. It looks like we just use to set it to INFO as shown here:
https://github.com/openshift/cluster-network-operator/blob/release-4.6/bindata/network/ovn-kubernetes/006-ovs-node.yaml#L92
We previously ran this OVS container in 4.6 that took care of this, but we no longer have it since 4.7. So log levels are not getting set correctly. I think you will need a PR in CNO to set the log level for OVS in the ovnkube-node template:
https://github.com/openshift/cluster-network-operator/blob/release-4.6/bindata/network/ovn-kubernetes/ovnkube-node.yaml
or alternatively you could put it in MCO:
https://github.com/openshift/machine-config-operator/blob/master/templates/common/_base/files/configure-ovs-network.yaml
There was a problem hiding this comment.
Thanks for the comment, I will open different PR for the bug fix, and leave minor fixes in this one
npinaeva
changed the title
openshift-ci
bot
removed
the
bugzilla/severity-high
|
@npinaeva: No Bugzilla bug is referenced in the title of this pull request. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci
bot
removed
the
bugzilla/valid-bug
|
/retest |
|
Reproducing: run Before In gathered files: After |
|
@davemulford, @sferich888: I think this is the final version of the PR, added a comment about testing changes. I will appreciate if you could take a look. |
|
/lgtm |
| @@ -10,3 +10,7 @@ Data collection scripts are kept in `./collection-scripts`. The content of that | |||
| The data collection scripts should only include collection logic for components that are included as part of the OpenShift | |||
| CVO payload. Outside components are encouraged to produce a similar "must-gather" image, but this is not the spot to be | |||
| included. | |||
|
|
|||
| ## How to run | |||
There was a problem hiding this comment.
Should we link to https://docs.okd.io/latest/support/gathering-cluster-data.html#support_gathering_data_gathering-cluster-data (or something similar on how to run advanced commands, later in these docs)?
There was a problem hiding this comment.
Sure, that may be useful
| echo "INFO: No ovn-ipsec pods exist, tunnel traffic will be unencrypted" | ||
| else | ||
| echo "INFO: ovn-ipsec is enabled, tunnel traffic should be encryted" | ||
| echo "INFO: ovn-ipsec is enabled, tunnel traffic should be encrypted" |
There was a problem hiding this comment.
With this being true is running gather_ovn_ipsec_data valuable?
Will we be decrypting it to use what we collect? If we do decrypt it do we need to tell the user? It doesn't look like we collect any traffic only how we shape/direct the traffic so these question may not matter, and if so I don't know if the messages/logs are needed (but they are not harmful).
openshift-ci
bot
added
the
approved
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dcbw, npinaeva, sferich888 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Fix ip route gathering (command should be evaluated inside OVNKUBE_NODE_POD)
Fix evaluating if ipsec is enabled (condition evaluation previously failed, always returning False condition result)
Add How to run section to README.md