Skip to content

Add helm chart for Apache Druid#11163

Merged
asdf2014 merged 2 commits intoapache:masterfrom
asdf2014:helm
Apr 29, 2021
Merged

Add helm chart for Apache Druid#11163
asdf2014 merged 2 commits intoapache:masterfrom
asdf2014:helm

Conversation

@asdf2014
Copy link
Copy Markdown
Member

Description

This patch added a helm chart for Apache Druid.


This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

cc @maver1ck @AWaterColorPen

@asdf2014 asdf2014 added the Helm Chart https://github.com/apache/druid/tree/master/helm/druid label Apr 27, 2021
@FrankChen021
Copy link
Copy Markdown
Member

Do we need a tutorial like the tutorial of docker ( docs/tutorials/docker.md ) ?

@asdf2014
Copy link
Copy Markdown
Member Author

Hi, @FrankChen021 . Thanks for your comment. I also considered whether to publish the doc, for example, put the helm/README.md file in the docs/tutorials directory. However, although this Helm Chart can deploy on the K8S cluster directly, there are still a few things that need to be completed before the document is published on the official website. For example: upgrade the Druid image version, support HDFS cluster deployment, release the Release version of Druid's Helm Chart by configuring Github Action, and etc. But these things are difficult to complete in a short period of time, so we should finish them in these follow-up PRs.

Copy link
Copy Markdown
Member

@QiuMM QiuMM left a comment

Choose a reason for hiding this comment

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

Nice work, lgtm.

@asdf2014 asdf2014 merged commit ed81548 into apache:master Apr 29, 2021
@asdf2014 asdf2014 deleted the helm branch April 29, 2021 04:38
@jihoonson
Copy link
Copy Markdown
Contributor

Hey @asdf2014, have you sorted out the potential license issue before you merge this PR? I don't see it anywhere. As Julian suggested in https://lists.apache.org/thread.html/ra93200245a27fb77ff0a1cfc17d21b7efbbb263ae422d6d2cc5ade11%40%3Cdev.druid.apache.org%3E, we may need to go through a process similar to the IP clearance for podling projects.

@asdf2014
Copy link
Copy Markdown
Member Author

Hi, @jihoonson. I didn't notice it, let me take a look. Thank you very much for reminding.

@clintropolis
Copy link
Copy Markdown
Member

Hey @asdf2014, have you sorted out the potential license issue before you merge this PR? I don't see it anywhere. As Julian suggested in https://lists.apache.org/thread.html/ra93200245a27fb77ff0a1cfc17d21b7efbbb263ae422d6d2cc5ade11%40%3Cdev.druid.apache.org%3E, we may need to go through a process similar to the IP clearance for podling projects.

I think we should consider reverting this until we get this resolved, after reading through https://incubator.apache.org/ip-clearance/, and looking at https://incubator.apache.org/ip-clearance/tika-tika-helm.html which appears to be a similar helm chart donation for another project, where I see linking to a voting thread, etc.

@clintropolis
Copy link
Copy Markdown
Member

unrelated nitpick, i think putting this stuff in distribution/helm/, which is also where the Docker stuff lives, makes more sense to me than top level helm/

@clintropolis
Copy link
Copy Markdown
Member

btw, in general I think it makes sense to maintain it here, so +1 on the idea (even though I know very little about helm)

@asdf2014
Copy link
Copy Markdown
Member Author

Hi, @clintropolis . Thanks for your comments and support. As far as I know, the license of helm/charts is
Apache License 2.0. Maintaining Apache Druid’s own Helm Chart in the Apache Druid repo should not have copyright issues. I heard about the process just mentioned by @jihoonson for the first time, so I’ll look at it. In addition, Apache Superset has actually done this, and we can get more details through the link https://github.com/apache/superset/tree/master/helm/superset.

@xvrl
Copy link
Copy Markdown
Member

xvrl commented Apr 29, 2021

