Skip to content
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

Integrate BanyanDB cluster as storage solution #145

Merged
merged 4 commits into from
Mar 27, 2024

Conversation

ButterBright
Copy link
Member

No description provided.

@wu-sheng
Copy link
Member

Default values of standalone.image.repository and tags in https://github.com/apache/skywalking-banyandb-helm should be official releases before the integration.

@wu-sheng wu-sheng added this to the 4.6.0 milestone Mar 27, 2024
@wu-sheng wu-sheng added the enhancement New feature or request label Mar 27, 2024
Copy link
Member

@kezhenxu94 kezhenxu94 left a comment

Choose a reason for hiding this comment

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

Overall looks good

@@ -39,6 +39,6 @@ dependencies:
repository: https://raw.githubusercontent.com/bitnami/charts/archive-full-index/bitnami
condition: postgresql.enabled
- name: skywalking-banyandb-helm
Copy link
Member

Choose a reason for hiding this comment

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

Let's rename this to skywalking-banyandb

Copy link
Member

@kezhenxu94 kezhenxu94 left a comment

Choose a reason for hiding this comment

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

Thanks

Comment on lines +43 to +44
version: 0.0.0-8bdff75
repository: oci://ghcr.io/apache/skywalking-banyandb-helm
Copy link
Member

Choose a reason for hiding this comment

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

You should point to docker hub, rather than ghcr. Why is this changed?

Copy link
Member

Choose a reason for hiding this comment

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

Could you release 0.2.0 of the banyandb helm?

Copy link
Member

Choose a reason for hiding this comment

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

Meanwhile, where is oci://registry-1.docker.io/apache? Is this repository existing?

Copy link
Member Author

Choose a reason for hiding this comment

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

We have not yet released banyandb and banyandb-helm, so I temporarily pointed the repository to ghcr. I will update these values together after the releases are completed.

Copy link
Member

Choose a reason for hiding this comment

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

We have the previous(0.1.0) version, https://github.com/apache/skywalking-banyandb-helm.

Are you going to release that soon? Otherwise, once skywalking-helm 4.6 is releasing, it will carry the unreleased BanyanDB version(some snapshot version). This is not correct in the ASF.

Copy link
Member Author

Choose a reason for hiding this comment

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

As far as I know, we will release shortly after the completion of testing on banyandb, and it should be relatively fast.

Copy link
Member

Choose a reason for hiding this comment

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

Could you post an issue on Helm 4.6.0 milestone to track this, and you should fix this

Copy link
Member

Choose a reason for hiding this comment

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

@hanahmily told me, your deployment should not care about BanyanDB 0.5 or 0.6(dev or unreleased). So, this should not be a blocker for you.
I know 0.6 is much better, so you only need to keep all in 0.5 by default, and upgrade to 0.6 once it is released.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay.

Copy link
Member Author

Choose a reason for hiding this comment

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

@hanahmily told me, your deployment should not care about BanyanDB 0.5 or 0.6(dev or unreleased). So, this should not be a blocker for you. I know 0.6 is much better, so you only need to keep all in 0.5 by default, and upgrade to 0.6 once it is released.

There is a feature related to etcd authentication that has not been implemented in version 0.5. It seems that we still need to rely on the upcoming BanyanDB 0.6 version. After discussing with @hanahmily , we decide to change these tags later on.

@wu-sheng wu-sheng merged commit 917a6ff into apache:master Mar 27, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants