Skip to content

Conversation

@jpds
Copy link
Contributor

@jpds jpds commented Aug 23, 2018

Jira

Description

  • Here are some details about my PR, including screenshots of any UI changes:

Added Helm chart for Airflow.

Tests

  • My PR adds the following unit tests OR does not need testing for this extremely good reason:

This probably should run helm lint, happy to add it if someone can show me how to integrate it into Travis CI.

Commits

  • My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":

@jpds
Copy link
Contributor Author

jpds commented Aug 23, 2018

@dimberman @gsemet

@ashb
Copy link
Member

ashb commented Aug 23, 2018

This is a -1 from me - I don't like Helm's requirement to be run as an all-powerful user in the cluster.

@jpds
Copy link
Contributor Author

jpds commented Aug 23, 2018

@ashb That's not a hard-requirement from Helm, it's completely possible to deploy Tiller in a namespace with only access to that namespace.

@ashb
Copy link
Member

ashb commented Aug 23, 2018

I'm also not a fan of that model as it's a resource hog ;)

@dimberman
Copy link
Contributor

dimberman commented Aug 23, 2018 via email

@ashb
Copy link
Member

ashb commented Aug 23, 2018

Yeah, I'm waiting quite eagerly for Helm3. I'm not a fan of this right now, but I also don't feel that strongly about it, certainly not enough to veto or anything like that.

@dimberman
Copy link
Contributor

dimberman commented Aug 23, 2018 via email

@gsemet
Copy link

gsemet commented Aug 23, 2018

@ashb I do not see what your remark has to do with Airflow, to what I see and use in other charts in this repository, they almost all follow the same structure, and this airflow chart looks like every other. If you want to deploy this chart without Tiller, you can do helm template and then kubectl apply. Or use Helm in client mode only.

@dimberman
Copy link
Contributor

@jpds It looks like you removed the kube/deploy folder but it's not removed from the setup_kubernetes script https://github.com/apache/incubator-airflow/blob/5bd5a7bad638241c01ac1ddd0fe8f8c7a3e95d27/scripts/ci/kubernetes/setup_kubernetes.sh.

It looks like this PR has screwed up the kubernetes tests and now they're just running the normal airflow tests.

https://travis-ci.org/apache/incubator-airflow/jobs/419688983

@dimberman
Copy link
Contributor

Wait. No. This is on the master branch. Thats... not good. Going to investigate further.

@dimberman
Copy link
Contributor

Ok so that was caused by the recent dockerized ci pipeline. I'm addressing the issue here #3797

@dimberman
Copy link
Contributor

Lol so now that the tests have been fixed, let's revisit this :). @jpds could you please rebase?

@ron819
Copy link
Contributor

ron819 commented Nov 12, 2018

@jpds ping

@dimberman
Copy link
Contributor

@jpds could you please rebase this? Would love to start offering this as a simple solution for testing out the k8sExecutor :).

@gauthiermartin
Copy link
Contributor

@jpds Still interested in this feature as i might be interested to take over if you dont ?

@dimberman
Copy link
Contributor

@gauthiermartin I think it's safe at this point to take over this effort since it's been a few months.

@dimberman
Copy link
Contributor

@jpds I'm going to close this PR as stale. Please let me know if you want me to reopen it.

@dimberman dimberman closed this Jun 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants