Thank you @ingmarfjolla for this! I will review it tomorrow!

@ingmarfjolla I also moved this PR to draft status for now until we get closer to a more "final" review.

hey @edeandrea thanks for taking a look! Let me answer the questions now.

There doesn't seem to be a chart for the UI project. This may need to be created manually since the UI project isn't Quarkus and there is no extension.

This would definitely be done manually and apart of the script. Since it wasn't using the extension, I wanted to push up this draft first to get feedback on the pieces utilizing the extension since the logic for this will be separate anyway but that's next on my agenda to do.

there is no "uber" chart where a user could easily "helm install" the entire system. That work would need to happen in the generate-helm-charts.sh script. Or maybe we hand-craft a "top-level" chart which references the other charts as dependencies? Just spit-balling. If we go the [dependencies route](https://quarkiverse.github.io/quarkiverse-docs/quarkus-helm/dev/index.html#helm-dependencies), then do the depended-upon projects need to be in a referenceable repo somewhere? Just thinking out loud.

This kind of depended on the conversation about different charts based on the JVM version matrix (java 11, java 17, native). I'd like to have an idea which way is preferred before I tried to automate that "uber" chart.

I agree with your idea that a single chart with multiple values files is the cleaner way to go, so I could investigate using helm profiles. I ran into issues with it at first so i'll try to see what issues I run into and open issues in the quarkus-helm repo accordingly. The only differences (to me) seem like image name, the resource requests, and maybe some labels and annotations.

So would there be a chart for each deployment (Kubernetes / Openshift / Minikube / Knative) or a single chart with all the resources and then multiple value files? that might be a little tougher since some deployment types had different resources than others

There is no chart for the monitoring stack. Should there be?
Same answer as for the UI component

When I ran it locally it seems like it did not generate any java17 resources. See screenshot below.
Isn't that expected? Unless you were able to switch to 17 locally when the script gets to that point? My script fails when it gets to the 17 point since I'm using 11 locally (or maybe theres something else I'm missing)

forgot to add on the topic of helm chart dependencies, Helm supports using a path to the directory instead of putting the charts in a repo: https://helm.sh/docs/helm/helm_dependency/#synopsis. I do need to do some more research because right now in the rest-fights directory for the downstream descriptors I just placed them in the charts directory instead of making them dependencies which installs them, but I'm not sure the downside of doing that VS explicitly making them dependencies

There doesn't seem to be a chart for the UI project. This may need to be created manually since the UI project isn't Quarkus and there is no extension.

This would definitely be done manually and apart of the script. Since it wasn't using the extension, I wanted to push up this draft first to get feedback on the pieces utilizing the extension since the logic for this will be separate anyway but that's next on my agenda to do.

I figured as much

there is no "uber" chart where a user could easily "helm install" the entire system. That work would need to happen in the generate-helm-charts.sh script. Or maybe we hand-craft a "top-level" chart which references the other charts as dependencies? Just spit-balling. If we go the [dependencies route](https://quarkiverse.github.io/quarkiverse-docs/quarkus-helm/dev/index.html#helm-dependencies), then do the depended-upon projects need to be in a referenceable repo somewhere? Just thinking out loud.

This kind of depended on the conversation about different charts based on the JVM version matrix (java 11, java 17, native). I'd like to have an idea which way is preferred before I tried to automate that "uber" chart.

I figured that as well

I agree with your idea that a single chart with multiple values files is the cleaner way to go, so I could investigate using helm profiles. I ran into issues with it at first so i'll try to see what issues I run into and open issues in the quarkus-helm repo accordingly. The only differences (to me) seem like image name, the resource requests, and maybe some labels and annotations.

My gut says this is the "right" way to go. Why duplicate everything if the only difference are some things which should be templatized?

I'm curious though about what @cescoffier and @agoncal think. Maybe @ebullient or @holly-cummins have an opinion too?

So would there be a chart for each deployment (Kubernetes / Openshift / Minikube / Knative) or a single chart with all the resources and then multiple value files? that might be a little tougher since some deployment types had different resources than others

Yes I think there would still need to be separate charts for the different deployment types. As you mention each type has different resources that are being deployed (i.e. Deployment vs DeploymentConfig vs Knative Service, values/properties in the ConfigMaps that are deployed are different, etc).

Now you could argue you could use templates/functions/etc within the templates themselves to do that, but honestly I think I'd prefer to keep them simple & separate. We already have this level of separation via the Quarkus Kubernetes extensions (i.e. things in each project's src/main/kubernetes folders). Keeping things as simple as possible so its easily maintained in the future is one of the overall "guiding principles" of this project in the first place.

There is no chart for the monitoring stack. Should there be?

Same answer as for the UI component

I thought so too

When I ran it locally it seems like it did not generate any java17 resources. See screenshot below.

Isn't that expected? Unless you were able to switch to 17 locally when the script gets to that point? My script fails when it gets to the 17 point since I'm using 11 locally (or maybe theres something else I'm missing)

The script should generate descriptors for java 11, java 17, & native, as it does currently with the generate-k8s-resources.sh. The script needs to be run with Java 17, though (see https://github.com/quarkusio/quarkus-super-heroes/blob/main/.github/workflows/create-deploy-resources.yml#L44-L49).

That being said, the script does force the compiler version down to 11 when generating for java 11 (see https://github.com/quarkusio/quarkus-super-heroes/blob/main/scripts/generate-k8s-resources.sh#L49).

forgot to add on the topic of helm chart dependencies, Helm supports using a path to the directory instead of putting the charts in a repo: https://helm.sh/docs/helm/helm_dependency/#synopsis. I do need to do some more research because right now in the rest-fights directory for the downstream descriptors I just placed them in the charts directory instead of making them dependencies which installs them, but I'm not sure the downside of doing that VS explicitly making them dependencies

I'm with you - I don't know a lot about it either. So maybe for now we do the poor man's approach, at least until we "shake out" the rest of the design (single chart, multiple values files, etc). Then if there's a way we can make it cleaner by using dependencies, maybe we do that.

@cescoffier / @agoncal any thoughts on the approach here?

@ingmarfjolla I've been thinking a bit about this and honestly I think the route of having a single chart per deployment type with multiple values files is the right way to go.

One thing I noticed though which would prevent that - when looking at the charts which were generated, it doesn't seem that the resource requests/limits are templatized in the deployment.yaml/deploymentconfig.yaml files which are generated (spec.template.spec.containers[0].resources)?

If we are to go the single chart/multiple values files route, then that would need to be templatized as well, since the values change depending on if it is jvm vs native.

Is that something that the current version of the extension does not templatize?

Also another question - depending on the deployment type there may be different resources that get generated. For example, when generating for OpenShift, there are Route objects, whereas when generating for Kubernetes there are Ingress objects. If we went the single chart/multiple values files route, how would that work? See https://github.com/quarkusio/quarkus-super-heroes/tree/main/rest-villains/src/main/kubernetes as an example. For openshift deployment type, there are additional resources in openshift.yml.

hey @edeandrea I agree, the one chart per deployment type is the cleanest and most similar to what we'd probably see at a client or more mature org.

Is that something that the current version of the extension does not templatize?

so out of the box it does not, but you can specify a helm expression: https://quarkiverse.github.io/quarkiverse-docs/quarkus-helm/dev/index.html#helm-expressions like this. So to templatize the labels and annotations you would need to add that.

When i did play around with this feature I had to add it to the app properties, I had a few issues using the maven command located in the build-helm script but it did work, just up to preferences if you were ok adding stuff to the app properties.

depending on the deployment type there may be different resources that get generated. For example, when generating for OpenShift, there are Route objects, whereas when generating for Kubernetes there are Ingress objects

this was one of the reasons I just went with the multiple chart approach, helm gives you flags to enable resources, so in your Openshift vs Kubernetes example the values.yaml might have something like "ingress.enabled" and you can set it to true for Kubernetes and false for Openshift and then it will just create a route instead (this is hypothetical it'll probably need more thought). It looks like the extension supports that here: https://quarkiverse.github.io/quarkiverse-docs/quarkus-helm/dev/index.html#conditionally-enable-disable-resources but I myself haven't used it yet. I'm not sure how to deal with the copying and pasting portion though in terms of the script. Probably something like "if we're doing the kubernetes deployment type, only copy the ingress) or something like that.

@someth2say wanted to add some commits/take over the pr, so please add your thoughts too!

When i did play around with this feature I had to add it to the app properties, I had a few issues using the maven command located in the build-helm script but it did work, just up to preferences if you were ok adding stuff to the app properties.

Depends on what we are adding. We add plenty of kubernetes/minishift/knative stuff there, so if there is stuff specific for helm then so be it. Other things (like the resource limits/requests), we input as -D params when we generate the resources, that way they aren't hard-coded in application.properties.

depending on the deployment type there may be different resources that get generated. For example, when generating for OpenShift, there are Route objects, whereas when generating for Kubernetes there are Ingress object

I wasn't just talking about when deploying the resources. If you look for example at one of the java17-kubernetes.yml vs java17-openshift.yml, the resources in each are different. In the kubernetes.yml flavor the app uses a Deployment, there is an Ingress, etc. Whereas in the openshift.yml variant the app is deployed as a DeploymentConfig and there are lots of Route objects.

I assume if we wanted to follow that pattern (which is what setting -Dquarkus.kubernetes.deployment-target=kubernetes vs -Dquarkus.kubernetes.deployment-target=openshift does), then it looks like we'd need to go the separate chart-per-deployment-target avenue?

I think maybe the terminology is tripping me up a bit, so

having a single chart per deployment type with multiple values files is the right way to go.

when you say that you meant like a chart for rest-villains, with an values-openshift and values-kubernetes file, or did you mean a rest-villains chart for Openshift , a rest-villains chart for kubernetes and then values-java17 ?

When I say

having a single chart per deployment type with multiple values files is the right way to go.

I would envision a single chart with multiple values files, like values-openshift.yml, values-kubernetes.yml, etc.

But it seems like we can't go that route because the templates that are inside the chart will also change based on the deployment target.

For example, if we generated a chart for rest-villains for openshift there would be a deploymentconfig.yml, route.yaml, deployment.yaml, and a bunch of others.

But if we generated a chart for rest-villains for kubernetes, there would be no deploymentconfig.yaml. There would be a deployment.yaml, but the application itself would be a Deployment resource in there. There would be no route.yaml. Instead there would be an Ingress.yaml.

Similarly, if we generated a chart for knative, the app would be deployed as a knative service, which would have a different structure and be in a different yaml.

Does that make sense? Given that, I think we have to generate a different chart per deployment target, like rest-villains-openshift, rest-villains-kubernetes, rest-villains-knative, etc.

Maybe if we go that route then we can still have multiple values files, but the values files would be the java version? So inside rest-villains-kubernetes you'd have a values-java11.yml, values-java17.yml, and values-native.yml. I think that might make sense?

yes, that does make sense. that will still work, but each value file will just have different flags enabled no? so in this example : https://quarkiverse.github.io/quarkiverse-docs/quarkus-helm/dev/index.html#conditionally-enable-disable-resources

couldn't you just have :

app:
       ingress:
           enabled: false

be the default for values-openshift or something like that? the automation surrounding that will just be the piece that's difficult to solve

but each value file will just have different flags enabled no?

The conditionally enabling-or-disabling resources are used when you install a chart. What I'm saying is that at build time, the kubernetes version will have an ingress.yaml but the openshift version will not even have that file in its chart. Similar for other resource types.

The "structure" of what the chart looks like I think is different based on the deployment target. Just take a look at https://github.com/quarkusio/quarkus-super-heroes/blob/main/rest-villains/deploy/k8s/java17-openshift.yml vs https://github.com/quarkusio/quarkus-super-heroes/blob/main/rest-villains/deploy/k8s/java17-kubernetes.yml vs https://github.com/quarkusio/quarkus-super-heroes/blob/main/rest-villains/deploy/k8s/java17-knative.yml for example. There are very different resource types in there.

It sounds like what you're saying is that there would be a single chart that would be like a conglomerate chart that had all resources for every deployment type, but then use the conditional stuff to control what actually gets deployed when you install the chart? That might start to get kind of messy and involve many different conditionals, since the conditionals would have to know which resource type the resource should be?

In the rest-villains example you'd have the rest-villains app as a knative service (not sure what the name of the template file would be - service.yml?), you'd have it in deployment.yml, and in deploymentconfig.yml. You'd have to know how to conditionally enable/disable lots of different resources by name and type, which could start to get ugly. You couldn't just globally disable the Deployment type if you were installing on openshift because there would be other resources in deployment.yml that you actually did want to install. I'm not sure the conditionals are that advanced. And even if they were, would we want them to be that complicated?

On the other hand, if we have separate charts per deployment type, each chart would correctly define the structure of the app for that deployment target. Then really the only things that would change would be the image to use (java 11/17/native) and the resource requests/limits, and some other labels/annotations. The structure of the application wouldn't change.

Feel free to continue disagreeing with me though :) And if what I'm saying isn't clicking maybe I can work up a quick example to demonstrate.

Just as a quick illustration I put this together. The values in the values files are gibberish and mostly meaningless, but more looking at it from structure. I haven't verified much in this chart. There's lots and lots of gibberish in the values files and I'm not sure where its coming from. This is merely to illustrate structure.

As you can see the actual templates (& their contents) are significantly different for the openshift vs kubernetes vs knative deployment types.

Download the zip that has all these in it

@edeandrea just by chance I reached the same conclusion as you did: each service might have a chart per "deployment" (kubernetes, openshift...), and a values file per "kind" (native, java11, java17).

I started my own fork with this idea at ingmarfjolla#1

Indeed, this is still far from being complete, but it´s a start.

Other things (like the resource limits/requests), we input as -D params when we generate the resources, that way they aren't hard-coded in application.properties.

I also opted by adding the limits/requests in the application.properties file.
The reason is that we need to add a "default" entry in the deployment, so Helm can overwrite it with the appropriate entry from the values file.
If we use -D to generate the default entry, then we are somehow saying that the script is the point where we decide te limits/resources. But with Helm, we are moving this decission to the values file (that is, the moment we install the chart).
So IMHO it is sane to put the limits in the application.properties as a "sane default", and leave the door open for different deployment approaches (say, k8s or Helm) to decide the value at deployment time.

Feel free to continue disagreeing with me though :) And if what I'm saying isn't clicking maybe I can work up a quick example to demonstrate.

Actually, this resonates a lot with my current experience. I tried to walk the way of having a single uber-chart, but it was really a mess.
I opted by chart-per-target + value-per-kind.

So IMHO it is sane to put the limits in the application.properties as a "sane default", and leave the door open for different deployment approaches (say, k8s or Helm) to decide the value at deployment time.

So what would the sane default be, since it varies greatly depending on the type of application (JVM vs native). For the k8s resources generation we make that determination in the generate-k8s-resources.sh script, since at that point in time we know exactly what type we need. Just having those in application.properties may or may not work for some people depending on their use case.

Superceded by #291