Re: [PR] fix: allow production image to run with arbitrary UIDs on Podman [airflow]
potiuk commented on PR #62318: URL: https://github.com/apache/airflow/pull/62318#issuecomment-4178129949 This pull request has been converted to draft due to quality issues more than a week ago and there has been no response from the author. **@SakshamSinghal20**, you are welcome to reopen this PR when you are ready to continue working on it. Thank you for your contribution! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: allow production image to run with arbitrary UIDs on Podman [airflow]
potiuk closed pull request #62318: fix: allow production image to run with arbitrary UIDs on Podman URL: https://github.com/apache/airflow/pull/62318 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: allow production image to run with arbitrary UIDs on Podman [airflow]
potiuk commented on PR #62318: URL: https://github.com/apache/airflow/pull/62318#issuecomment-4035056419 @SakshamSinghal20 This PR has been converted to **draft** because it does not yet meet our [Pull Request quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria). **Issues found:** - :x: **Other failing CI checks**: Failing: Basic tests / React UI tests. Run `prek run --from-ref main` locally to reproduce. See [static checks docs](https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst). > **Note:** Your branch is **487 commits behind `main`**. Some check failures may be caused by changes in the base branch rather than by your PR. Please rebase your branch and push again to get up-to-date CI results. **What to do next:** - The comment informs you what you need to do. - Fix each issue, then mark the PR as "Ready for review" in the GitHub UI - but only after making sure that all the issues are fixed. - Maintainers will then proceed with a normal review. Converting a PR to draft is **not** a rejection — it is an invitation to bring the PR up to the project's standards so that maintainer review time is spent productively. If you have questions, feel free to ask on the [Airflow Slack](https://s.apache.org/airflow-slack). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: allow production image to run with arbitrary UIDs on Podman [airflow]
potiuk commented on PR #62318:
URL: https://github.com/apache/airflow/pull/62318#issuecomment-4033419161
> @potiuk! I've updated the implementation to address the feedback:
>
> ✅ Fixed the chgrp/chmod commands to retain both ${AIRFLOW_USER_HOME_DIR}
and ${AIRFLOW_HOME} ✅ Added set_pythonpath_for_arbitrary_user() function that
explicitly sets PYTHONPATH and PATH for arbitrary user execution ✅ Synced the
inline heredoc with the standalone entrypoint script The changes allow the
production image to run with arbitrary UIDs on Podman while maintaining Docker
compatibility.
No . the issues wer enot addressed. You are not detecting podman and the way
it was passed also you are - for some reson, not explained changing the chown
into chgrp.
Are you sure you know what you are doing ? Or are you blindly followig what
AI generates for you? Can you explain in short paragraph what your PR is doing
now?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] fix: allow production image to run with arbitrary UIDs on Podman [airflow]
SakshamSinghal20 commented on PR #62318:
URL: https://github.com/apache/airflow/pull/62318#issuecomment-4010971810
@potiuk! I've updated the implementation to address the feedback:
✅ Fixed the chgrp/chmod commands to retain both ${AIRFLOW_USER_HOME_DIR} and
${AIRFLOW_HOME}
✅ Added set_pythonpath_for_arbitrary_user() function that explicitly sets
PYTHONPATH and PATH for arbitrary user execution
✅ Synced the inline heredoc with the standalone entrypoint script
The changes allow the production image to run with arbitrary UIDs on Podman
while maintaining Docker compatibility.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] fix: allow production image to run with arbitrary UIDs on Podman [airflow]
SakshamSinghal20 commented on PR #62318: URL: https://github.com/apache/airflow/pull/62318#issuecomment-4010925545 Thank you @potiuk, @Ajay9704, and @MCollier24 for the detailed feedback. I understand the concerns raised and I'd like to address them: ## Key Issues Identified 1. **Root cause**: The issue stems from Podman's `keep-id` namespace mode automatically creating a user in `/etc/passwd` with the wrong HOME directory (`/opt/airflow` instead of `/home/airflow`). This prevents Python from finding packages in `$HOME/.local`. 2. **Current approach problems**: My changes modified the Dockerfile in ways that are Podman-specific and could break Docker compatibility. Specifically: - Changing group permissions and ownership patterns that work well for Docker users - Not detecting whether the container is running in Podman vs Docker ## Proposed Next Steps 1. **Fix static checks first**: Run `breeze run --all-files` to resolve the failing static checks as suggested by @Ajay9704. 2. **Implement Podman detection**: As @potiuk suggested, modify the `create_system_user_if_missing` function to: - Detect if running under Podman (possibly by checking for Podman-specific environment variables or filesystem indicators) - Run in an idempotent manner as @MCollier24 suggested - always verify and adjust HOME directory for the current user - Only apply Podman-specific fixes when Podman is detected, maintaining backward compatibility with Docker 3. **Keep the `g=u` change**: This seems like a beneficial change that doesn't break Docker. 4. **Add CI tests for Podman**: If we want Podman to be officially supported, we should add CI tests to prevent regressions. I'll work on a revised approach that: - Detects the container runtime environment - Fixes the HOME directory issue for Podman without modifying Docker behavior - Ensures the solution works for both rootless and non-rootless configurations Would this approach address your concerns? Any suggestions on the best way to detect Podman vs Docker at runtime? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: allow production image to run with arbitrary UIDs on Podman [airflow]
potiuk commented on PR #62318: URL: https://github.com/apache/airflow/pull/62318#issuecomment-3993694109 > To me it seems like the largest issue here is that the create_system_user_if_missing function in the entrypoint only does anything if the user doesn't exists in /etc/passwd which on Docker is fair as for this image the only user we'd be running as that could exist there is the default airflow. Could this function be modified to run an idempotent manner where it always attempts to adjust these values for the user? Yes. But also yoy have to make sure that that function continues to work in docker as well. Ideally if you want to change some behaviour beacause podman needs it, it should detect that you are in podman and act differently then -without modifying what happens in docker. Otherwise you are likely to make it works for podman but break for docker, which would be quite unfortunate, because almost everyone contributing uses docker. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: allow production image to run with arbitrary UIDs on Podman [airflow]
MCollier24 commented on PR #62318: URL: https://github.com/apache/airflow/pull/62318#issuecomment-3941576438 Hi all, I was the one who originally raised this bug report, I'm new to airflow so apologies if I'm asking stupid questions! To me it seems like the largest issue here is that the `create_system_user_if_missing` function in the entrypoint only does anything if the user doesn't exists in `/etc/passwd` which on Docker is fair as for this image the only user we'd be running as that could exist there is the default `airflow`. Could this function be modified to run an idempotent manner where it always attempts to adjust these values for the user? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: allow production image to run with arbitrary UIDs on Podman [airflow]
SakshamSinghal20 commented on PR #62318: URL: https://github.com/apache/airflow/pull/62318#issuecomment-3940626444 @jscheffl You can have a look now. I have fixed the branch issue. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: allow production image to run with arbitrary UIDs on Podman [airflow]
SakshamSinghal20 commented on PR #62318: URL: https://github.com/apache/airflow/pull/62318#issuecomment-3940602076 Yess I am fixing it there was some issue with my branch. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: allow production image to run with arbitrary UIDs on Podman [airflow]
jscheffl commented on PR #62318: URL: https://github.com/apache/airflow/pull/62318#issuecomment-3940597136 Some rebase fail it seems. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
