Skip to content

Conversation

@ilyam8
Copy link
Member

@ilyam8 ilyam8 commented Nov 27, 2025

Summary

This PR fixes several bugs in the Docker entrypoint script and refactors group management logic for better maintainability.

  • Fixes
Bug Description
--apend typo usermod --apend silently failed, preventing users from being added to the Proxmox config files group
Group membership check grep -q "${DOCKER_USR}" matched substrings, causing false positives (e.g., user net incorrectly detected as member when netdata was in the group)
Telemetry disable logic Setting DISABLE_TELEMETRY=0 incorrectly disabled telemetry because -n "$VAR" is true for any non-empty string including "0"
Unconditional export PGID and DOCKER_HOST were exported even when empty
Missing default NETDATA_LISTENER_PORT had no fallback value (now defaults to 19999)
  • Refactoring
Change Description
is_user_in_group() New helper function for proper group membership checking
add_user_to_gid() New helper function to eliminate duplicate group creation/assignment logic
Variable naming Consistent group_gid instead of mixed group_guid/group_gid
Test Plan
Additional Information
For users: How does this change affect me?

Summary by cubic

Fixes multiple issues in the Docker entrypoint to make group assignment and telemetry control reliable, and sets a default Netdata listener port. Refactors group management into small helpers to reduce duplication and improve maintainability.

  • Bug Fixes

    • Corrected usermod --append and added precise GID-based group membership check (no substring matches).
    • Fixed telemetry opt-out to treat "0" as enabled; export PGID/DOCKER_HOST only when set.
    • Defaulted NETDATA_LISTENER_PORT to 19999; minor robustness tweaks (error redirection, container hostname write).
  • Refactors

    • Added is_user_in_group and add_user_to_gid helpers and reused them for Docker, Proxmox, and NVIDIA group setup.
    • Standardized variable names (group_gid) and simplified early returns for root/no-GID cases.

Written for commit 77344f9. Summary will update automatically on new commits.

@ilyam8 ilyam8 requested a review from a team as a code owner November 27, 2025 22:24
@ilyam8 ilyam8 requested review from Copilot and removed request for a team November 27, 2025 22:24
@github-actions github-actions bot added the area/packaging Packaging and operating systems support label Nov 27, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes critical bugs in the Docker entrypoint script and refactors group management logic for better maintainability and reliability.

Key Changes:

  • Fixes the --apend typo (now --append) that prevented users from being added to groups
  • Corrects group membership detection using exact matching instead of substring matching
  • Fixes telemetry disable logic to properly handle DISABLE_TELEMETRY=0
  • Introduces reusable helper functions is_user_in_group() and add_user_to_gid() to eliminate code duplication

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 1 file

@ilyam8 ilyam8 enabled auto-merge (squash) November 28, 2025 03:43
@ilyam8 ilyam8 merged commit 3a3fdab into netdata:master Nov 28, 2025
116 checks passed
@ilyam8 ilyam8 deleted the fix-refactor-docker-run branch November 28, 2025 09:27
@stelfrag stelfrag mentioned this pull request Dec 1, 2025
stelfrag pushed a commit to stelfrag/netdata that referenced this pull request Dec 1, 2025
Ferroin pushed a commit that referenced this pull request Dec 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/packaging Packaging and operating systems support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants