-
Notifications
You must be signed in to change notification settings - Fork 199
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
Add helm chart for swck v0.7.0 #91
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this commitment. I would like to feed some rough problems in the first round.
Chart Structure
SWCK is a hybrid project that provides two unique components. We should provide two charts, "operator" and "adapter" to the heml repo.
No Reduction
Please don't remove anything from CRDs' YAML. Those details are important to users who will apply CRs.
The reduction process is hard to maintain. Once some APIs are updated upstream, we must take a long time to sync YAML.
"SkyWalking" Everywhere
I noticed you amended several places to apply "SkyWalking". Most of them are redundant since this is a Skywalking subproject.
Thanks for this review, I will submit a new version later. |
@dashanji Are you still working on this? |
Yes, sorry for the long delay, I will perfect it recently. |
Signed-off-by: dashanji <dashanjic@gmail.com>
* Update relavant doc. Signed-off-by: dashanji <dashanjic@gmail.com>
cd8472a
to
fc95698
Compare
Signed-off-by: dashanji <dashanjic@gmail.com>
@kezhenxu94 Please take a look. |
chart/operator/Chart.yaml
Outdated
# limitations under the License. | ||
|
||
apiVersion: v2 | ||
name: operator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you rename this to a more specific name such as skywalking-swck-operator
or something you prefer? To deploy this to docker hub we must have this name starts with skywalking-
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the detail, done!
chart/adapter/Chart.yaml
Outdated
# limitations under the License. | ||
|
||
apiVersion: v2 | ||
name: adapter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here in terms of the name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Signed-off-by: dashanji <dashanjic@gmail.com>
@@ -52,6 +52,66 @@ helm install "${SKYWALKING_RELEASE_NAME}" \ | |||
```shell | |||
export REPO=skywalking | |||
helm repo add ${REPO} https://apache.jfrog.io/artifactory/skywalking-helm | |||
## Install development version of Skywalking using master branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's also publish the snapshot Helm Chart to ghcr.io and publish the released to Docker Hub, if you want me to do these please let me know
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the released image has already been published to Docker Hub, but the helm chart has not. It looks like we need to have access to this address https://apache.jfrog.io/artifactory/skywalking-helm
. I would be very appreciative if you could publish it, and I will update the relevant doc for the published helm chart later.
The pr is related apache/skywalking#9626.
The templates file are mostly based on the release version of swck v0.7.0, but in order to reduce the size of the
crds.yaml
file, I removed most of the useless information such as description information.