Skip to content

Pass workload to KE using command args instead of init container - #50448

Merged
amoghrajesh merged 32 commits into
apache:mainfrom
davidsharp7:remove_init_container_from_k8
May 20, 2025
Merged

Pass workload to KE using command args instead of init container#50448
amoghrajesh merged 32 commits into
apache:mainfrom
davidsharp7:remove_init_container_from_k8

Conversation

@davidsharp7

Copy link
Copy Markdown
Contributor

closes: #50025

Remove init container and start kubernetes executor via json string.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborgboring-cyborgBot added area:providers provider:cncf-kubernetes Kubernetes (k8s) provider related issues labels May 11, 2025
@jscheffl
jscheffl requested a review from amoghrajeshMay 11, 2025 12:53
@amoghrajesh

Copy link
Copy Markdown
Contributor

Thanks! I will take a look later today or tomorrow

@davidsharp7

Copy link
Copy Markdown
ContributorAuthor

I will look at the integration tests. Breeze is being problematic.

@potiuk

Copy link
Copy Markdown
Member

I will look at the integration tests. Breeze is being problematic.

What problems do you have with it? Would love to hear.

@davidsharp7

Copy link
Copy Markdown
ContributorAuthor

I will look at the integration tests. Breeze is being problematic.

What problems do you have with it? Would love to hear.

Corporate proxy 😀

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approach is ok @davidsharp7. Some comments

@davidsharp7
davidsharp7 marked this pull request as ready for review May 13, 2025 05:49
@eladkal

Copy link
Copy Markdown
Contributor

Tests fail

=========================== short test summary info ============================
FAILED tests/kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag - AssertionError: assert equals failed
'failed' 'success'
FAILED tests/kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag_task_mapping - AssertionError: assert equals failed
'failed' 'success'
FAILED tests/kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag_with_scheduler_failure - AssertionError: assert equals failed
'failed' 'success'
======= 3 failed, 47 passed, 3 skipped, 6 warnings in 287.36s (0:04:47) ========

@amoghrajesh
amoghrajesh self-requested a review May 14, 2025 07:06
@amoghrajeshamoghrajesh changed the title Remove init container from k8Pass workload to KE using command args instead of init containerMay 15, 2025

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, i like how it turned out to be so simple. @davidsharp7 if you can run some dags with these changes, it would give me confidence to approve it.

@davidsharp7

Copy link
Copy Markdown
ContributorAuthor

Nice, i like how it turned out to be so simple. @davidsharp7 if you can run some dags with these changes, it would give me confidence to approve it.

Are the various tests not enough? Would be happy to do it but my local Mac is spluttering.

@amoghrajesh

Copy link
Copy Markdown
Contributor

Nice, i like how it turned out to be so simple. @davidsharp7 if you can run some dags with these changes, it would give me confidence to approve it.

Are the various tests not enough? Would be happy to do it but my local Mac is spluttering.

Actually yeah, the CI is running fine with integration tests, so we are good.

@amoghrajesh

Copy link
Copy Markdown
Contributor

@davidsharp7 no further need to rebase, i will merge it once its green.

@amoghrajesh
amoghrajesh merged commit 967da37 into apache:mainMay 20, 2025
@boring-cyborg

Copy link
Copy Markdown

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

@amoghrajesh

Copy link
Copy Markdown
Contributor

@davidsharp7 good work on this one!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providersprovider:cncf-kubernetesKubernetes (k8s) provider related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Forced busybox init_container in kubernetes executor after upgrade to v3.

5 participants

@davidsharp7@amoghrajesh@potiuk@eladkal@jpmenil
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Pass workload to KE using command args instead of init container by davidsharp7 · Pull Request #50448 · apache/airflow · GitHub
Skip to content

Pass workload to KE using command args instead of init container - #50448

Merged
amoghrajesh merged 32 commits into
apache:mainfrom
davidsharp7:remove_init_container_from_k8
May 20, 2025
Merged

Pass workload to KE using command args instead of init container#50448
amoghrajesh merged 32 commits into
apache:mainfrom
davidsharp7:remove_init_container_from_k8

Conversation

@davidsharp7

Copy link
Copy Markdown
Contributor

closes: #50025

