Skip to content

Add deferrable mode to Kubernetes and GKE operators #28230

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.

Sign up for GitHub

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

Jump to bottom
Closed
MrGeorgeOwl wants to merge 18 commits into from
Closed

Add deferrable mode to Kubernetes and GKE operators #28230

MrGeorgeOwl wants to merge 18 commits into from

Conversation

@MrGeorgeOwl
Copy link
Contributor

@MrGeorgeOwl MrGeorgeOwl commented Dec 8, 2022

^ 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 newsfragments.

@boring-cyborg boring-cyborg bot added provider:cncf-kubernetes Kubernetes (k8s) provider related issues area:providers area:system-tests kind:documentation provider:google Google (including GCP) related issues labels Dec 8, 2022
@MrGeorgeOwl MrGeorgeOwl force-pushed the kubernetes-def-mode branch 8 times, most recently from 7790412 to f290082 Compare December 19, 2022 08:22
@MrGeorgeOwl MrGeorgeOwl force-pushed the kubernetes-def-mode branch 2 times, most recently from ca232d5 to 2843e15 Compare December 20, 2022 07:56
kaxil
kaxil reviewed Dec 21, 2022
View reviewed changes
kaxil
kaxil reviewed Dec 21, 2022
View reviewed changes
kaxil
kaxil reviewed Dec 21, 2022
View reviewed changes
kaxil
kaxil reviewed Dec 21, 2022
View reviewed changes
@kaxil kaxil mentioned this pull request Dec 21, 2022
Closed
kaxil
kaxil reviewed Dec 21, 2022
View reviewed changes
kaxil
kaxil reviewed Dec 21, 2022
View reviewed changes
kaxil
kaxil reviewed Dec 21, 2022
View reviewed changes
Copy link
Member

@kaxil kaxil left a comment

Choose a reason for hiding this comment

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

If you can separate this into 2 PRs - one for async mode for KPO and the other for async mode for GKE, that will help in reviewing

@VladaZakharova
Copy link
Contributor

VladaZakharova commented Dec 22, 2022 •
edited
Loading

As we are adding new feature to existing operator instead of creating new operator for deferrable mode, we decided to also use KubernetesPodOperator as a base class for the GKEStartPodOperator to instantiate needed behavior. So splitting this to 2 separate PRs will be not an option here as we will also need to wait until KubernetesPodOperator will be merged to main and only then add new PR for the GKEStartPodOperator.

kaxil
kaxil reviewed Jan 14, 2023
View reviewed changes
@kaxil
Copy link
Member

kaxil commented Jan 14, 2023 •
edited
Loading

@eladkal @potiuk @kaxil Hi Team! :) Could we please review the changes in this PR?

As I had mentioned in #28230 (comment) , if the PR is big, it will take time to review and makes our life as reviewers hard. So please expect it will take some time to review the PR fully

kaxil
kaxil reviewed Jan 14, 2023
View reviewed changes
kaxil
kaxil reviewed Jan 14, 2023
View reviewed changes
MrGeorgeOwl and others added 17 commits January 17, 2023 08:33
@MrGeorgeOwl @VladaZakharova
Add deferrable mode to Kubernetes and GKE operator
95fb8f7
@MrGeorgeOwl @VladaZakharova
Fix import of the dag
da29787
@VladaZakharova
update kubernetesPodOperator to return xcom value
5928c66
@MrGeorgeOwl @VladaZakharova
Change cleanup method to first implementation
e109aad
@VladaZakharova
Add logic to convert config_file content to prevent memory leak
3bf7750
@MrGeorgeOwl @VladaZakharova
Middlework.Change call to db, refactor temporary file creation and li…
b0e464e 
…ttle refactoring
@MrGeorgeOwl @VladaZakharova
Middlework.Add caching to async request to db, remove error imports, …
4cba01a 
…remove method for writing permanent config file
@MrGeorgeOwl @VladaZakharova
Change the way of authentication to GKE cluster
afd17ac
@MrGeorgeOwl @VladaZakharova
Fix marking task failed, pending for additional resources, add failed…
c62aa31 
… event handling
@MrGeorgeOwl @VladaZakharova
Add additional check for timeout event
dc857e1
@VladaZakharova
Add caching for Kubernetes client
665b0f9
@VladaZakharova
Add logging on all stages of pod run
0becd01 
Add logging on all stages of pod run, also description for parameters and unit tests
@VladaZakharova
Fix tests for gke operator and kubernetes hook
c431381
@VladaZakharova
Add asgiref module to cncf provider.yaml
e68cc21
@MrGeorgeOwl @VladaZakharova
Change version ranging, add deprecation messages
ff813af
@MrGeorgeOwl @VladaZakharova
Add tests for GKE pod hook, refactor test for GKE operator
0e438de
@VladaZakharova
Add tests for GKE trigger + add some params for it for correct inheri…
fb6f2c5 
…tance
@VladaZakharova
Copy link
Contributor

VladaZakharova commented Jan 18, 2023

@kaxil @MrGeorgeOwl @lwyszomi
Hi Team!
Please, check separate PR for the KubernetesPodOperator: #29017
Hope that it can really speed up little bit the review process :)

@eladkal
Copy link
Contributor

eladkal commented Mar 4, 2023 •
edited
Loading

@MrGeorgeOwl can you rebase and fix conflicts?

@VladaZakharova
Copy link
Contributor

VladaZakharova commented Mar 5, 2023

@eladkal
Hi!
This PR was splitted to 2 separate. Please, check the link in the comment above :)
I think this PR can be closed in this case. But i am not the owner, so i cant.

@eladkal
Copy link
Contributor

eladkal commented Mar 5, 2023

@eladkal Hi! This PR was splitted to 2 separate. Please, check the link in the comment above :) I think this PR can be closed in this case. But i am not the owner, so i cant.

Thanks. i thought by split some parts were extracted then we have some parts left here as a followup. Thanks for the clarification

@eladkal eladkal closed this Mar 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kaxil kaxil kaxil left review comments

@jedcunningham jedcunningham Awaiting requested review from jedcunningham

@turbaszek turbaszek Awaiting requested review from turbaszek

@mik-laj mik-laj Awaiting requested review from mik-laj

+1 more reviewer

@VladaZakharova VladaZakharova VladaZakharova left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area:providers area:system-tests kind:documentation provider:cncf-kubernetes Kubernetes (k8s) provider related issues provider:google Google (including GCP) related issues

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@MrGeorgeOwl @VladaZakharova @kaxil @eladkal