[PR] Stage the Git bundle working clone so an interrupted clone cannot poison it [airflow]

["hkc-8010 (via GitHub)" <gi...@apache.org>]

hkc-8010 opened a new pull request, #72923:
URL: https://github.com/apache/airflow/pull/72923

   `GitDagBundle` clones the working repository straight into its final `repo_path`. A process
   killed part way through that clone leaves a directory that git can open but that cannot check
   out the tracking ref, and because `_clone_repo_if_required` only clones when `repo_path` does
   not exist, every later initialization reuses it and fails. On a long-lived worker, where the
   bundle directory is pod-local and lives as long as the pod, that turns a single interrupted
   clone into a permanently broken worker.
   
   Seen in production on a worker whose clone was killed by SIGTERM 315 ms in, after a task was
   marked failed from the UI. Every one of the 30 tasks that landed on that pod afterwards failed
   at parse time with:
   
   ```
   git checkout master
   error: The following untracked working tree files would be overwritten by checkout:
           .agents/instructions/airflow-patterns.md
           <...>
   Aborting
   ```
   
   ## Change
   
   Clone into a staging directory next to `repo_path` and `os.replace` it into place once the
   clone (and any sparse-checkout setup) is complete. `rename` is atomic, so `repo_path` either
   does not exist or holds a finished clone. An interrupted clone now leaves only an orphaned
   staging directory, which the next attempt removes before it starts.
   
   This is deliberately the prevention half only. Recovering an already-broken `repo_path` is
   #72787, which moves the checkout inside `_clone_repo_if_required`'s retry and `rmtree` block,
   and I would rather that landed than have a competing PR for it. The two compose: with both,
   a directory cannot get poisoned in the first place, and one poisoned by an older provider
   version still gets discarded and re-cloned. #72759 also names a residual case that recovery
   alone does not reach, where the clone dies before the remote configuration is written and
   `self.repo.remotes.origin` raises `AttributeError`; staging removes that one too, because
   nothing lands at `repo_path` until the clone is finished.
   
   Note for whoever merges second: #72787 and this PR both touch the clone block, so one will
   need a trivial rebase. There is no semantic conflict.
   
   ## Tests
   
   - `test_interrupted_clone_leaves_no_working_repo`: patches `Repo.clone_from` to write a `.git`
     skeleton into its target and then raise, which is the state a real `git clone` leaves when
     it is killed before it writes refs or the working tree. Asserts `repo_path` does not exist
     afterwards and that the next `initialize()` produces a working bundle. On main the first
     assertion fails (`assert not True`) because the half-written directory is left at
     `repo_path`.
   - `test_stale_staging_directory_is_discarded`: a staging directory left by a clone that had no
     chance to clean up does not block the next one.
   - Two existing tests that mock `Repo` wholesale needed their `clone_from` to create its target
     directory, which is what a real clone does. Factored that into
     `_clone_from_creating_target`.
   
   Validation:
   
   - `providers/git/tests/unit/git/bundles/test_git.py` - 107 passed locally (105 before this PR).
   - `breeze testing providers-tests --backend sqlite --python 3.10 --db-reset` on the same file -
     107 passed.
   - Both new tests confirmed failing with the source file reverted to `upstream/main`.
   - `prek` on both changed files and `mypy` on `git.py` are clean.
   
   No newsfragment: provider bugfix, no user-facing API change.
   
   closes: #72922
   
   ---
   
   ##### Was generative AI tooling used to co-author this PR?
   
   - [X] Yes (please specify the tool below)
   
   Generated-by: Claude Code following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
   


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Stage the Git bundle working clone so an interrupted clone cannot poison it [airflow]

["hkc-8010 (via GitHub)" <gi...@apache.org>]

hkc-8010 commented on PR #72923:
URL: https://github.com/apache/airflow/pull/72923#issuecomment-5631144457

   The one red check, `Special tests / Latest Boto test: providers`, is not from this PR.
   
   The only failure in that job is:
   
   ```
   FAILED providers/amazon/tests/unit/amazon/aws/waiters/test_base_waiter.py::TestBaseBotoWaiter::test_waiter_deferrable
     - ModuleNotFoundError: No module named 'aiobotocore'
   ```
   
   That test was added to main yesterday in #72819, and it patches `aiobotocore.waiter.create_waiter_with_client`
   with no `importorskip` guard. The `--upgrade-boto` job removes `aiobotocore==3.9.0` when it upgrades
   boto3 and botocore to latest, so the import is missing in that environment only. CI tests the branch
   merged with main, so any PR opened after #72819 landed picks this up.
   
   This PR only touches `providers/git`, and the git tests in that same job passed. Everything else is
   green so far.
   


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org