Remove init container and start kubernetes executor via json string.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborgboring-cyborgBot added area:providers provider:cncf-kubernetes Kubernetes (k8s) provider related issues labels May 11, 2025
@jscheffl
jscheffl requested a review from amoghrajeshMay 11, 2025 12:53
@amoghrajesh

Copy link
Copy Markdown
Contributor

Thanks! I will take a look later today or tomorrow

@davidsharp7

Copy link
Copy Markdown
ContributorAuthor

I will look at the integration tests. Breeze is being problematic.

@potiuk

Copy link
Copy Markdown
Member

I will look at the integration tests. Breeze is being problematic.

What problems do you have with it? Would love to hear.

@davidsharp7

Copy link
Copy Markdown
ContributorAuthor

I will look at the integration tests. Breeze is being problematic.

What problems do you have with it? Would love to hear.

Corporate proxy 😀

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approach is ok @davidsharp7. Some comments

@davidsharp7
davidsharp7 marked this pull request as ready for review May 13, 2025 05:49
@eladkal

Copy link
Copy Markdown
Contributor

Tests fail

=========================== short test summary info ============================
FAILED tests/kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag - AssertionError: assert equals failed
'failed' 'success'
FAILED tests/kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag_task_mapping - AssertionError: assert equals failed
'failed' 'success'
FAILED tests/kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag_with_scheduler_failure - AssertionError: assert equals failed
'failed' 'success'
======= 3 failed, 47 passed, 3 skipped, 6 warnings in 287.36s (0:04:47) ========

@amoghrajesh
amoghrajesh self-requested a review May 14, 2025 07:06
@amoghrajeshamoghrajesh changed the title Remove init container from k8Pass workload to KE using command args instead of init containerMay 15, 2025

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, i like how it turned out to be so simple. @davidsharp7 if you can run some dags with these changes, it would give me confidence to approve it.

@davidsharp7

Copy link
Copy Markdown
ContributorAuthor

Nice, i like how it turned out to be so simple. @davidsharp7 if you can run some dags with these changes, it would give me confidence to approve it.

Are the various tests not enough? Would be happy to do it but my local Mac is spluttering.

@amoghrajesh

Copy link
Copy Markdown
Contributor

Nice, i like how it turned out to be so simple. @davidsharp7 if you can run some dags with these changes, it would give me confidence to approve it.

Are the various tests not enough? Would be happy to do it but my local Mac is spluttering.

Actually yeah, the CI is running fine with integration tests, so we are good.

@amoghrajesh

Copy link
Copy Markdown
Contributor

@davidsharp7 no further need to rebase, i will merge it once its green.

@amoghrajesh
amoghrajesh merged commit 967da37 into apache:mainMay 20, 2025
@boring-cyborg

Copy link
Copy Markdown

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

@amoghrajesh

Copy link
Copy Markdown
Contributor

@davidsharp7 good work on this one!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providersprovider:cncf-kubernetesKubernetes (k8s) provider related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Forced busybox init_container in kubernetes executor after upgrade to v3.

5 participants

@davidsharp7@amoghrajesh@potiuk@eladkal@jpmenil
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Pass workload to KE using command args instead of init container by davidsharp7 · Pull Request #50448 · apache/airflow · GitHub
Skip to content

Pass workload to KE using command args instead of init container - #50448

Merged
amoghrajesh merged 32 commits into
apache:mainfrom
davidsharp7:remove_init_container_from_k8
May 20, 2025
Merged

Pass workload to KE using command args instead of init container#50448
amoghrajesh merged 32 commits into
apache:mainfrom
davidsharp7:remove_init_container_from_k8

Conversation

@davidsharp7

Copy link
Copy Markdown
Contributor

closes: #50025

Remove init container and start kubernetes executor via json string.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborgboring-cyborgBot added area:providers provider:cncf-kubernetes Kubernetes (k8s) provider related issues labels May 11, 2025
@jscheffl
jscheffl requested a review from amoghrajeshMay 11, 2025 12:53
@amoghrajesh

Copy link
Copy Markdown
Contributor

Thanks! I will take a look later today or tomorrow

@davidsharp7

Copy link
Copy Markdown
ContributorAuthor

I will look at the integration tests. Breeze is being problematic.

@potiuk

Copy link
Copy Markdown
Member

I will look at the integration tests. Breeze is being problematic.

What problems do you have with it? Would love to hear.

@davidsharp7

Copy link
Copy Markdown
ContributorAuthor

