Skip to content

Add istio test, use curl /quitquitquit to exit sidecar, and some othe… #33306

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

Merged
merged 8 commits into from
Aug 23, 2023

Conversation

Owen-CH-Leung
Copy link
Contributor

@Owen-CH-Leung Owen-CH-Leung commented Aug 11, 2023

Previous 2 PR: #31389, #31390

Refactor the KPO to include istio support. Addressing all the feedback given by others such as:

  • removing extra parameter in KPO, and add logic to programatically check whether istio-proxy is injected
  • Use curl localhost:15020/quitquitquit to kill the sidecar when istio is enabled, if istio is not enabled it will fallback to using API call delete_namespaced_pod
  • No new element was added in OnFinishAction. Developers can use existing arguments and the operator will check if sidecar is injected, and use different methods to kill the pod depending on whether sidecar is injected or not

@boring-cyborg boring-cyborg bot added area:providers provider:cncf-kubernetes Kubernetes (k8s) provider related issues labels Aug 11, 2023
@Owen-CH-Leung Owen-CH-Leung marked this pull request as ready for review August 11, 2023 08:03
@Owen-CH-Leung Owen-CH-Leung requested a review from potiuk August 15, 2023 02:27
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.

LGTM. But I would love someone elses to take a look as well - who knows better the istio/k8s relations.

@Owen-CH-Leung
Copy link
Contributor Author

@potiuk @eladkal Apology - I just tested locally using breeze with a custom KubernetesPodOperator dag and with istio 1.18.2 enabled. It's still not killing the istio-proxy. I've figured out the fix though (just small code change). Will be creating PR in 1 or 2 hours.

Setting up local airflow env to run KPO with istio enabled isn't so straight forward, so I'll document the steps that I've done here for record also. (FYI I'm using the docker-desktop which comes with k8s cluster and kubectl client)

  1. use breeze to start up airflow as usual (breeze start-airflow --backend postgres --load-example-dags --db-reset)
  2. run the command kubectl config view --minify --flatten -o json --context=docker-desktop. This should output a long json format
  3. Copy the json format output that you got before, and in airflow UI, under the Admin/Connections tab, you should see a kubernetes_default row. Click edit record
  4. You should see a row Kube Config (JSON format). Paste your json output into that row and saved.
  5. The example dags that breeze creates didn't have any DAG that uses a KPO. So you'll have to create your own DAG. Feel free to use mine.
    my_k8s_dag.txt
  6. rename this file into a .py file and put it under files/dags in your local airflow directory
  7. Restart your airflow (schedule, trigger, webserver...everything). Now you should see your DAG appear in the airflow UI
  8. Download istio. (follow this link for detailed instructions). Make sure istioctl is accessible
  9. Don't forget to run kubectl label namespace airflow istio-injection=enabled to enable istio-proxy sidecar injection in the namespace
  10. You are now ready to run the custom KPO Dag =)

@potiuk
Copy link
Member

potiuk commented Aug 26, 2023

OH... Would be nice to get it into breeze k8s command :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:cncf-kubernetes Kubernetes (k8s) provider related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants