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

[traffic-gen-in-vm] Place TRex config file and scripts #150

Merged
merged 3 commits into from
Jul 20, 2023

Conversation

orelmisan
Copy link
Member

@orelmisan orelmisan commented Jul 19, 2023

Following PR #149, the traffic-gen ConfigMap contains the TRex configuration file and several Python scripts.
When mounting the disk containing the ConfigMap, the files are read only.

  1. Copy the TRex config file to /etc in the traffic-gen guest.
  2. Create a directory to hold the Python scripts /opt/tests.
  3. Copy the Python scripts to the above directory.

AFAIU, there is no need to make the Python scripts executable, because they are not executed as standalone scripts,
and do not have a Shebang [1].

[1] https://en.wikipedia.org/wiki/Shebang_(Unix)

When mounting the disk containing the ConfigMap, the files are read only.
Copy the TRex configuration file to `/etc` using cloud-init.

Signed-off-by: Orel Misan <[email protected]>
When mounting the disk containing the ConfigMap, the files are read only.
Create a directory to hold the Python scripts that will be used by TRex.

Signed-off-by: Orel Misan <[email protected]>
Copy the Python scripts from the disk containing the ConfigMap
data to `/opt/tests`.

Signed-off-by: Orel Misan <[email protected]>
@orelmisan orelmisan requested a review from RamLavi July 19, 2023 13:50
@orelmisan
Copy link
Member Author

Tested against an OpenShift Virtualization 4.14 cluster.

@@ -143,10 +143,12 @@ func CloudInit(username, password string, bootCommands []string) string {

func trafficGenBootCommands(configDiskSerial string) []string {
const configMountDirectory = "/mnt/app-config"
const testScriptsDirectory = "/opt/tests"
Copy link
Collaborator

@RamLavi RamLavi Jul 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this const is going to be used also in the executor (see main branch reference here)
Do we want to make it a const on the trex package perhaps?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good idea.
The problem is, that currently there is no file with a proper name in the trex package to hold this const,
and I find creating a whole file just for this little const problematic.

I suggest to move it in the PR that will introduce the trex client logic.
WDYT?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree.

Copy link
Collaborator

@RamLavi RamLavi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

@orelmisan orelmisan merged commit 8351bd6 into kiagnose:traffic-gen-in-vm Jul 20, 2023
4 checks passed
@orelmisan orelmisan deleted the trex_cloud_init branch July 20, 2023 07:36
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.

2 participants