I will look at the integration tests. Breeze is being problematic.

What problems do you have with it? Would love to hear.

Corporate proxy 😀

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approach is ok @davidsharp7. Some comments

@davidsharp7
davidsharp7 marked this pull request as ready for review May 13, 2025 05:49
@eladkal

Copy link
Copy Markdown
Contributor

Tests fail

=========================== short test summary info ============================
FAILED tests/kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag - AssertionError: assert equals failed
'failed' 'success'
FAILED tests/kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag_task_mapping - AssertionError: assert equals failed
'failed' 'success'
FAILED tests/kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag_with_scheduler_failure - AssertionError: assert equals failed
'failed' 'success'
======= 3 failed, 47 passed, 3 skipped, 6 warnings in 287.36s (0:04:47) ========

@amoghrajesh
amoghrajesh self-requested a review May 14, 2025 07:06
@amoghrajeshamoghrajesh changed the title Remove init container from k8Pass workload to KE using command args instead of init containerMay 15, 2025

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, i like how it turned out to be so simple. @davidsharp7 if you can run some dags with these changes, it would give me confidence to approve it.

@davidsharp7

Copy link
Copy Markdown
ContributorAuthor

Nice, i like how it turned out to be so simple. @davidsharp7 if you can run some dags with these changes, it would give me confidence to approve it.

Are the various tests not enough? Would be happy to do it but my local Mac is spluttering.

@amoghrajesh

Copy link
Copy Markdown
Contributor

Nice, i like how it turned out to be so simple. @davidsharp7 if you can run some dags with these changes, it would give me confidence to approve it.

Are the various tests not enough? Would be happy to do it but my local Mac is spluttering.

Actually yeah, the CI is running fine with integration tests, so we are good.

@amoghrajesh

Copy link
Copy Markdown
Contributor

@davidsharp7 no further need to rebase, i will merge it once its green.

@amoghrajesh
amoghrajesh merged commit 967da37 into apache:mainMay 20, 2025
@boring-cyborg

Copy link
Copy Markdown

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

@amoghrajesh

Copy link
Copy Markdown
Contributor

@davidsharp7 good work on this one!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providersprovider:cncf-kubernetesKubernetes (k8s) provider related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Forced busybox init_container in kubernetes executor after upgrade to v3.

5 participants

@davidsharp7@amoghrajesh@potiuk@eladkal@jpmenil
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Pass workload to KE using command args instead of init container by davidsharp7 · Pull Request #50448 · apache/airflow · GitHub
Skip to content

Pass workload to KE using command args instead of init container - #50448

Merged
amoghrajesh merged 32 commits into
apache:mainfrom
davidsharp7:remove_init_container_from_k8
May 20, 2025
Merged

Pass workload to KE using command args instead of init container#50448
amoghrajesh merged 32 commits into
apache:mainfrom
davidsharp7:remove_init_container_from_k8

Conversation

@davidsharp7

Copy link
Copy Markdown
Contributor

closes: #50025

Remove init container and start kubernetes executor via json string.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborgboring-cyborgBot added area:providers provider:cncf-kubernetes Kubernetes (k8s) provider related issues labels May 11, 2025
@jscheffl
jscheffl requested a review from amoghrajeshMay 11, 2025 12:53
@amoghrajesh

Copy link
Copy Markdown
Contributor

Thanks! I will take a look later today or tomorrow

@davidsharp7

Copy link
Copy Markdown
ContributorAuthor

I will look at the integration tests. Breeze is being problematic.

@potiuk

Copy link
Copy Markdown
Member

I will look at the integration tests. Breeze is being problematic.

What problems do you have with it? Would love to hear.

@davidsharp7

Copy link
Copy Markdown
ContributorAuthor

I will look at the integration tests. Breeze is being problematic.

What problems do you have with it? Would love to hear.

Corporate proxy 😀

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approach is ok @davidsharp7. Some comments

@davidsharp7
davidsharp7 marked this pull request as ready for review May 13, 2025 05:49
@eladkal

Copy link
Copy Markdown
Contributor

Tests fail

=========================== short test summary info ============================
FAILED tests/kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag - AssertionError: assert equals failed
'failed' 'success'
FAILED tests/kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag_task_mapping - AssertionError: assert equals failed
'failed' 'success'
FAILED tests/kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag_with_scheduler_failure - AssertionError: assert equals failed
'failed' 'success'
======= 3 failed, 47 passed, 3 skipped, 6 warnings in 287.36s (0:04:47) ========

