-
-
Notifications
You must be signed in to change notification settings - Fork 3
Feat/multi git sync #729
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
Open
Maleware
wants to merge
36
commits into
main
Choose a base branch
from
feat/multi-git-sync
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Feat/multi git sync #729
Changes from all commits
Commits
Show all changes
36 commits
Select commit
Hold shift + click to select a range
f939934
feat: Support objectOverrides
sbernauer d6b4c95
update to newer op-rs
sbernauer 49848cf
changelog
sbernauer fe99463
Adding support for multiple git-sync repos
Maleware 65e39a9
Merge branch 'main' into feat/multi-git-sync
Maleware 2cbae70
Cleaning up
Maleware 3ea363a
Merge branch 'feat/multi-git-sync' of github.com:stackabletech/airflo…
Maleware 26c11a7
makeing pre-commit happy
Maleware 7c7f25f
Adding check if cm is mounted into core folder, if omit mkdir
Maleware ffa249a
store wip
Maleware 984d7c1
working example, clean up
Maleware 5021b33
Merge branch 'main' into feat/multi-git-sync
Maleware 6538c67
update to v1alpha2
Maleware 0e0b09f
Add CHANGELOG.md
Maleware 04ec315
Adding docs draft
Maleware 3e2c438
Better documentation
Maleware 010bfd3
Refactor strings into constants
Maleware 50427cb
Further removing strings and consolidate in constant
Maleware a54f9e5
Improve comments and refactor
Maleware 8b2f4b6
Add two more comments
Maleware 61dc8ce
Another comment
Maleware 7e80e9a
removing add_env for dags-init container
Maleware d9558ea
consistent names across init containers
Maleware c5befaf
add only if git-sync is not empty in pythonpath
Maleware c2b6e44
Updating integration test mount-dags-gitsync to use two git-syncs
Maleware cf8b245
making pre-commit happy again
Maleware 1c188c5
again, pre-commit for dag-metrics.py
Maleware db8ecbd
rework typos
Maleware 8db36d6
better documentation for multi-git-sync
Maleware c7052a8
Merge branch 'main' into feat/multi-git-sync
Maleware 1b43f0d
again pre-commit hook was unhappy
Maleware bd8d7a4
Merge branch 'feat/multi-git-sync' of github.com:stackabletech/airflo…
Maleware ff8bb20
Add test print I forgot
Maleware e6a3e29
Updating 31-assert test step for airflow mount-dags-gitsync
Maleware 9d85772
Better comments according to review
Maleware 3982793
Update docs/modules/airflow/pages/usage-guide/mounting-dags.adoc
Maleware File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -71,6 +71,7 @@ pub const STACKABLE_LOG_DIR: &str = "/stackable/log"; | |
| pub const LOG_CONFIG_DIR: &str = "/stackable/app/log_config"; | ||
| pub const AIRFLOW_HOME: &str = "/stackable/airflow"; | ||
| pub const AIRFLOW_CONFIG_FILENAME: &str = "webserver_config.py"; | ||
| pub const AIRFLOW_DAGS_FOLDER: &str = "/stackable/app/allDAGs"; | ||
|
|
||
| pub const TEMPLATE_VOLUME_NAME: &str = "airflow-executor-pod-template"; | ||
| pub const TEMPLATE_LOCATION: &str = "/templates"; | ||
|
|
@@ -439,6 +440,42 @@ impl v1alpha2::AirflowCluster { | |
| fragment::validate(conf_rolegroup).context(FragmentValidationFailureSnafu) | ||
| } | ||
|
|
||
| // Softlink from each single folder git-{i} into {AIRFLOW_DAGS_FOLDER}/. | ||
| pub fn get_multi_gitsync_commands(&self) -> Vec<String> { | ||
| let mut symlinks = Vec::<String>::new(); | ||
| for (i, _) in self.spec.cluster_config.dags_git_sync.iter().enumerate() { | ||
| symlinks.push( | ||
| format!("ln -s /stackable/app/git-{i}/current {AIRFLOW_DAGS_FOLDER}/current-{i}") | ||
| .to_string(), | ||
| ) | ||
| } | ||
| symlinks | ||
| } | ||
|
|
||
| // kubernetesExecuter needs to copy into an empty volume since git-{0}/current | ||
| // is a softlink and thus can't be shared via a volume mount (volume remains empty). | ||
| pub fn get_kubernetes_executer_multi_gitsync_commands(&self) -> Vec<String> { | ||
| let mut cp_commands = Vec::<String>::new(); | ||
| for (i, _) in self.spec.cluster_config.dags_git_sync.iter().enumerate() { | ||
| cp_commands.push( | ||
| format!("cp -r /stackable/app/git-{i}/current/ {AIRFLOW_DAGS_FOLDER}/current-{i}") | ||
| .to_string(), | ||
| ); | ||
| } | ||
| // init-container seems to only accept one command line. | ||
| vec![cp_commands.join(" && ")] | ||
| } | ||
|
|
||
| // PYTHONPATH contains folder-name provided in CRD. | ||
| pub fn get_gitsync_absolute_paths(&self) -> Vec<String> { | ||
| let mut python_path = Vec::<String>::new(); | ||
| for (i, git_sync) in self.spec.cluster_config.dags_git_sync.iter().enumerate() { | ||
| let folder = &git_sync.git_folder.display(); | ||
| python_path.push(format!("{AIRFLOW_DAGS_FOLDER}/current-{i}/{folder}").to_string()) | ||
| } | ||
| python_path | ||
| } | ||
|
|
||
| /// Retrieve and merge resource configs for the executor template | ||
| pub fn merged_executor_config( | ||
| &self, | ||
|
|
@@ -600,10 +637,13 @@ impl AirflowRole { | |
| format!( | ||
| "cp -RL {CONFIG_PATH}/{AIRFLOW_CONFIG_FILENAME} {AIRFLOW_HOME}/{AIRFLOW_CONFIG_FILENAME}" | ||
| ), | ||
| // Adding cm as dags within the same AIRFLOW_DAGS_FOLDER may lead to problems, thus checking if exists. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does mean that we can't simply have, say, 2 DAGs in a single ConfigMap, which are mounted to |
||
| format!("mkdir -p {AIRFLOW_DAGS_FOLDER}"), | ||
| // graceful shutdown part | ||
| COMMON_BASH_TRAP_FUNCTIONS.to_string(), | ||
| remove_vector_shutdown_file_command(STACKABLE_LOG_DIR), | ||
| ]; | ||
| command.extend(airflow.get_multi_gitsync_commands()); | ||
|
|
||
| if resolved_product_image.product_version.starts_with("3.") { | ||
| // Start-up commands have changed in 3.x. | ||
|
|
@@ -663,6 +703,8 @@ impl AirflowRole { | |
| container_debug_command(), | ||
| "airflow triggerer &".to_string(), | ||
| ]), | ||
| // KubernetesExecutor intentionally not covered, it requires command | ||
| // generated by airflow and written into a pod template. | ||
| AirflowRole::Worker => command.extend(vec![ | ||
| "prepare_signal_handlers".to_string(), | ||
| container_debug_command(), | ||
|
|
@@ -715,6 +757,8 @@ impl AirflowRole { | |
| container_debug_command(), | ||
| "airflow triggerer &".to_string(), | ||
| ]), | ||
| // KubernetesExecutor intentionally not covered, it requires command | ||
| // generated by airflow and written into a pod template. | ||
| AirflowRole::Worker => command.extend(vec![ | ||
| "prepare_signal_handlers".to_string(), | ||
| container_debug_command(), | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the user needs to know this as it is (or should be?) an internal detail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tend to say yes, but there would still be the corner case if you want to deploy / run files from airflow you need to know where this is mounted. Then it becomes valuable information I believe.