eclipse / jifa

🔬 Online Heap Dump, GC Log, Thread Dump & JFR File Analyzer.
https://eclipse.github.io/jifa/
Eclipse Public License 2.0
543 stars 96 forks source link

feat: add jifa helm chart #268

Closed mark8s closed 8 months ago

mark8s commented 8 months ago

I found that jifa does not support helm deployment method, so I wrote one here. I've tested it and the service is available.

y1yang0 commented 8 months ago

This adds complexity but what's the gains?

mark8s commented 8 months ago

It is more convenient for version control, upgrades, and standardized deployment, maintenance, and use by enterprise users.

---- Replied Message ---- | From | Yi @.> | | Date | 03/19/2024 19:09 | | To | eclipse/jifa @.> | | Cc | mark @.>, Author @.> | | Subject | Re: [eclipse/jifa] feat: add jifa helm chart (PR #268) |

This adds complexity but what's the gains?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

mark8s commented 8 months ago

ok~

---- Replied Message ---- | From | Denghui @.> | | Date | 03/19/2024 19:33 | | To | eclipse/jifa @.> | | Cc | mark @.>, Author @.> | | Subject | Re: [eclipse/jifa] feat: add jifa helm chart (PR #268) |

@D-D-H commented on this pull request.

In manifests/README.md:

@@ -0,0 +1,72 @@ +## Deploy jifa by helm

Please add the license header to new files.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

y1yang0 commented 8 months ago

Please correct me if my understanding is wrong: This patch consolidates various configuration items scattered across different YAML files into a single YAML file(value.yaml?), and then utilizes these configuration items via referencing them in template YAML files. An obvious benefit I can see is that this makes it easier for users to configure various parameters for JIFA, and potentially, some platforms could visualize these parameters, among other things. However, on the flip side, a clear drawback is that in the future, each time we add a configuration item to JIFA's YAML, we would need to make corresponding changes to the Helm part. At a minimum, this would involve adding a new item to the values.yaml and then referencing it in the templates/*.yaml files. I believe that current and foreseeable configurations for JIFA will not be overly complex, and perhaps the attractiveness is not sufficient compared to the complexity introduced.

mark8s commented 8 months ago

For now, using native yaml is certainly the best fit, but using helm doesn't make deploying jira or iterating jira any more complicated. The complexity is now predictable because jifa does not use configuration files, and the parameters are implemented through args. My habit is to use helm, and in its current state of use, it's actually easier to use native yaml.

---- Replied Message ---- | From | Yi @.> | | Date | 03/19/2024 19:37 | | To | eclipse/jifa @.> | | Cc | mark @.>, Author @.> | | Subject | Re: [eclipse/jifa] feat: add jifa helm chart (PR #268) |

Please correct me if my understanding is wrong: This patch consolidates various configuration items scattered across different YAML files into a single YAML file(value.yaml?), and then utilizes these configuration items via referencing them in template YAML files. An obvious benefit I can see is that this makes it easier for users to configure various parameters for JIFA, and potentially, some platforms could visualize these parameters, among other things. However, on the flip side, a clear drawback is that in the future, each time we add a configuration item to JIFA's YAML, we would need to make corresponding changes to the Helm part. At a minimum, this would involve adding a new item to the values.yaml and then referencing it in the templates/*.yaml files. I believe that current and foreseeable configurations for JIFA will not be overly complex, and perhaps the attractiveness is not sufficient compared to the complexity introduced.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

D-D-H commented 8 months ago

@mark8s I wanna know what storageClassName you are using.

The current value in cluster.yml is alibabacloud-cnfs-nas which is only available in Alibaba Cloud.

We should not hardcode it since it depends on the vendor, so I plan to delete it.

mark8s commented 8 months ago

My k8s cluster is set to the default sc (nfs), so I didn't set a specific sc. If a default storage class is set in the cluster, you do not need to set a specific value. K8s will select the default storage class by default. Like below:

# kubectl get sc
NAME                   PROVISIONER                                                RECLAIMPOLICY   VOLUMEBINDINGMODE   ALLOWVOLUMEEXPANSION   AGE
nfs-client (default)   cluster.local/nfs-client-nfs-subdir-external-provisioner   Delete          Immediate           true                   211d

@D-D-H

mark8s commented 8 months ago

@D-D-H I have added the license information, please take a look

D-D-H commented 8 months ago

My k8s cluster is set to the default sc (nfs), so I didn't set a specific sc. If a default storage class is set in the cluster, you do not need to set a specific value. K8s will select the default storage class by default.

Thanks for the clarification. I've removed the hard-coded StorageClass.

D-D-H commented 8 months ago

@D-D-H I have added the license information, please take a look

Okay. I'll take a full review these two days.

D-D-H commented 8 months ago

@mark8s

Are you still working on this?