@amoghrajesh
amoghrajesh self-requested a review May 14, 2025 07:06
@amoghrajeshamoghrajesh changed the title Remove init container from k8Pass workload to KE using command args instead of init containerMay 15, 2025

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, i like how it turned out to be so simple. @davidsharp7 if you can run some dags with these changes, it would give me confidence to approve it.

@davidsharp7

Copy link
Copy Markdown
ContributorAuthor

Nice, i like how it turned out to be so simple. @davidsharp7 if you can run some dags with these changes, it would give me confidence to approve it.

Are the various tests not enough? Would be happy to do it but my local Mac is spluttering.

@amoghrajesh

Copy link
Copy Markdown
Contributor

Nice, i like how it turned out to be so simple. @davidsharp7 if you can run some dags with these changes, it would give me confidence to approve it.

Are the various tests not enough? Would be happy to do it but my local Mac is spluttering.

Actually yeah, the CI is running fine with integration tests, so we are good.

@amoghrajesh

Copy link
Copy Markdown
Contributor

@davidsharp7 no further need to rebase, i will merge it once its green.

@amoghrajesh
amoghrajesh merged commit 967da37 into apache:mainMay 20, 2025
@boring-cyborg

Copy link
Copy Markdown

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

@amoghrajesh

Copy link
Copy Markdown
Contributor

@davidsharp7 good work on this one!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providersprovider:cncf-kubernetesKubernetes (k8s) provider related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Forced busybox init_container in kubernetes executor after upgrade to v3.

5 participants

@davidsharp7@amoghrajesh@potiuk@eladkal@jpmenil
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' Pass workload to KE using command args instead of init container by davidsharp7 · Pull Request #50448 · apache/airflow · GitHub
Skip to content

Pass workload to KE using command args instead of init container - #50448

Merged
amoghrajesh merged 32 commits into
apache:mainfrom
davidsharp7:remove_init_container_from_k8
May 20, 2025
Merged

Pass workload to KE using command args instead of init container#50448
amoghrajesh merged 32 commits into
apache:mainfrom
davidsharp7:remove_init_container_from_k8

Conversation

@davidsharp7

Copy link
Copy Markdown
Contributor

closes: #50025

Remove init container and start kubernetes executor via json string.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborgboring-cyborgBot added area:providers provider:cncf-kubernetes Kubernetes (k8s) provider related issues labels May 11, 2025
@jscheffl
jscheffl requested a review from amoghrajeshMay 11, 2025 12:53
@amoghrajesh

Copy link
Copy Markdown
Contributor

Thanks! I will take a look later today or tomorrow

@davidsharp7

Copy link
Copy Markdown
ContributorAuthor

I will look at the integration tests. Breeze is being problematic.

@potiuk

Copy link
Copy Markdown
Member

I will look at the integration tests. Breeze is being problematic.

What problems do you have with it? Would love to hear.

@davidsharp7

Copy link
Copy Markdown
ContributorAuthor

I will look at the integration tests. Breeze is being problematic.

What problems do you have with it? Would love to hear.

Corporate proxy 😀

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approach is ok @davidsharp7. Some comments

@davidsharp7
davidsharp7 marked this pull request as ready for review May 13, 2025 05:49
@eladkal

Copy link
Copy Markdown
Contributor

Tests fail

=========================== short test summary info ============================
FAILED tests/kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag - AssertionError: assert equals failed
'failed' 'success'
FAILED tests/kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag_task_mapping - AssertionError: assert equals failed
'failed' 'success'
FAILED tests/kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag_with_scheduler_failure - AssertionError: assert equals failed
'failed' 'success'
======= 3 failed, 47 passed, 3 skipped, 6 warnings in 287.36s (0:04:47) ========

@amoghrajesh
amoghrajesh self-requested a review May 14, 2025 07:06
@amoghrajeshamoghrajesh changed the title Remove init container from k8Pass workload to KE using command args instead of init containerMay 15, 2025

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, i like how it turned out to be so simple. @davidsharp7 if you can run some dags with these changes, it would give me confidence to approve it.

@davidsharp7

Copy link
Copy Markdown
ContributorAuthor

