cloudfoundry-incubator / kubecf

Cloud Foundry on Kubernetes
Apache License 2.0
115 stars 62 forks source link

Spike: Allow usage of the official NATS helm chart #5

Open viovanov opened 5 years ago

viovanov commented 5 years ago

There should be a feature flag that allows users to deploy the official NATS helm chart instead of the converted BOSH release.

mook-as commented 4 years ago

Some notes:

@viovanov, thoughts (especially on that second point)?

There's https://github.com/helm/helm/issues/2492 that kind-of has stuff that would make that possible, but that's been closed in favour of Lua scripting in helm 3, which hasn't been implemented yet. So as far as I can tell this isn't possible even if we transitioned to helm 3.

alex-slynko commented 4 years ago

Hi there

I don't think kubecf should force an operator to use nats helm chart. But there should be an option for the operator to come with NATS deployed. The operator can figure out how to deploy NATS on their own.

As an operator, I want to reuse my NATS if I have credentials. As a Cloud Foundry contributor, I want to deploy without NATS at all. "Where we're going, we don't need NATS"

These things overlap into one - I want to deploy without any given part of Cloud Foundry, but be able to provide credentials/address of that part manually.

PS. It is even more important from our side because it would much simpler for us to provide just Eirini helm chart(s) with placeholders for different credentials/addresses.

kramerul commented 4 years ago

We have done some analysis of existing helm chart on how they could be integrated with cf-operator. Here is our summary:

Nats

:warning: Passwords are not stored in secrets. They are rendered into a ConfigMap

:warning: Name of service nats-client doesn't map to BOSH domain nats.service.cf.interal

:white_check_mark: TLS is not used by kubecf and the nats helm-chart

Mysql

:white_check_mark: SSL certificates can be specified with the help of a secret. This should work with cf-operator.

:warning: kubecf requires multiple databases with one credential each. mysql helm template doesn't provide this.

:warning: Password secret uses mysql-password as key for the password and not password, which is the default mapping of cf-operator

:warning: Password secret includes 2 passwords: mysql-root-password and mysql-password

Eirini

:warning: Name of service eirini-opi doesn't map to BOSH domain eirini.service.cf.interal

:warning: rootfs-patcher requires preinstalled bit-service

:warning: secrets-{{ .Values.scf.version }}-{{ .Values.scf.secrets_generation_counter }} doesn't match the current naming of secrets in cf-operator

HA proxy

:warning: No general purpose helm chart available (only as ingress controller)

Uaa (from eirini-release)

:warning: mysql included

:warning: built using fissile

Conclusion

Using existing helm charts will cause a lot of problems. Some are under control of cf and could be adapted upstream.

mook-as commented 4 years ago

See also: SUSE/scf#2865

Given the outcome of that, and how the official NATS chart is kind of terrible at being a subchart, I'm going to close this (well, move it to review) and says we don't want to use that chart.

I think, if we want to use composable pieces in the future, we'd need to write at least some ourselves to ensure they're geared towards composability >_< (and that sounds like a tautology…) At least, the upstream helm charts are not it, and from what I've seen so far, helm charts in general don't seem like they're a great solution (though that last part could still change).

voelzmo commented 4 years ago

As a side note: I think the official way to install NATS on k8s would be https://github.com/nats-io/nats-operator. The helm chart by bitname definitely has interesting issues, not sure how many people really use something that stores passwords in configMaps and not in secrets.

troytop commented 4 years ago

@viovanov please close this if you agree with @mook-as and @voelzmo 's assessment.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. If this issue is still relevant then let us know how to help move it forward. Thank you for your contributions.

viovanov commented 4 years ago

@mook-as - could we try to use the nats operator that @voelzmo mentioned? It has a helm chart. Not sure it's published anywhere, but we can use the release bundles.