Skip to content
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

helm : enhance chart generation #113

Closed
survivant opened this issue Mar 19, 2020 · 13 comments
Closed

helm : enhance chart generation #113

survivant opened this issue Mar 19, 2020 · 13 comments
Assignees

Comments

@survivant
Copy link
Contributor

I would like to have the option to choose where I want to output charts to be generated. I'll like prefer to have it in ./charts folder instead of target... like that I could add it to my source code.

something like :

<plugin>
        <groupId>org.eclipse.jkube</groupId>
        <artifactId>k8s-maven-plugin</artifactId>
        <version>0.2.0</version>
         <configuration>
              <outputDir>./charts</outputDir>
          </configuration>
      </plugin>

for now I'm using this 

(your helm mojo is broken for now, that's why I'm using fabric8)
mvn fabric8:helm -D=fabric8.helm.outputDir=./charts

the file Chart.yaml could be enhanced like that (it contains more useful informations and display nice in chartmuseum UI too :

apiVersion: v1
appVersion: "1.0"
description: A Helm TEST chart for Kubernetes
name: test-k8s-helm
version: 0.1.1
maintainers: [email protected]
home: http://url-du-projet
icon: http://littleicon.png
sources:
  - githubpath

all that information could be in configuration in the pom.xml

if the folder is present.. just override the files without deleted the content.

@rohanKanojia
Copy link
Member

Thanks for your bug report. @manusa is working on fixing helm support. It would be fixed before the next release

@rohanKanojia
Copy link
Member

related to #102

@manusa manusa self-assigned this Mar 20, 2020
@manusa
Copy link
Member

manusa commented Mar 20, 2020

Implementation fix includes options to specify both outputDir where all chart files will be generated, and tarballOutputDir where the tar archive will be generated (independent from outputDir)

Both options can be setup within pom.xml configuration or overridden with properties (e.g. jkube.helm.outputDir)

Regarding outputDir behavior, generated charts will be output by default in target/$chartName/, so for each helmType (one of kubernetes or openshift) charts and files will be output in $outputDir/$helmType. As said the value of outputDir can be overridden, but every generated file will always be nested under $outputDir/kubernetes or $outputDir/openshift.

Regarding tarballOutputDir will default to target.

So implementation differs a little with that of FMP, hope this is OK for you. Please if you find something wrong with this or new behavior is of great inconvenience post a reply with your thoughts.

@manusa
Copy link
Member

manusa commented Mar 20, 2020

if the folder is present.. just override the files without deleted the content.

Seems fair for this to be the default behavior, as mvn clean should take care of cleaning. I'll modify current behavior

@manusa
Copy link
Member

manusa commented Mar 30, 2020

Hi @survivant

Last Friday we released 1.0.0-alpha-1 with most of the suggested changes.

You can check configuration examples here and updated documentation here (until we have the final release).

Could you please check if the provided solution works for you, or if there's something missing or that you would like to change?

@survivant
Copy link
Contributor Author

thanks really nice. I just found something last week. If we don't respect the format "\d.\d.\d" like 1.0.0 for the version example : 1.0.0-SNAPSHOT, when you do helm search ... Helm (3.x) is not expecting that and won't show it in the result. But I love the commit you did.. thanks a lot

@manusa
Copy link
Member

manusa commented Mar 30, 2020

Are you referring to the quickstart? or to some point in the documentation?

We can change the example to always override the version number and provide one using the suggested format. or hard-code the version in the documentation.

In any case, 1.0.0-SNAPSHOT is a valid Semver 2 version number. There's probably an issue in helm cli.

@survivant
Copy link
Contributor Author

I agree that's a bug in helm cli. but there is over 900 issues, so I'm not sure if I add one more it will make a difference.

@survivant
Copy link
Contributor Author

here my tests :

mvn k8s:build generate a artifcat in the root folder : targetquarkus-k8s-hello-1.0.0-SNAPSHOT.jar there is a "/" missing. should be : target/quarkus-k8s-hello-1.0.0-SNAPSHOT.jar

for : ./charts

it generate the charts in

./charts/kubernetes instead

should we add this also in the chart

apiVersion: v1
appVersion: "1.0" // could it be the project version like 1.0.0-SNAPSHOT ?

@manusa
Copy link
Member

manusa commented Mar 30, 2020

There's an active issue fabric8io/fabric8-maven-plugin#1516 (will be ported to JKube once FMP gets deprecated > Eclipse JKube 1.0.0 released) where we want to provide support for Helm v3 (or latest)

So the generated yaml, as of now, should be compatible with this spec (https://github.com/helm/helm/blob/release-2.0/docs/charts.md)

Regarding your output issues, I don't understand what's the issue.
I've tried the following config:

<helm>
  <outputDir>./charts</outputDir>
  <tarballOutputDir>./charts</tarballOutputDir>
</helm>

Which generates the following:
image

If I use this:

<helm>
  <outputDir>./charts</outputDir>
  <tarballOutputDir>./</tarballOutputDir>
</helm>

I get this instead:
image

Are you running this on windows BTW?

@survivant
Copy link
Contributor Author

yes I'm in Windows.

there was 2 issues in my comment.

#1- k8s:build generated the jar file in the wrong folder : missing / to get target/quarkus-k8s-hello-1.0.0-SNAPSHOT.jar

and the second was that the output of the charts should be

./charts/*.yaml

instead of ./charts/kubernetes/*.yalm ? I don't see the point of puting the type in that folder. I suppose the folder "kubernetes" is because the type is kubernetes, if I put openshift in pom.xml the output will be

./charts/openshfift/*.yaml

there is no point of using that information. The output will be the same for both of them. maybe if you use the default values, but if you override the value in the pom.xml, you should use that foldr ?

@manusa
Copy link
Member

manusa commented Mar 30, 2020

I understand now.

Regarding #1 I've opened #131 which is quite critical.

Regarding #2 This is the expected behavior, because you can generate charts for both clusters <type>kubernetes,openshift</type> (provided that resources exist for both).

So for 99% of the use cases it doesn't make sense to nest the chart files under a kubernetes directory.
But there is still this edge-case where a user will create charts for both clusters thus needs to have charts nested under directories to prevent files to be overwritten.

The only way I see to omit the cluster type subdirectory is to add an extra-flag (e.g. outputDirectoryOmitTypeClassifier defaults to false) but I find it very ugly. Is this something you really need?

@survivant
Copy link
Contributor Author

ok, I see your point. I can live with that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants