Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add decription on how you can customize image entrypoint #18915

Merged
merged 7 commits into from
Oct 14, 2021
Merged

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Oct 12, 2021


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

@potiuk potiuk requested a review from kaxil as a code owner October 12, 2021 17:49
@boring-cyborg boring-cyborg bot added area:production-image Production image improvements and fixes kind:documentation labels Oct 12, 2021
@potiuk potiuk changed the title Update entrypoint.rst Add custom entrypoint documentation Oct 12, 2021
@potiuk potiuk closed this Oct 12, 2021
@potiuk potiuk reopened this Oct 12, 2021
@yan-hic
Copy link

yan-hic commented Oct 13, 2021

I still believe there is an use case for executing stuff BEFORE the entrypoint. In our case, we override the DB pointer dynamically, depending on some values in the ENV -> can only be set at runtime. Hence must run before the entrypoint as the latter (entrypoint_prod) performs a DB check, which we want (+ create user).

Our way around it: build an image with our my_entrypoint.sh as entrypoint, which will do all ENV settings, and then exec the base entrypoint passing on any argument.

@potiuk
Copy link
Member Author

potiuk commented Oct 14, 2021

Our way around it: build an image with our my_entrypoint.sh as entrypoint, which will do all ENV settings, and then exec the base entrypoint passing on any argument.

Good point. I added this option to the PR with documentation : #18915

There are a few things that you have to be aware of (read the docs) in this case:

  • never add any secrets this way (either via image variables, scripts, or arguments of image call)
  • make sure you use dumb-init in the entrypoint (otherwise your signal propagation will be broken)
ENTRYPOINT ["/usr/bin/dumb-init", "--", "/your_entrypoint.sh"]
  • always use exec /entrypoint "${@}" as last command in your custom entrypoint.

I explained that all in the PR #18915

@potiuk
Copy link
Member Author

potiuk commented Oct 14, 2021

I also added code explaining that "before" executing Airflow's entrypoint, certain functionality is not be available (updated the PR)

Also be aware that code executed before Airflow's entrypoint should not create any files or
directories inside the image and not everything might work the same way as after it is executed.
Before Airflow entrypoint is executed, the following functionalities are not available:

  • umask is not set properly to allow "group" write access
  • user is not yet created in /etc/passwd in case aribitrary user is used to run the image
  • the database and brokers might not be available yet

@yan-hic
Copy link

yan-hic commented Oct 14, 2021

@potiuk great ! Thanks so much for the detailed documentation. I am all set ! Btw, your PR pretty much closes #14346 in my mind.

@potiuk
Copy link
Member Author

potiuk commented Oct 14, 2021

Cool. Now I need just an approve from another committer :). Anyone?

@potiuk potiuk linked an issue Oct 14, 2021 that may be closed by this pull request
@potiuk potiuk requested a review from jedcunningham October 14, 2021 14:25
docs/docker-stack/entrypoint.rst Outdated Show resolved Hide resolved
docs/docker-stack/entrypoint.rst Outdated Show resolved Hide resolved
@potiuk potiuk changed the title Add custom entrypoint documentation Add decription on how you can customize image entrypoint Oct 14, 2021
Copy link
Member

@jedcunningham jedcunningham left a comment

Choose a reason for hiding this comment

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

Overall looks good, it makes sense as it is written now 👍

docs/docker-stack/entrypoint.rst Outdated Show resolved Hide resolved
docs/docker-stack/entrypoint.rst Outdated Show resolved Hide resolved
docs/docker-stack/entrypoint.rst Outdated Show resolved Hide resolved
docs/docker-stack/entrypoint.rst Outdated Show resolved Hide resolved
docs/docker-stack/entrypoint.rst Outdated Show resolved Hide resolved
docs/docker-stack/entrypoint.rst Outdated Show resolved Hide resolved
@github-actions github-actions bot added the okay to merge It's ok to merge this PR as it does not require more tests label Oct 14, 2021
@github-actions
Copy link

The PR is likely ready to be merged. No tests are needed as no important environment files, nor python files were modified by it. However, committers might decide that full test matrix is needed and add the 'full tests needed' label. Then you should rebase it to the latest main or amend the last commit of the PR, and push it with --force-with-lease.

1 similar comment
@github-actions
Copy link

The PR is likely ready to be merged. No tests are needed as no important environment files, nor python files were modified by it. However, committers might decide that full test matrix is needed and add the 'full tests needed' label. Then you should rebase it to the latest main or amend the last commit of the PR, and push it with --force-with-lease.

@potiuk potiuk merged commit cc627b3 into main Oct 14, 2021
@ashb ashb deleted the potiuk-patch-1 branch November 22, 2021 10:10
potiuk added a commit that referenced this pull request Nov 23, 2021
jedcunningham pushed a commit that referenced this pull request Dec 7, 2021
@jedcunningham jedcunningham added the type:doc-only Changelog: Doc Only label Dec 8, 2021
potiuk added a commit that referenced this pull request Jan 22, 2022
@potiuk potiuk modified the milestones: Airflow 2.2.3, Airflow 2.2.4 Jan 22, 2022
@jedcunningham jedcunningham added changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) and removed type:doc-only Changelog: Doc Only labels Feb 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:production-image Production image improvements and fixes changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) kind:documentation okay to merge It's ok to merge this PR as it does not require more tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

init scripts for dockerfile
6 participants