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
Update and Activate ovn_ipsec_connectivity
#37
Update and Activate ovn_ipsec_connectivity
#37
Conversation
/retest |
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.
Thanks for the PR Andrew! couple of questions inline.
# TODO check with oc get network.operator.openshift.io/cluster -o=jsonpath='{.items[*].spec.defaultNetwork.ovnKubernetesConfig.ipsecConfig}' | ||
# once tests can be run with real cluster |
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.
we can't run it using cluster-bot now ?
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.
not yet unfortunately I think Mark has some work in the pipeline to be able to do so though
} | ||
|
||
help() |
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.
this func has to be changed in accordance to this script.
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.
Ack sorry missed that
debug-scripts/common
Outdated
@@ -175,3 +199,12 @@ create_host_network_pod_on_node () { | |||
sleep 2 | |||
oc wait -n "$NAMESPACE" --for=condition=Ready pod/"$POD_NAME" --timeout=3m | |||
} | |||
|
|||
format_sdout () { |
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.
nit: stdout
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.
Yep I actually didn't mean to have the color formatting in this PR... Sorry about that!
4b0fff5
to
720912a
Compare
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
just a few nits, wouldn't block merge. As noted, we still need to do some improvements to the ipsec script once we have a better way to spawn an ipsec cluster on openshift.
debug-scripts/common
Outdated
@@ -174,4 +191,4 @@ create_host_network_pod_on_node () { | |||
# wait till pod is running | |||
sleep 2 | |||
oc wait -n "$NAMESPACE" --for=condition=Ready pod/"$POD_NAME" --timeout=3m | |||
} | |||
} |
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.
woops, we actually need the newline.
debug-scripts/ovn_ipsec_connectivity
Outdated
|
||
global_namespace="${1}" | ||
|
||
main |
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.
ditto...
/assign @rcarrillocruz |
Cleanup the info logging Make the tcpdump command run in a dedicated host-networked container copy pcap capture back to the network-tools container TODO: Copy this file out to the local machine Fix some empty fields erros In some of these functions they didn't seem to work if the namespace or other parameters were not specified This commit just adds some error checking to those functions Signed-off-by: Andrew Stoycos <astoycos@redhat.com>
Activiates this test for the network-tools image Script will check for ipsec enablement internally and only run if so Signed-off-by: Andrew Stoycos <astoycos@redhat.com>
720912a
to
8f259c0
Compare
New changes are detected. LGTM label has been removed. |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: astoycos, rcarrillocruz, tssurya 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 |
/retest |
2 similar comments
/retest |
/retest |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: astoycos, rcarrillocruz, tssurya 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 |
/retest Please review the full test history for this PR and help us cut down flakes. |
Update and Activate `ovn_ipsec_connectivity`
Activate IPSEC test, and convert test to use built in functions described in the
common
scriptWhen run it will check if node to node traffic in an OVN-K cluster is encrypted with IPSEC
Output will resemble the following
This PR also adds some fixes to common functions (see commit messages for more detail)
TODO: The script generates a PCAP which should prove that the traffic is encrypted... This file is copied to the network-tools image and should be accessible to the user at some point