Skip to content

Conversation

@alpetric
Copy link
Collaborator

No description provided.

@claude
Copy link
Contributor

claude bot commented Dec 19, 2025

Claude finished @alpetric's task in 56s —— View job


Review Summary

  • Gather context and review changed files
  • Analyze code quality and best practices
  • Check for bugs, security, and performance issues
  • Provide review feedback

LGTM

This PR improves the error message for unshare failures by adding two helpful diagnostic hints:

  1. New check for max_user_namespaces - This is excellent because some AWS AMIs like Bottlerocket set this to 0, which silently disables user namespaces. Users encountering this issue would otherwise have no idea why unshare fails.

  2. Helm chart option reference - Adding disableUnsharePid=true as the Helm equivalent to ENABLE_UNSHARE_PID=false makes the documentation more complete for Kubernetes users who configure via Helm values.

Considerations

The changes are purely additive to a panic message and don't affect runtime behavior. The formatting is consistent with existing bullet points, and the parenthetical note explaining Bottlerocket provides good context.

Testing Instructions

To verify this change, you would need to trigger an unshare failure scenario. This is typically done by:

  1. Deploying a Windmill worker in an environment where user namespaces are disabled (e.g., an EKS cluster with Bottlerocket AMI, or a container without privileged mode)
  2. Setting ENABLE_UNSHARE_PID=true in the worker environment
  3. Starting the worker and observing the panic message, which should now include the new hints about checking max_user_namespaces and the Helm chart option

Alternatively, in a development environment, you can temporarily modify the unshare test to fail and verify the error message contains the new content.

@cloudflare-workers-and-pages
Copy link

Deploying windmill with  Cloudflare Pages  Cloudflare Pages

Latest commit: 99b9a76
Status: ✅  Deploy successful!
Preview URL: https://5c710456.windmill.pages.dev
Branch Preview URL: https://alp-unshare-errmsg.windmill.pages.dev

View logs

@rubenfiszel rubenfiszel merged commit cdd5d9f into main Dec 19, 2025
12 checks passed
@rubenfiszel rubenfiszel deleted the alp/unshare_errmsg branch December 19, 2025 23:14
@github-actions github-actions bot locked and limited conversation to collaborators Dec 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants