-
Notifications
You must be signed in to change notification settings - Fork 230
USHIFT-2226: Test Multus - Bridge CNI #3137
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| apiVersion: "k8s.cni.cncf.io/v1" | ||
| kind: NetworkAttachmentDefinition | ||
| metadata: | ||
| name: bridge-conf | ||
| spec: | ||
| config: '{ | ||
| "cniVersion": "0.4.0", | ||
| "type": "bridge", | ||
| "bridge": "br-test", | ||
| "mode": "bridge", | ||
| "ipam": { | ||
| "type": "host-local", | ||
| "ranges": [ | ||
| [ | ||
| { | ||
| "subnet": "10.10.0.0/24", | ||
| "rangeStart": "10.10.0.20", | ||
| "rangeEnd": "10.10.0.50", | ||
| "gateway": "10.10.0.254" | ||
| } | ||
| ] | ||
| ], | ||
| "dataDir": "/var/lib/cni/br-test" | ||
| } | ||
| }' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| kind: Pod | ||
| apiVersion: v1 | ||
| metadata: | ||
| name: test-bridge | ||
| annotations: | ||
| k8s.v1.cni.cncf.io/networks: bridge-conf | ||
| labels: | ||
| app: test-bridge | ||
| spec: | ||
| terminationGracePeriodSeconds: 0 | ||
| containers: | ||
| - name: hello-microshift | ||
| image: quay.io/microshift/busybox:1.36 | ||
| command: ["/bin/sh"] | ||
| args: ["-c", "while true; do echo -ne \"HTTP/1.0 200 OK\r\nContent-Length: 16\r\n\r\nHello MicroShift\" | nc -l -p 8080 ; done"] | ||
| ports: | ||
| - containerPort: 8080 | ||
| protocol: TCP | ||
| securityContext: | ||
| allowPrivilegeEscalation: false | ||
| capabilities: | ||
| drop: | ||
| - ALL | ||
| runAsNonRoot: true | ||
| runAsUser: 1001 | ||
| runAsGroup: 1001 | ||
| seccompProfile: | ||
| type: RuntimeDefault | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| apiVersion: "k8s.cni.cncf.io/v1" | ||
| kind: NetworkAttachmentDefinition | ||
| metadata: | ||
| name: bridge-preexisting-conf | ||
| spec: | ||
| config: '{ | ||
| "cniVersion": "0.4.0", | ||
| "type": "bridge", | ||
| "bridge": "br-preexisting", | ||
| "mode": "bridge", | ||
| "ipam": { | ||
| "type": "host-local", | ||
| "ranges": [ | ||
| [ | ||
| { | ||
| "subnet": "10.10.1.0/24", | ||
| "rangeStart": "10.10.1.20", | ||
| "rangeEnd": "10.10.1.50", | ||
| "gateway": "10.10.1.254" | ||
| } | ||
| ] | ||
| ], | ||
| "dataDir": "/var/lib/cni/br-preexisting" | ||
| } | ||
| }' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| kind: Pod | ||
| apiVersion: v1 | ||
| metadata: | ||
| name: test-bridge-preexisting | ||
| annotations: | ||
| k8s.v1.cni.cncf.io/networks: bridge-preexisting-conf | ||
| labels: | ||
| app: test-bridge-preexisting | ||
| spec: | ||
| terminationGracePeriodSeconds: 0 | ||
| containers: | ||
| - name: hello-microshift | ||
| image: quay.io/microshift/busybox:1.36 | ||
| command: ["/bin/sh"] | ||
| args: ["-c", "while true; do echo -ne \"HTTP/1.0 200 OK\r\nContent-Length: 16\r\n\r\nHello MicroShift\" | nc -l -p 8080 ; done"] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to add "sleep" here not to burn CPU?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't spin furiously - |
||
| ports: | ||
| - containerPort: 8080 | ||
| protocol: TCP | ||
| securityContext: | ||
| allowPrivilegeEscalation: false | ||
| capabilities: | ||
| drop: | ||
| - ALL | ||
| runAsNonRoot: true | ||
| runAsUser: 1001 | ||
| runAsGroup: 1001 | ||
| seccompProfile: | ||
| type: RuntimeDefault | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| *** Settings *** | ||
| Documentation Tests for Multus and Bridge plugin on MicroShift | ||
|
|
||
| Resource ../../resources/common.resource | ||
| Resource ../../resources/microshift-process.resource | ||
|
|
||
| Suite Setup Setup Suite With Namespace | ||
| Suite Teardown Teardown Suite With Namespace | ||
|
|
||
|
|
||
| *** Variables *** | ||
| ${BRIDGE_INTERFACE} br-test | ||
| ${BRIDGE_NAD_YAML} ./assets/multus/bridge-nad.yaml | ||
| ${BRIDGE_POD_YAML} ./assets/multus/bridge-pod.yaml | ||
| ${BRIDGE_POD_NAME} test-bridge | ||
| ${BRIDGE_IP} 10.10.0.10/24 | ||
|
|
||
| ${PE_BRIDGE_INTERFACE} br-preexisting | ||
| ${PE_BRIDGE_NAD_YAML} ./assets/multus/bridge-preexisting-nad.yaml | ||
| ${PE_BRIDGE_POD_YAML} ./assets/multus/bridge-preexisting-pod.yaml | ||
| ${PE_BRIDGE_POD_NAME} test-bridge-preexisting | ||
| ${PE_BRIDGE_IP} 10.10.1.10/24 | ||
|
|
||
|
|
||
| *** Test Cases *** | ||
| Pre-Existing Bridge Interface | ||
| [Documentation] Test verifies if Bridge CNI plugin will work correctly with pre-existing interface. | ||
| [Setup] Run Keywords | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You don't need to change it, but I'm curious why there are so many individual keywords in the setup for this case. Why not include them in the test body, or write a separate keyword?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I was being literal - test setup in
I didn't like how this requires a jump to different location in the file - this way everything is single place. So I think I just felt like it :D It's not perfect though, this keyword (in this or future PR) is maxing out the length, but separate keyword was "context jump" and writing general function for both Bridge tests was quite ugly and required some mental processing on what happens with which arguments.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It can definitely be hard to find a balance between inline and separate code blocks. That max keyword aspect to the linter is a nudge towards what the linter author thought we should be doing, but there have definitely been cases where we overrode the rule. Maybe we should have written more of those cases in Python? I don't know. As I said, there's no need to change anything here, I was just curious about the evolution of the code.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Perhaps, but python function don't support so great support for log.html. We'll see in the future |
||
| ... Interface Should Not Exist ${PE_BRIDGE_INTERFACE} | ||
| ... AND | ||
| ... Create Interface ${PE_BRIDGE_INTERFACE} | ||
| ... AND | ||
| ... Create NAD And Pod ${PE_BRIDGE_NAD_YAML} ${PE_BRIDGE_POD_YAML} | ||
| ... AND | ||
| ... Named Pod Should Be Ready ${PE_BRIDGE_POD_NAME} ${NAMESPACE} | ||
| ... AND | ||
| ... Interface Should Exist ${PE_BRIDGE_INTERFACE} | ||
| ... AND | ||
| ... Set IP For Host Interface ${PE_BRIDGE_INTERFACE} ${PE_BRIDGE_IP} | ||
|
|
||
| Connect To Pod Over Local Interface ${PE_BRIDGE_POD_NAME} ${NAMESPACE} ${PE_BRIDGE_INTERFACE} | ||
|
|
||
| [Teardown] Cleanup Bridge Test | ||
| ... ${PE_BRIDGE_NAD_YAML} | ||
| ... ${PE_BRIDGE_POD_YAML} | ||
| ... ${PE_BRIDGE_INTERFACE} | ||
|
|
||
| No Pre-Existing Bridge Interface | ||
| [Documentation] Test verifies if Bridge CNI plugin will | ||
| ... work correctly if there is no pre-existing bridge | ||
| ... interface - it needs to be created. | ||
| [Setup] Run Keywords | ||
| ... Interface Should Not Exist ${BRIDGE_INTERFACE} | ||
| ... AND | ||
| ... Create NAD And Pod ${BRIDGE_NAD_YAML} ${BRIDGE_POD_YAML} | ||
| ... AND | ||
| ... Named Pod Should Be Ready ${BRIDGE_POD_NAME} ${NAMESPACE} | ||
| ... AND | ||
| ... Interface Should Exist ${BRIDGE_INTERFACE} | ||
| ... AND | ||
| ... Set IP For Host Interface ${BRIDGE_INTERFACE} ${BRIDGE_IP} | ||
|
|
||
| Connect To Pod Over Local Interface ${BRIDGE_POD_NAME} ${NAMESPACE} ${BRIDGE_INTERFACE} | ||
|
|
||
| [Teardown] Cleanup Bridge Test | ||
| ... ${BRIDGE_NAD_YAML} | ||
| ... ${BRIDGE_POD_YAML} | ||
| ... ${BRIDGE_INTERFACE} | ||
|
|
||
|
|
||
| *** Keywords *** | ||
| Create NAD And Pod | ||
| [Documentation] Creates provided NetworkAttachmentDefinition and Pod. | ||
| [Arguments] ${nad} ${pod} | ||
| Oc Create -n ${NAMESPACE} -f ${nad} | ||
| Oc Create -n ${NAMESPACE} -f ${pod} | ||
|
|
||
| Cleanup Bridge Test | ||
| [Documentation] Removes provided NetworkAttachmentDefinition, Pod and network interface to allow for test rerun. | ||
| [Arguments] ${nad} ${pod} ${if} | ||
| Run Keyword And Continue On Failure | ||
| ... Oc Delete -n ${NAMESPACE} -f ${pod} | ||
| Run Keyword And Continue On Failure | ||
| ... Oc Delete -n ${NAMESPACE} -f ${nad} | ||
| Command Should Work ip link delete ${if} | ||
|
|
||
| Connect To Pod Over Local Interface | ||
| [Documentation] Makes a HTTP request to 8080 for a given Pod over given interface. | ||
| [Arguments] ${pod} ${ns} ${if} | ||
|
|
||
| ${networks}= Get And Verify Pod Networks ${pod} ${ns} | ||
| ${extra_ip}= Set Variable ${networks}[1][ips][0] | ||
|
|
||
| ${stdout}= Command Should Work curl -v --interface ${if} ${extra_ip}:8080 | ||
| Should Contain ${stdout} Hello MicroShift | ||
|
|
||
| Interface Should Not Exist | ||
| [Documentation] Verifies that network interface does not exist. | ||
| [Arguments] ${if} | ||
| Command Should Fail ip link show ${if} | ||
|
|
||
| Create Interface | ||
| [Documentation] Creates network interface. | ||
| [Arguments] ${if} ${type}=bridge | ||
| Command Should Work ip link add dev ${if} type ${type} | ||
|
|
||
| Interface Should Exist | ||
| [Documentation] Verifies that interface exists on the host. | ||
| [Arguments] ${if} | ||
| Command Should Work ip link show ${if} | ||
|
|
||
| Set IP For Host Interface | ||
| [Documentation] Sets IP address for the interface. | ||
| [Arguments] ${if} ${cidr} | ||
| Command Should Work ip addr add ${cidr} dev ${if} | ||
|
|
||
| Get And Verify Pod Networks | ||
| [Documentation] Obtains interfaces of the Pod from its annotation. | ||
| ... The annotation is managed by Multus. | ||
| [Arguments] ${pod} ${ns} | ||
|
|
||
| ${networks_str}= Oc Get JsonPath | ||
| ... pod | ||
| ... ${ns} | ||
| ... ${pod} | ||
| ... .metadata.annotations.k8s\\.v1\\.cni\\.cncf\\.io/network-status | ||
| Should Not Be Empty ${networks_str} | ||
|
|
||
| ${networks}= Json Parse ${networks_str} | ||
| ${n}= Get Length ${networks} | ||
| Should Be Equal As Integers ${n} 2 | ||
| Should Be Equal As Strings ${networks}[0][name] ovn-kubernetes | ||
| Should Match ${networks}[1][name] ${NAMESPACE}/bridge*-conf | ||
|
|
||
| RETURN ${networks} | ||
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.
Do we want to add "sleep" here not to burn CPU?
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 doesn't spin furiously -
ncblocks until a client arrives, so the loop has a many iterations as many GETs.