Nice, i like how it turned out to be so simple. @davidsharp7 if you can run some dags with these changes, it would give me confidence to approve it.

Are the various tests not enough? Would be happy to do it but my local Mac is spluttering.

@amoghrajesh

Copy link
Copy Markdown
Contributor

Nice, i like how it turned out to be so simple. @davidsharp7 if you can run some dags with these changes, it would give me confidence to approve it.

Are the various tests not enough? Would be happy to do it but my local Mac is spluttering.

Actually yeah, the CI is running fine with integration tests, so we are good.

@amoghrajesh

Copy link
Copy Markdown
Contributor

@davidsharp7 no further need to rebase, i will merge it once its green.

@amoghrajesh
amoghrajesh merged commit 967da37 into apache:mainMay 20, 2025
@boring-cyborg

Copy link
Copy Markdown

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

@amoghrajesh

Copy link
Copy Markdown
Contributor

@davidsharp7 good work on this one!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providersprovider:cncf-kubernetesKubernetes (k8s) provider related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Forced busybox init_container in kubernetes executor after upgrade to v3.

5 participants

@davidsharp7@amoghrajesh@potiuk@eladkal@jpmenil
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Pass workload to KE using command args instead of init container by davidsharp7 · Pull Request #50448 · apache/airflow · GitHub
Skip to content

Pass workload to KE using command args instead of init container - #50448

Merged
amoghrajesh merged 32 commits into
apache:mainfrom
davidsharp7:remove_init_container_from_k8
May 20, 2025
Merged

Pass workload to KE using command args instead of init container#50448
amoghrajesh merged 32 commits into
apache:mainfrom
davidsharp7:remove_init_container_from_k8

Conversation

@davidsharp7

Copy link
Copy Markdown
Contributor

closes: #50025

Remove init container and start kubernetes executor via json string.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborgboring-cyborgBot added area:providers provider:cncf-kubernetes Kubernetes (k8s) provider related issues labels May 11, 2025
@jscheffl
jscheffl requested a review from amoghrajeshMay 11, 2025 12:53
@amoghrajesh

Copy link
Copy Markdown
Contributor

Thanks! I will take a look later today or tomorrow

@davidsharp7

Copy link
Copy Markdown
ContributorAuthor

I will look at the integration tests. Breeze is being problematic.

@potiuk

Copy link
Copy Markdown
Member

I will look at the integration tests. Breeze is being problematic.

What problems do you have with it? Would love to hear.

@davidsharp7

Copy link
Copy Markdown
ContributorAuthor

I will look at the integration tests. Breeze is being problematic.

What problems do you have with it? Would love to hear.

Corporate proxy 😀

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approach is ok @davidsharp7. Some comments

@davidsharp7
davidsharp7 marked this pull request as ready for review May 13, 2025 05:49
@eladkal

Copy link
Copy Markdown
Contributor

Tests fail

=========================== short test summary info ============================
FAILED tests/kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag - AssertionError: assert equals failed
'failed' 'success'
FAILED tests/kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag_task_mapping - AssertionError: assert equals failed
'failed' 'success'
FAILED tests/kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag_with_scheduler_failure - AssertionError: assert equals failed
'failed' 'success'
======= 3 failed, 47 passed, 3 skipped, 6 warnings in 287.36s (0:04:47) ========

@amoghrajesh
amoghrajesh self-requested a review May 14, 2025 07:06
@amoghrajeshamoghrajesh changed the title Remove init container from k8Pass workload to KE using command args instead of init containerMay 15, 2025

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, i like how it turned out to be so simple. @davidsharp7 if you can run some dags with these changes, it would give me confidence to approve it.

@davidsharp7

Copy link
Copy Markdown
ContributorAuthor

Nice, i like how it turned out to be so simple. @davidsharp7 if you can run some dags with these changes, it would give me confidence to approve it.

Are the various tests not enough? Would be happy to do it but my local Mac is spluttering.

@amoghrajesh

Copy link
Copy Markdown
Contributor

Nice, i like how it turned out to be so simple. @davidsharp7 if you can run some dags with these changes, it would give me confidence to approve it.

Are the various tests not enough? Would be happy to do it but my local Mac is spluttering.

Actually yeah, the CI is running fine with integration tests, so we are good.

@amoghrajesh

