Helm/refactor nifi cluster - Githubissues

konpyutaika / nifikop

The NiFiKop NiFi Kubernetes operator makes it easy to run Apache NiFi on Kubernetes. Apache NiFI is a free, open-source solution that support powerful and scalable directed graphs of data routing, transformation, and system mediation logic.

https://konpyutaika.github.io/nifikop/

Apache License 2.0

122 stars 40 forks source link

Helm/refactor nifi cluster #341

Closed umarhussain15 closed 6 months ago

umarhussain15 commented 6 months ago

Q	A
Bug fix?	no	yes
New feature?	no	yes
API breaks?	no	yes
Deprecations?	no	yes
Related tickets
License	Apache 2.0

What's in this PR?

This PR is mostly refactoring the helm chart nifi-cluster.

It removes the template files under helm/nifi-cluster/charts/zookeeper since we get the zookeeper chart via dependencies from Chart.yaml and that will be packaged with nifi-cluster.
The webProxyHosts value substitution in nifi-cluster.yaml is fixed to convert it to yaml so it is yaml array instead of spaced values. The default value is fixed to array in values.yaml as well.
The nifi-cluster.yaml is refactored to remove extra lines in the output.
Documentation links in values.yaml updated to correct one. More comments added. And the chart's README is regenerated (Maybe at some point, add the option to Makefile to generate it?)
Warning added for SSL certificate generation via cert-manager to indicate that hostname should be at max 64 bytes.

Checklist

[x] Implementation tested
[x] Error handling code meets the guideline
[x] Logging code meets the guideline
[x] User guide and development docs updated (if needed)
[x] Append changelog with changes

mh013370 commented 6 months ago

We should also update this zookeeper chart dependency. It's 2 major versions behind.

We're running with the latest bitnami zookeeper docker image, so i know that's safe but perhaps we should also update the chart dependency?

https://artifacthub.io/packages/helm/bitnami/zookeeper

umarhussain15 commented 6 months ago

I have added helm-chart-version-match in Makefile which is now called with test and test-with-vendor. It will fail if there is a mismatch between helm chart version value under /helm directory

mh013370 commented 6 months ago

any concerns @juldrixx ?

umarhussain15 commented 6 months ago

Hi @juldrixx, the zookeeper chart will be packaged inside the nifi-cluster chart. Running make helm-pacakge in the pipeline generates the tgz file in which the under charts folder the zookeeper template files are present. So access to bitnami chart repo will only be required during pipeline generating the helm chart package. Users of the nifi-cluster will only need access to oci://ghcr.io/konpyutaika/helm-charts.

You can check the content of the packaged helm chart for this PR:

make helm-pacakge generates nifi-cluster-1.6.0.tgz
tar -tvf nifi-cluster-1.6.0.tgz --wildcards 'nifi-cluster/charts/zookeeper/*' lists all the templates present for zookeeper

mh013370 commented 6 months ago

So this is effectively just removing it from version control, which i'm on board with and it simplifies maintenance of the dependent chart!

mh013370 commented 6 months ago

Okay, this all looks good to me. I think @juldrixx 's concern is addressed: The zookeeper chart is still bundled (for airgap purposes), but the bundling is performed by make at release time and will not be maintained in version control from now on.

mh013370 commented 6 months ago

@umarhussain15 : Can you please update the CHANGELOG with the changes/additions you've made?