remove limit for number of containers in containers.conf #125
Conversation
WalkthroughTwo new tasks have been added to the Ansible playbook. The first task, "Unlimited number of pids for podman," utilizes the Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Playbook as Ansible Playbook
participant Lineinfile as lineinfile Module
participant Config as containers.conf
participant Systemd as systemd_service Module
User->>Playbook: Execute network_setup role
Playbook->>Lineinfile: Run "Unlimited number of pids for podman" task
Lineinfile->>Config: Search for line matching '^#pids_limit = .*'
Config-->>Lineinfile: Return line (if found)
Lineinfile->>Config: Replace with 'pids_limit = -1'
Config-->>Lineinfile: Confirm update
Lineinfile-->>Playbook: Task complete
Playbook->>Systemd: Run "Restart podman service" task
Systemd-->>Playbook: Restart service complete
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
openshift-ci
bot
added
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/playbooks/roles/network_setup/tasks/main.yaml (1)
15-20: Configuration Update: Remove PID Limit for Containerd
The new task effectively removes the PID limit by uncommenting and setting thepids_limitto 0 in/usr/share/containers/containers.conf. This addresses the crashing pods issue due to reaching the PID limit.However, please verify that:
- The regex
'^#pids_limit = .*'reliably matches the commented-out configuration across various environments.- In cases where the line might already be uncommented or set differently, the task’s idempotence behaves as expected.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/playbooks/roles/network_setup/tasks/main.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Security scan
rccrdpccl
force-pushed
the
unlimit-number-of-pids-per-container
branch
from
eb09d3a to
2708bec
Compare
openshift-ci
bot
added
size/S
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/playbooks/roles/network_setup/tasks/main.yaml (1)
21-25: Restart Podman Service to Apply New Configuration:
This task uses theansible.builtin.systemd_servicemodule to restart the podman service withdaemon_reloadenabled, which is a good practice to ensure that the configuration changes are applied immediately. However, please confirm that appropriate privilege escalation (e.g.,become: true) is in effect at the play or role level, as modifying system configuration files and restarting services typically require elevated permissions.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/playbooks/roles/network_setup/tasks/main.yaml(1 hunks)
🔇 Additional comments (1)
test/playbooks/roles/network_setup/tasks/main.yaml (1)
15-20: Unlimited PIDs Configuration in containers.conf:
The new task correctly updates the containers configuration by settingpids_limitto 0, which effectively removes the process ID limit as intended. Please verify that the provided regular expression (^#pids_limit = .*) matches the commented line format across all target deployments. This will help ensure that no valid configuration lines are inadvertently skipped.
Signed-off-by: Riccardo Piccoli <rpiccoli@redhat.com>
rccrdpccl
force-pushed
the
unlimit-number-of-pids-per-container
branch
from
2708bec to
f21c523
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
test/playbooks/roles/network_setup/tasks/main.yaml (2)
15-20: Review "Unlimited number of pids for podman" Task – Validate Regex RobustnessThis task correctly updates the configuration by ensuring that the commented-out PID limit line is replaced with
pids_limit = -1. Please double-check that the regular expression'^# pids_limit = .*'will match all possible variations (e.g. potential leading spaces or different comment styles) in the actualcontainers.conf. Adjusting the regex for optional whitespace (for example, by using something like'^\\s*#\\s*pids_limit\\s*=\\s*.*') might improve idempotence and robustness.
21-25: Review "Restart podman service" Task – Verify Module UsageThis task restarts the podman service and reloads the daemon, which is essential after modifying
containers.conf. Please confirm that using the moduleansible.builtin.systemd_serviceis appropriate for your Ansible version and target environment; in many cases, the commonly used module isansible.builtin.systemd. If you encounter any issues withsystemd_service, consider switching toansible.builtin.systemd.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/playbooks/roles/network_setup/tasks/main.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Security scan
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: carbonin, rccrdpccl 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 |
openshift-merge-bot
bot
merged commit 1ca5747
into
openshift-assisted:master
This settings was never persisted in ansible, however we normally reach this number easily. The symptoms are pods crashing badly
Summary by CodeRabbit