feat: add hardware server info - #79
Conversation
WalkthroughAdds hardware detection and reporting: the shell playbook now detects Changes
Sequence Diagram(s)sequenceDiagram
participant Playbook as server-info.sh
participant Target as Remote Server
participant PHP as ServersTrait
participant User as CLI/UI
rect `#E8F5E9`
Playbook->>Target: detect_cpu_cores()
Playbook->>Target: detect_ram_mb()
Playbook->>Target: detect_disk_type()
Target-->>Playbook: { cpu_cores, ram_mb, disk_type }
end
rect `#E3F2FD`
Playbook->>Playbook: assemble YAML payload (includes hardware)
Playbook-->>PHP: send YAML payload
end
rect `#FFF3E0`
PHP->>PHP: parse payload
alt hardware present
PHP->>PHP: render "Hardware" block
end
PHP-->>User: display server info (with Hardware)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![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)
playbooks/server-info.sh (1)
182-202: Disk type detection works well for common scenarios.The implementation correctly detects SSD vs HDD using TRIM support (discard granularity) as the primary method and rotation flag as fallback. This approach works reliably for cloud VMs and physical servers with standard disk naming (sda/vda).
Optional: Consider adding NVMe detection.
The current regex
^[sv]dadoesn't match NVMe drives (nvme0n1). While NVMe is less common in typical deployment scenarios, you could extend support:- disc_gran=$(lsblk -d -o name,disc-gran 2> /dev/null | grep -E "^[sv]da" | head -n1 | awk '{print $2}') + disc_gran=$(lsblk -d -o name,disc-gran 2> /dev/null | grep -E "^([sv]da|nvme)" | head -n1 | awk '{print $2}')And similarly for the rotation check on line 196.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/Traits/ServersTrait.php(1 hunks)playbooks/server-info.sh(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/00-main.mdc)
**/*.php: Eliminate single-use methods: inline if a method is called only once
Cache computed values: initialize expensive calculations in the constructor
Avoid method call overhead: prefer direct property access when appropriate
Files:
app/Traits/ServersTrait.php
🔇 Additional comments (6)
app/Traits/ServersTrait.php (1)
147-178: LGTM!The hardware display logic is well-structured and defensive. It correctly handles mixed types for hardware values, provides clear formatting (GB vs MB, singular vs plural cores), and only displays the section when data is available.
playbooks/server-info.sh (5)
146-157: LGTM!The tool installation logic correctly adds
util-linux(which provideslsblk) to the package list for hardware detection. The check on line 147 now verifies bothssandlsblkare available before skipping installation.
168-170: LGTM!The CPU core detection uses
nprocwith a sensible fallback to "1".
175-177: LGTM!The RAM detection correctly uses
free -mto get total memory in MB with a reasonable fallback.
377-377: LGTM!The hardware detection is properly integrated into the main execution flow with appropriate variable declarations and user feedback messages.
Also applies to: 389-393
425-428: LGTM!The hardware section is correctly formatted as valid YAML with proper indentation and uses the variables populated by the detection functions.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
playbooks/server-info.sh (1)
146-159: Ensure tools are available before they're used in hardware detection.Hardware detection functions
detect_cpu_cores()anddetect_disk_type()(lines 389–392) invokenprocandlsblkrespectively, butensure_tools()is called afterward at line 395. If these tools are not already present, the detection will silently fall back to defaults ("1" for cores, "hdd" for disk type) before the installation attempt.While most modern systems have these tools pre-installed (triggering the early return at line 147), the logic ordering is backwards. Reorder
ensure_tools()to be called immediately after permission checks and before hardware detection.echo "✓ Checking permissions..." permissions=$(check_permissions) + echo "✓ Cataloging services..." + ensure_tools "$family" "$permissions" + echo "✓ Detecting hardware..." cpu_cores=$(detect_cpu_cores) ram_mb=$(detect_ram_mb) disk_type=$(detect_disk_type) - echo "✓ Cataloging services..." - ensure_tools "$family" "$permissions" echo "✓ Checking Caddy status..."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
playbooks/server-info.sh(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
playbooks/server-info.sh (2)
playbooks/demo-site.sh (1)
run_cmd(37-43)playbooks/server-install.sh (1)
run_cmd(49-55)
🔇 Additional comments (2)
playbooks/server-info.sh (2)
168-202: Conservative hardware detection with safe fallbacks.The three new detection functions handle missing tools gracefully:
detect_cpu_cores()falls back to "1"detect_ram_mb()falls back to "512"detect_disk_type()uses two methods (disc-gran → rotation flag → "hdd" fallback)The pattern matching in
detect_disk_type()(lines 187, 196) targets common primary disk names (^([sv]da|nvme)), which covers standard Linux scenarios. Secondary devices or unusual setups would gracefully default to "hdd".
425-428: Verify YAML hardware section is rendered correctly by ServersTrait.The new
hardwaresection follows proper YAML nesting and includes three fields. Ensure the PHP ServersTrait (mentioned in the AI summary but not provided) correctly parses and displays this structure, particularly handling the case where values may be placeholder defaults from fallbacks.If you have the PHP ServersTrait available, verify it correctly renders:
cpu_coresas an integer (with proper singular/plural handling if displaying as text)ram_mbas an integer or converted to GB when appropriatedisk_typeas uppercase ("SSD" or "HDD")
Reorder ensure_tools() execution before detect_disk_type() to prevent silent fallback to incorrect 'hdd' default when lsblk is not available.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
playbooks/server-info.sh (2)
1-16: Update file header documentation to include hardware detection.The docstring describes the script's outputs but omits hardware metrics. Update line 5 and the "Returns YAML with" section (lines 10-16) to reflect the new
hardwareblock withcpu_cores,ram_mb, anddisk_type.Apply this diff:
# Gather Server Information # ---- -# This playbook detects distribution, family, permissions, listening services, Caddy metrics, and PHP-FPM metrics. +# This playbook detects distribution, family, permissions, hardware info, listening services, Caddy metrics, and PHP-FPM metrics. # # Required Environment Variables: # DEPLOYER_OUTPUT_FILE - Output file path (provided automatically) # # Returns YAML with: # - distro: ubuntu|debian|fedora|centos|rocky|alma|rhel|amazon|unknown # - family: debian|fedora|redhat|amazon|unknown # - permissions: root|sudo|none +# - hardware: cpu_cores, ram_mb, disk_type # - caddy: Caddy metrics (available, version, sites_count, domains, uptime_seconds, active_requests, total_requests, memory_mb) # - php_fpm: PHP-FPM metrics (available, pool, process_manager, uptime_seconds, accepted_conn, listen_queue, idle_processes, active_processes, total_processes, max_children_reached, slow_requests) # - ports: map of port numbers to process names
182-202: Disk detection regex pattern may miss some disk types.The grep pattern
^([sv]da|nvme)on lines 187 and 196 matches only sda, vda, and nvme—it would skip sdb, sdc, vdb, vdc, etc. While this is unlikely in practice (first disk is typically sda or nvme0n1), the function would default to "hdd" in those cases.Consider making the pattern more permissive to cover all virtio and SATA disks.
Apply this diff to match all disk types more reliably:
- disc_gran=$(lsblk -d -o name,disc-gran 2> /dev/null | grep -E "^([sv]da|nvme)" | head -n1 | awk '{print $2}') + disc_gran=$(lsblk -d -o name,disc-gran 2> /dev/null | grep -E "^([sv]d|nvme)" | head -n1 | awk '{print $2}') # If disc-gran is non-zero (e.g., "512B"), it's an SSD if [[ -n $disc_gran && $disc_gran != "0B" ]]; then @@ -194,7 +194,7 @@ detect_disk_type() { fi # Fallback: Check rotation flag (works for physical disks) - rotation=$(lsblk -d -o name,rota 2> /dev/null | grep -E "^([sv]da|nvme)" | head -n1 | awk '{print $2}') + rotation=$(lsblk -d -o name,rota 2> /dev/null | grep -E "^([sv]d|nvme)" | head -n1 | awk '{print $2}') if [[ $rotation == "0" ]]; then echo "ssd" else
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
playbooks/server-info.sh(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
playbooks/server-info.sh (2)
playbooks/demo-site.sh (1)
run_cmd(37-43)playbooks/server-install.sh (1)
run_cmd(49-55)
🔇 Additional comments (3)
playbooks/server-info.sh (3)
142-159: Tool installation logic is sound.The
ensure_toolsfunction correctly checks for bothssandlsblkbefore attempting installation, installs the right packages for each family (iproute2/util-linux on Debian; iproute/util-linux on RedHat), and includes a yum→dnf fallback for newer RHEL/CentOS versions.
168-177: Hardware detection functions have appropriate fallbacks.
detect_cpu_cores()anddetect_ram_mb()both use standard tools (nproc,free -m) and include sensible defaults ("1" and "512" respectively), ensuring the script won't fail if these commands behave unexpectedly.
375-428: Hardware detection integration is clean and well-ordered.Variables are declared upfront (line 377), functions are called after
ensure_toolsto guarantee lsblk availability (lines 392–395), and YAML output structure is properly indented and consistent with the rest of the file. The hardware block will integrate seamlessly with the existing PHP rendering logic.
Add hardware detection to script description and YAML output documentation. The playbook now detects cpu_cores, ram_mb, and disk_type.
Change pattern from ^([sv]da|nvme) to ^([sv]d|nvme) to detect all SATA and virtio disks (sdb, sdc, vdb, vdc, etc.), not just the first disk.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
playbooks/server-info.sh (1)
169-203: Hardware detection logic is sound with appropriate fallbacks.The three-tier approach (nproc →
1, free -m →512, TRIM detection → rotation flag →hdd) provides sensible degradation. The TRIM-first detection is cloud-friendly.One minor note: variable naming uses
disc_granrather thandisk_gran. Consider aligning with the function name for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
playbooks/server-info.sh(6 hunks)
🔇 Additional comments (4)
playbooks/server-info.sh (4)
5-5: Excellent documentation updates.The header comments clearly document the new hardware detection capability alongside existing distro and service metrics.
Also applies to: 14-14
143-160: Tool installation logic is sound.The early-return optimization correctly checks for both required tools, and the package installation for each distribution family is accurate. The dnf fallback for RedHat-based distros is a good defensive choice.
378-378: Clean integration into main execution flow.Variable declarations are scoped appropriately, and hardware detection is called at the right point in the sequence (after tool validation, before service checks).
Also applies to: 393-396
426-429: YAML structure is properly formatted and integrated.The hardware block is correctly nested with consistent indentation and positioned logically between system metadata and service metrics. Structure aligns with expected downstream consumption by the PHP trait.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
playbooks/server-info.sh (1)
169-171: Minor: CPU core detection fallback is conservative.Using
nprocwith fallback to "1" is safe, but falling back to a single core on error could misrepresent a multi-core system. Consider whether "1" is the best default, or if logging/warning would be helpful. This is acceptable as-is but worth noting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
playbooks/server-info.sh(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
playbooks/server-info.sh (2)
playbooks/demo-site.sh (1)
run_cmd(37-43)playbooks/server-install.sh (1)
run_cmd(49-55)
🔇 Additional comments (6)
playbooks/server-info.sh (6)
5-5: Documentation updates are clear and accurate. Includes hardware detection in overview and output schema.Also applies to: 14-14
378-378: Hardware detection integration in main() is well-structured.Variables are properly scoped and detection functions are called in the correct sequence (after
ensure_tools()). The progress messages are helpful for debugging.Also applies to: 393-396
426-429: YAML hardware block structure is clean and well-positioned.Proper indentation and placement. Since all hardware detection functions have fallback defaults (e.g., "1" for cores, "512" for RAM, "hdd" for disk type), the values should always be populated.
176-178: I'll search for information about thefreecommand output format stability across util-linux versions, then check the codebase for any version or distro support constraints.Let me check the codebase to understand the context and purpose of this function:
Let me check the codebase context for this function:
Based on my verification, the
Mem:line format infree -moutput shows columns for total, used, free, shared, buff/cache, and available. The output format differs between RHEL 6 and RHEL 7/8, but key parameters remain consistent.The code
free -m 2> /dev/null | awk 'NR==2 {print $2}'extracts:
NR==2: The second line (the Mem: line)$2: The total RAM column (first numeric value after the "Mem:" label)This field position is stable across all documented util-linux versions. However, parsing the output of free is not totally reliable, and reading values from /proc/meminfo is an alternative with a format that is somewhat easier to parse and should be stable.
The code's fallback (
|| echo "512") provides basic resilience. The implementation works correctly across supported distros, though consider these minor caveats:
- Localization (LC_ALL) could theoretically affect output
- Very old systems might behave differently
- /proc/meminfo would be a more robust alternative
No critical compatibility issues found; the current implementation is functional.
147-160: Package names verified—code is correct.The verify confirms that the package selection is accurate across all distributions:
- Debian/Ubuntu correctly uses
iproute2(providesss) andutil-linux(provideslsblk)- Fedora/RHEL/CentOS correctly uses
iproute(providesss) andutil-linux(provideslsblk)The code's distro-specific package selection is appropriate. No changes needed.
183-203: Disk type detection scope is cloud-VM focused; confirm if environment coverage is complete.The function is explicitly designed for "virtualized environments (cloud VMs)" per the comment at line 186. In this scope, the regex
^([sv]d|nvme)is reasonably appropriate:
- Covered: sd* (cloud provider disks), vd* (QEMU/KVM virtio), nvme (instance storage)
- Gaps exist: xvd* (Xen/Citrix hypervisors), mmcblk* (embedded/ARM)
However, one clarification on the review comment: it mentions vd may be missed, but vd is matched by the
[sv]dpattern.Action: Confirm whether your deployment environment includes Xen-based or other hypervisors beyond cloud providers. If so, expand the regex to include xvd* and any other device prefixes in your target environments. Otherwise, the current scope is appropriate.
1 participant
Summary by CodeRabbit
New Features
Chores