Copy link
Copy Markdown
Contributor

@davidsharp7 no further need to rebase, i will merge it once its green.

@amoghrajesh
amoghrajesh merged commit 967da37 into apache:mainMay 20, 2025
@boring-cyborg

Copy link
Copy Markdown

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

@amoghrajesh

Copy link
Copy Markdown
Contributor

@davidsharp7 good work on this one!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providersprovider:cncf-kubernetesKubernetes (k8s) provider related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Forced busybox init_container in kubernetes executor after upgrade to v3.

5 participants

@davidsharp7@amoghrajesh@potiuk@eladkal@jpmenil
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); Pass workload to KE using command args instead of init container by davidsharp7 · Pull Request #50448 · apache/airflow · GitHub
Skip to content

Pass workload to KE using command args instead of init container - #50448

Merged
amoghrajesh merged 32 commits into
apache:mainfrom
davidsharp7:remove_init_container_from_k8
May 20, 2025
Merged

Pass workload to KE using command args instead of init container#50448
amoghrajesh merged 32 commits into
apache:mainfrom
davidsharp7:remove_init_container_from_k8

Conversation

@davidsharp7

Copy link
Copy Markdown
Contributor

closes: #50025

Remove init container and start kubernetes executor via json string.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborgboring-cyborgBot added area:providers provider:cncf-kubernetes Kubernetes (k8s) provider related issues labels May 11, 2025
@jscheffl
jscheffl requested a review from amoghrajeshMay 11, 2025 12:53
@amoghrajesh

Copy link
Copy Markdown
Contributor

Thanks! I will take a look later today or tomorrow

@davidsharp7

Copy link
Copy Markdown
ContributorAuthor

I will look at the integration tests. Breeze is being problematic.

@potiuk

Copy link
Copy Markdown
Member

I will look at the integration tests. Breeze is being problematic.

What problems do you have with it? Would love to hear.

@davidsharp7

Copy link
Copy Markdown
ContributorAuthor

I will look at the integration tests. Breeze is being problematic.

What problems do you have with it? Would love to hear.

Corporate proxy 😀

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approach is ok @davidsharp7. Some comments

@davidsharp7
davidsharp7 marked this pull request as ready for review May 13, 2025 05:49
@eladkal

Copy link
Copy Markdown
Contributor

Tests fail

=========================== short test summary info ============================
FAILED tests/kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag - AssertionError: assert equals failed
'failed' 'success'
FAILED tests/kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag_task_mapping - AssertionError: assert equals failed
'failed' 'success'
FAILED tests/kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag_with_scheduler_failure - AssertionError: assert equals failed
'failed' 'success'
======= 3 failed, 47 passed, 3 skipped, 6 warnings in 287.36s (0:04:47) ========

@amoghrajesh
amoghrajesh self-requested a review May 14, 2025 07:06
@amoghrajeshamoghrajesh changed the title Remove init container from k8Pass workload to KE using command args instead of init containerMay 15, 2025

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, i like how it turned out to be so simple. @davidsharp7 if you can run some dags with these changes, it would give me confidence to approve it.

@davidsharp7

Copy link
Copy Markdown
ContributorAuthor

Nice, i like how it turned out to be so simple. @davidsharp7 if you can run some dags with these changes, it would give me confidence to approve it.

Are the various tests not enough? Would be happy to do it but my local Mac is spluttering.

@amoghrajesh

Copy link
Copy Markdown
Contributor

Nice, i like how it turned out to be so simple. @davidsharp7 if you can run some dags with these changes, it would give me confidence to approve it.

Are the various tests not enough? Would be happy to do it but my local Mac is spluttering.

Actually yeah, the CI is running fine with integration tests, so we are good.

@amoghrajesh

Copy link
Copy Markdown
Contributor

@davidsharp7 no further need to rebase, i will merge it once its green.

@amoghrajesh
amoghrajesh merged commit 967da37 into apache:mainMay 20, 2025
@boring-cyborg

Copy link
Copy Markdown

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

@amoghrajesh

Copy link
Copy Markdown
Contributor

@davidsharp7 good work on this one!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providersprovider:cncf-kubernetesKubernetes (k8s) provider related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Forced busybox init_container in kubernetes executor after upgrade to v3.

5 participants

@davidsharp7@amoghrajesh@potiuk@eladkal@jpmenil