bintocher commented on PR #62284:
URL: https://github.com/apache/airflow/pull/62284#issuecomment-3939570702

   Thanks for the review @jscheffl!
   
   The motivation comes from issue #61074: when using the Kubernetes executor 
in multi-namespace mode with git-sync, the Helm chart configures the same 
git-sync init container for all components. But a worker only needs a specific 
submodule, not the full repo. With `executor_config`'s `pod_override`, users 
should be able to override the existing init container by name (e.g. change 
just the repo URL or args) rather than getting a duplicate.
   
   This aligns with how regular containers already work in 
`reconcile_containers` — containers are matched by position and merged (client 
values override base, lists like `env` and `volume_mounts` are extended). The 
new `reconcile_init_containers` does the same but matches by **name** instead 
of position, since init containers don't have a guaranteed positional 
correspondence.
   
   When names **don't** match, containers are still appended (same behavior as 
before). Name-based merge only kicks in when both base and client have an init 
container with the same name.
   
   Regarding volumes — good point about consistency. However, volumes don't 
have the same "override by identity" semantic since they are referenced by name 
from `volumeMounts`. Extending (appending) volumes is the correct behavior as 
it is today — you want all volumes available. The init container case is 
different because you want to *replace* the configuration of a specific named 
container, not add a duplicate.
   
   ---
   
   Regarding the hook approach: a hook would be a more flexible solution 
long-term, but it would be a much larger change involving new hook points, 
configuration, and documentation. This PR follows the existing pattern already 
established by `reconcile_containers` for regular containers, keeping the 
change minimal and consistent with the current architecture. A hook could be 
considered as a follow-up if more advanced customization is needed.
   
   I also pushed a fix for the CI failures — the original code was mutating 
`client_spec` directly instead of using `extend_object_field`'s deepcopy 
pattern, which caused `init_containers` to become `[]` instead of `None` when 
not present.


-- 
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]

Reply via email to