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

Use readOnlyRootFilesystem securityContext in data plane pods #96

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

ramperher
Copy link
Collaborator

@ramperher ramperher commented Oct 25, 2024

  • This intends to fix access-control-security-context-read-only-file-system certsuite test for data plane pods, since this has to fail in controller-manager pods because they require it for reconciliation loop.
  • Apart from including readOnlyRootFilesystem securityContext field, it's also needed to define volumes for the directories where there may be modified or newly created files during execution.

Tests:

@ramperher ramperher force-pushed the security-context-read-only branch from 8d6cd7a to 7d2e05c Compare October 25, 2024 15:57
@dcibot
Copy link

dcibot commented Oct 25, 2024

@ramperher ramperher force-pushed the security-context-read-only branch 2 times, most recently from 9989411 to b9b31ee Compare October 25, 2024 17:03
@dcibot
Copy link

dcibot commented Oct 25, 2024

@dcibot
Copy link

dcibot commented Oct 25, 2024

@ramperher
Copy link
Collaborator Author

from change #96:

Up to now, testpmd is working but trex is failing. By checking the logs, I've seen the same issue I saw in testpmd logs, complaining about /var/run/dpdk folder:

+ ./_t-rex-64 --cfg /usr/local/bin/example-cnf/trex_cfg.yaml -i --no-hw-flow-stat -c 4                              
Starting  TRex v2.85 please wait  ...
 You might need to run ./trex-cfg  once
EAL: Error creating '/var/run/dpdk': Read-only file system                                                          
EAL: Cannot create runtime directory
EAL: FATAL: Invalid 'command line' arguments.
EAL: Invalid 'command line' arguments.
EAL: Error - exiting with code: 1

So I'll add a volume also in that folder.

@ramperher ramperher force-pushed the security-context-read-only branch from b9b31ee to 4ee46b6 Compare October 28, 2024 14:58
@dcibot
Copy link

dcibot commented Oct 28, 2024

@ramperher ramperher force-pushed the security-context-read-only branch from 4ee46b6 to 9180b71 Compare October 28, 2024 19:14
@dcibot
Copy link

dcibot commented Oct 28, 2024

@ramperher
Copy link
Collaborator Author

Testing is finished, now the data plane pods are passing this test related to readOnlyRootFilesystem feature, while testpmd is behaving correctly and the result in terms of performance is the same.

I've created this change in dallas-pipelines to reflect this update: https://github.com/dci-labs/dallas-pipelines/pull/1259

Copy link
Collaborator

@manurodriguez manurodriguez left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -31,7 +31,7 @@ port_obj_l2_template = {
"src_mac": ""
}

CFG_FILE = "/etc/trex_cfg.yaml"
CFG_FILE = "/usr/local/bin/example-cnf/trex_cfg.yaml"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here, just for my understanding, I'm curious why you decided to move the trex cfg file to /usr/local/bin/example-cnf/ ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because, if leaving it at /etc/, I would have needed to mount a volume on /etc folder to be able to create that file, and I didn't want to expose the whole /etc directory just for that

@ramperher ramperher merged commit 549300b into main Oct 29, 2024
1 check passed
@tonyskapunk tonyskapunk deleted the security-context-read-only branch November 20, 2024 13:48
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.

3 participants