Skip to content

Refactor GKE hooks #38404

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
Merged
potiuk merged 1 commit into from
Mar 25, 2024
Merged

Refactor GKE hooks #38404

potiuk merged 1 commit into from
Mar 25, 2024

Conversation

@moiseenkov
Copy link
Contributor

@moiseenkov moiseenkov commented Mar 22, 2024

In order to reduce code duplication and high complexity of the implementation, GKE hooks were refactored in this PR:

  1. Existing GKEHook and GKEAsyncHook provide interface to GKE API remain unchanged.
  2. The new GKEKubernetesHook(GoogleBaseHook, KubernetesHook) was introduced as a replacement for redundant hooks that used to contain duplicated methods that now are simply inherited from GoogleBaseHook and KubernetesHook:
  • GKEDeploymentHook (deprecated)
  • GKECustomResourceHook (deprecated)
  • GKEPodHook (deprecated)
  • GKEJobHook (deprecated)
  1. By following the same pattern, the new GKEKubernetesAsyncHook(GoogleBaseAsyncHook, AsyncKubernetesHook) was introduced as a replacement for redundant hooks that used to contain duplicated methods that now are simply inherited from GoogleBaseAsyncHook and AsyncKubernetesHook:
  • GKEPodAsyncHook (deprecated)
  1. New hooks (GKEKubernetesHook, GKEKubernetesAsyncHook) provide access to the standard Kubernetes API with a GKE authentication.
  2. Slightly adjusted system tests (they are all green now).
    system tests
  3. Except tiny changes, unit tests remain the same in order to prove stability of the code after refactoring. In the following PR we will disconnect GKE operators from the deprecated hooks, so it will be easy to remove them in the future.

@boring-cyborg boring-cyborg bot added area:providers area:system-tests provider:cncf-kubernetes Kubernetes (k8s) provider related issues provider:google Google (including GCP) related issues labels Mar 22, 2024
@VladaZakharova
Copy link
Contributor

VladaZakharova commented Mar 25, 2024

Hi @potiuk ! Can you please take a look on this PR? This is a blocker for several other PRs for GKE :c

potiuk
potiuk approved these changes Mar 25, 2024
View reviewed changes
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

Sure. Not a complex one.

@potiuk potiuk merged commit 32ed83b into apache:main Mar 25, 2024
@eladkal eladkal mentioned this pull request Apr 10, 2024
Closed
@moiseenkov moiseenkov mentioned this pull request May 17, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@potiuk potiuk potiuk approved these changes

@jedcunningham jedcunningham Awaiting requested review from jedcunningham

@hussein-awala hussein-awala Awaiting requested review from hussein-awala

Assignees

No one assigned

Labels

area:providers area:system-tests 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.

3 participants

@moiseenkov @VladaZakharova @potiuk