@asdf2014 my understanding is that we still need to highlight were the code came from, just like we do for other Apache License 2.0 code we include or borrow.

Also, has this chart been tested? Does it still work?

@jihoonson
Copy link
Copy Markdown
Contributor

jihoonson commented Apr 29, 2021

I haven't closely checked the IP clearance or other ASF processes to adopt codes from outside the project after graduation. But my 2 cents is we probably need a PMC vote for adopting because we need a plan for how to maintain and continue developing the code.

@asdf2014
Copy link
Copy Markdown
Member Author

Hi, @xvrl. Thanks for your comment. I agree with you, the follow-up related documents will be improved. I also mentioned before that this Druid Helm Chart is completely usable, and it has been tested several times last week.

@asdf2014
Copy link
Copy Markdown
Member Author

Hi, @jihoonson. Thank you for your comments. It was previously maintained by @maver1ck @AWaterColorPen and I. Currently, https://github.com/helm/charts/tree/master/incubator/druid cannot be maintained. So, we want maintain it here, just like other Apache projects did, such as Apache Superset. Indeed, I agree with your proposal. Let's raise up a PMC vote. If it really fails to pass the vote, we can revert this PR ASAP. Could you please help initiate the vote?

@jihoonson
Copy link
Copy Markdown
Contributor

@asdf2014 I'm not saying we should not accept this. I'm saying this doesn't seem the right process. Have you read the ASF IP clearance doc? The IP clearance is not just about the license. All codes in the ASF projects are owned by the ASF and the IP clearance process is required to make this part clear. The IP clearance doc explicitly says

This is for projects and PMCs that have already been created and are receiving a code donation into an existing codebase. Any code that was developed outside of the ASF SVN repository and our public mailing lists must be processed like this, even if the external developer is already an ASF committer.

I think we should revert this PR and go through the IP clearance process because this was developed outside of ASF. Please go back to the dev thread for this issue and add more details there (not here) for people like me who don't know much about the helm chart such as

  • What is the current status of the project?
    • Does it reflect the most recent release of Druid?
    • How is it being tested?
  • Why is it best to host the helm chart in the druid repo? What alternatives have you considered?
  • After migration, what is your plan for automatic testing?

I don't think this is an exhaustive list. Please add details as much as possible.

@asdf2014
Copy link
Copy Markdown
Member Author

@jihoonson I see. And I agree with your suggestion. Let us follow the process of IP clearance to eliminate potential problems. I will do my best to provide answers to these questions. I have discussed it with other maintainers, and there are many improvements to it that cannot be contributed to the open source community. So, let's unify the maintenance of Druid's Helm Chart as soon as possible. Thanks a lot for your patience.

gianm added a commit to gianm/druid that referenced this pull request Feb 14, 2024
The helm chart was originally moved here in apache#11163 from
https://github.com/helm/charts/tree/master/incubator/druid after the
helm/charts repository was deprecated. However, it has been excluded
from releases since then, due to uncertainty around whether we need
IP clearance. We have not had volunteers willing to sort this out,
so this patch removes the code.

It can be re-added if a volunteer is available to sort out the
IP clearance process.

See thread at: https://lists.apache.org/thread/ygyzt23m06vc775nq5dsm349rf0j47dg
@gianm gianm mentioned this pull request Feb 14, 2024
FrankChen021 pushed a commit that referenced this pull request Feb 21, 2024
The helm chart was originally moved here in #11163 from
https://github.com/helm/charts/tree/master/incubator/druid after the
helm/charts repository was deprecated. However, it has been excluded
from releases since then, due to uncertainty around whether we need
IP clearance. We have not had volunteers willing to sort this out,
so this patch removes the code.

It can be re-added if a volunteer is available to sort out the
IP clearance process.

See thread at: https://lists.apache.org/thread/ygyzt23m06vc775nq5dsm349rf0j47dg
@asdf2014 asdf2014 mentioned this pull request Feb 28, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Helm Chart https://github.com/apache/druid/tree/master/helm/druid

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants