-
Notifications
You must be signed in to change notification settings - Fork 230
USHIFT-5818: Generic Device Plugin - Sanity Smoke Test #5121
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
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
60 changes: 60 additions & 0 deletions
60
test/assets/generic-device-plugin/fake-serial-communication.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| #!/usr/bin/env python3 | ||
|
|
||
| """ | ||
| This script simulates a simple serial communication between a host and a pod. | ||
| It supports two modes of operation: 'host' and 'pod': | ||
| - Host listens for a message and responds. | ||
| - Pod sends a message and waits for a response. | ||
| Both sides are verifying the messages they send and receive. | ||
| """ | ||
|
|
||
| import serial | ||
| import sys | ||
|
|
||
| DEVICE_POD = "/dev/ttyPipeB0" | ||
| DEVICE_HOST = "/dev/ttyPipeA0" | ||
|
|
||
| MSG_1 = b"HELLO\n" | ||
| MSG_2 = b"THERE\n" | ||
|
|
||
|
|
||
| def send_msg(ser, msg): | ||
| print(f"Sending message: {msg}") | ||
| ser.write(msg) | ||
|
|
||
|
|
||
| def recv_msg(ser, expected_msg): | ||
| print(f"Listening for a message. Expecting: {expected_msg}") | ||
| line = ser.readline() | ||
| print(f"Received message: {line}") | ||
| if expected_msg != line: | ||
| print("Received message does not match expected one") | ||
| sys.exit(1) | ||
|
|
||
|
|
||
| def host(): | ||
| s = serial.Serial(DEVICE_HOST, timeout=60) | ||
| recv_msg(s, MSG_1) | ||
| send_msg(s, MSG_2) | ||
| print("Test successful") | ||
|
|
||
|
|
||
| def pod(): | ||
| s = serial.Serial(DEVICE_POD, timeout=10) | ||
| send_msg(s, MSG_1) | ||
| recv_msg(s, MSG_2) | ||
| print("Test successful") | ||
|
|
||
|
|
||
| if len(sys.argv) == 1: | ||
| print("Not enough args") | ||
| sys.exit(1) | ||
|
|
||
| mode = sys.argv[1] | ||
| if mode == "host": | ||
| host() | ||
| elif mode == "pod": | ||
| pod() | ||
| else: | ||
| print(f"Invalid arg: {mode}. Accepted args: host | pod") | ||
| sys.exit(1) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| apiVersion: batch/v1 | ||
| kind: Job | ||
| metadata: | ||
| name: gdp-test | ||
| spec: | ||
| template: | ||
| spec: | ||
| restartPolicy: Never | ||
| containers: | ||
| - name: gdp-test | ||
| image: registry.access.redhat.com/ubi9/ubi:9.6 | ||
| command: ["/bin/bash"] | ||
| args: ["/script/entrypoint.sh", "pod"] | ||
| resources: | ||
| limits: | ||
| device.microshift.io/fakeserial: 1 | ||
| securityContext: | ||
| allowPrivilegeEscalation: false | ||
| capabilities: | ||
| drop: ["ALL"] | ||
| runAsNonRoot: true | ||
| seccompProfile: | ||
| type: "RuntimeDefault" | ||
| volumeMounts: | ||
| - name: script-volume | ||
| mountPath: /script/ | ||
| volumes: | ||
| - name: script-volume | ||
| configMap: | ||
| name: gdp-script | ||
| defaultMode: 0755 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| *** Settings *** | ||
| Documentation Generic Device Plugin | ||
|
|
||
| Resource ../../resources/microshift-config.resource | ||
| Resource ../../resources/microshift-process.resource | ||
| Variables strings.py | ||
| Library strings.py | ||
|
|
||
| Suite Setup Setup Suite With Namespace | ||
| Suite Teardown Teardown Suite With Namespace | ||
|
|
||
|
|
||
| *** Variables *** | ||
| ${NAMESPACE} ${EMPTY} | ||
|
|
||
|
|
||
| *** Test Cases *** | ||
| Sanity Test | ||
| [Documentation] Performs a simple test of Generic Device Plugin | ||
| [Setup] Run Keywords | ||
| ... Enable And Configure GDP | ||
| ... Enable Serialsim | ||
| ... Copy Script To Host | ||
|
|
||
| Wait Until Device Is Allocatable | ||
|
|
||
| Command Should Work crictl pull registry.access.redhat.com/ubi9/ubi:9.6 | ||
| Start Script On Host | ||
| Create Test Job | ||
|
|
||
| Wait For Job Completion And Check Logs | ||
|
|
||
| [Teardown] Run Keywords | ||
| ... Stop Script On Host | ||
| ... Disable GDP | ||
|
|
||
|
|
||
| *** Keywords *** | ||
| Enable And Configure GDP | ||
| [Documentation] Enables GDP and adds fake device path in MicroShift configuration | ||
| Drop In MicroShift Config ${GDP_CONFIG_DROPIN} 10-gdp | ||
| Restart MicroShift | ||
|
|
||
| Disable GDP | ||
| [Documentation] Removes GDP configuration drop-in | ||
| Remove Drop In MicroShift Config 10-gdp | ||
| Restart MicroShift | ||
|
|
||
| Enable Serialsim | ||
| [Documentation] Enables the serialsim kernel module. | ||
| ... serialsim creates echo and pipe devices. | ||
| Command Should Work modprobe serialsim | ||
|
|
||
| Copy Script To Host | ||
| [Documentation] Starts fake serial communication script to the host | ||
| Put File | ||
| ... ./assets/generic-device-plugin/fake-serial-communication.py | ||
| ... /tmp/fake-serial-communication.py | ||
|
|
||
| Start Script On Host | ||
| [Documentation] Starts fake serial communication script on the host in the background | ||
| ${cmd}= Catenate | ||
| ... systemd-run -u gdp-test-comm | ||
| ... python /tmp/fake-serial-communication.py host | ||
| Command Should Work ${cmd} | ||
|
|
||
| Stop Script On Host | ||
| [Documentation] Attempts to stop the fake serial communication script on the host. | ||
| ... If it was successful, the unit is deleted automatically. | ||
| Command Execution journalctl -u gdp-test-comm.service | ||
| Command Execution systemctl stop gdp-test-comm | ||
| Command Execution systemctl reset-failed gdp-test-comm | ||
|
|
||
| Create Test Job | ||
| [Documentation] Creates Job that spawns test Pod running to completion. | ||
| ${script}= OperatingSystem.Get File ./assets/generic-device-plugin/fake-serial-communication.py | ||
| ${configmap}= Append To Preamble ${script} | ||
| Log ${configmap} | ||
| ${path}= Create Random Temp File ${configmap} | ||
| Oc Create -f ${path} -n ${NAMESPACE} | ||
| Oc Create -f ./assets/generic-device-plugin/job.yaml -n ${NAMESPACE} | ||
|
|
||
| Wait Until Device Is Allocatable | ||
| [Documentation] Waits until device device.microshift.io/fakeserial is allocatable | ||
| ${node}= Run With Kubeconfig oc get node -o=name | ||
| ${node_name}= Remove String ${node} node/ | ||
|
|
||
| Wait Until Keyword Succeeds 60s 5s | ||
| ... Device Should Be Allocatable ${node_name} | ||
|
|
||
| Device Should Be Allocatable | ||
| [Documentation] Checks if device device.microshift.io/fakeserial is allocatable | ||
| [Arguments] ${node_name} | ||
| ${device_amount}= Oc Get JsonPath | ||
| ... node | ||
| ... ${EMPTY} | ||
| ... ${node_name} | ||
| ... .status.allocatable.device\\.microshift\\.io/fakeserial | ||
| Should Be Equal As Integers ${device_amount} 1 | ||
|
|
||
| Wait For Job Completion And Check Logs | ||
| [Documentation] Waits for Job completion and checks Pod logs looking for 'Test successful' message | ||
|
|
||
| Oc Wait -n ${NAMESPACE} job/gdp-test --for=condition=complete --timeout=120s | ||
| ${pod}= Oc Get JsonPath | ||
| ... pod | ||
| ... ${NAMESPACE} | ||
| ... --selector=batch.kubernetes.io/job-name=gdp-test | ||
| ... .items[*].metadata.name | ||
| ${logs}= Oc Logs ${pod} ${NAMESPACE} | ||
| Should Contain ${logs} Test successful |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| GDP_CONFIG_DROPIN = ''' | ||
| genericDevicePlugin: | ||
| status: Enabled | ||
| devices: | ||
| - name: fakeserial | ||
| groups: | ||
| - paths: | ||
| - path: /dev/ttyPipeB0 | ||
| ''' | ||
|
|
||
| CONFIGMAP_PREAMBLE = ''' | ||
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: gdp-script | ||
| data: | ||
| entrypoint.sh: | | ||
| #!/usr/bin/env bash | ||
| # Fake user homedir for installing pip and pyserial. | ||
| HOME=/tmp python3 -m ensurepip --upgrade | ||
| HOME=/tmp python3 -m pip install pyserial | ||
| HOME=/tmp /script/fake-serial-communication.py pod | ||
|
|
||
| fake-serial-communication.py: | | ||
| ''' | ||
|
|
||
|
|
||
| def append_to_preamble(content: str) -> str: | ||
| # Add 4 spaces before each line | ||
| content = " " + content.replace("\n", "\n ") | ||
| return CONFIGMAP_PREAMBLE + content |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 you think we should use uname -r / pass one single kernel if there is more than one being listed ? if the system has more than one kernel i see this command is failing.
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 cannot use
uname -rbecause it returns the kernel of the host, bootc container can (and had when I tested it) contain different kernel.It's true that
rpm -q --qfwill return all matching pkgs, but the problem is that what you're running might not be the newest one.Because these commands are inside containerfile, not really reusable automatically (by importing a file) anywhere else, I would recommend that for QE testing you can copy and tweak them to your needs.
Uh oh!
There was an error while loading. Please reload this page.
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 is not clear for me. Containers are using kernel of the host, aren't they?
What would happen if the kernel in the container is different from the kernel on the host (majority of cases)?
Should we install both the kernel and the kernel-devel of the host?
Uh oh!
There was an error while loading. Please reload this page.
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.
Yes, containers are using kernel from the host and that's why there's a difference between
uname -mand version returned byrpm.There's already a kernel installed in the base bootc image, so I decided to install matching kernel-devel (rather to upgrade kernel and install kernel-devel). The kernel in the image is not active when building an image, but it will be when the image is installed onto the disk.
Personally, I wouldn't attempt to install the kernel of the host because there's no guarantee what someone has on their hypervisor (maybe it's older one). I feel like keeping the kernel might be easier if there's some kind of bug to report (I mean: image tag is pointing to kernel we're not changing).
Though maybe there's value in upgrading the kernel, so we can do it.
WDYT?
Uh oh!
There was an error while loading. Please reload this page.
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.
Let's think what users would do in this case. Wouldn't they likely try to sync the kernel version between the host and the container?
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.
What would they get by syncing the, for examples, CI's host kernel with the kernel of the image that will be installed on another host? At most, I would expect them to run
dnf upgradeto get latest packages (including kernel).I feel like this discussion is out of scope for this PR - current solution works and is completely valid: it installs kernel-devel for kernel already included in the bootc image, then compiles a driver for that particular kernel, there's no runtime involved, so there's no need to involve host's kernel.
Whether we should upgrade all packages or match kernel versions if more of a testing approach question and is, imho, applicable to all images we build (like, let's take rhel96-bootc-source image for example, question of upgrading the pkgs and/or kernel is just as valid).
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.
OK, we can continue this interesting discussion elsewhere.
I'll approve the PR for now.