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

k8s:helm generated chart tar archive contains reference to tar archive #2622

Closed
rohanKanojia opened this issue Feb 5, 2024 · 9 comments · Fixed by #2643
Closed

k8s:helm generated chart tar archive contains reference to tar archive #2622

rohanKanojia opened this issue Feb 5, 2024 · 9 comments · Fixed by #2643
Assignees
Labels
bug Something isn't working
Milestone

Comments

@rohanKanojia
Copy link
Member

rohanKanojia commented Feb 5, 2024

Describe the bug

Originally posted by @adriannowak in Gitter thread

I am generating basic helm image from spring boot application, but unexpectedly I have tgz file in my helm tgz
what is interesting that file exist only in helm that is in /target/ file, that one located in ~/.m2/repository looks ok

when packing resource into Helm chart using k8s:helm goal, it generates a tar.gz file in target folder:

ls target/jkube/helm/outgoing-backup-service/kubernetes/
Chart.yaml  license.txt  outgoing-backup-service-0.0.2-SNAPSHOT.tar.gz  readme.md  templates  values.yaml

when extracting this archive file it contains reference to another empty tar archive:
obraz

Eclipse JKube version

SNAPSHOT

Component

Kubernetes Maven Plugin

Apache Maven version

None

Gradle version

None

Steps to reproduce

  1. Run mvn k8s:resource k8s:helm
  2. Extract the generated archive present in target/jkube/helm/$projectName/kubernetes/$projectName-$version.tar.gz

Expected behavior

generated helm chart archive should not contain reference to another tar.gz file

Runtime

Kubernetes (vanilla)

Kubernetes API Server version

1.25.3

Environment

Linux

Eclipse JKube Logs

No response

Sample Reproducer Project

No response

Additional context

No response

@rohanKanojia rohanKanojia added the bug Something isn't working label Feb 5, 2024
@rohanKanojia
Copy link
Member Author

@l3002 : This is not a good first issue.

In order to reproduce you need to try it out in some demo project (for example this). You are running this goal directly in main repository.

@l3002
Copy link
Contributor

l3002 commented Feb 8, 2024

@rohanKanojia, Yeah, I realized that after a few minutes of posting that comment, that's why I deleted it.

@manusa manusa added this to the 1.16.0 milestone Feb 8, 2024
@rohanKanojia
Copy link
Member Author

This does not happen on the first run. It only happens on subsequent runs when target/ directory already contains the generated chart from previous execution.

When I run mvn clean package k8s:resource k8s:helm, I can see tarball contains chart files as expected:
Screenshot_20240208_233844

Running mvn k8s:resource k8s:helm without cleaning target directory reproduces the issue:
Screenshot_20240208_234026

@rohanKanojia
Copy link
Member Author

Adding a Files.deleteIfExists(tarballFile.toPath()); before creating tarball here should fix the problem:
https://github.com/eclipse/jkube/blob/a616fe68e3d348f5f5c4f94844c50ba3ecb12ede/jkube-kit/helm/src/main/java/org/eclipse/jkube/kit/resource/helm/HelmService.java#L127

@manusa
Copy link
Member

manusa commented Feb 9, 2024

But the question is how is the tarball added to the list of paths/files to be included in the archive.
AFAIR the logic just considered a list of files that were computed previously.

@rohanKanojia
Copy link
Member Author

List of files to be packed in tarball is prepared via doing listFilesAndDirsRecursivelyInDirectory(outputDir) in outputDir

https://github.com/eclipse/jkube/blob/a616fe68e3d348f5f5c4f94844c50ba3ecb12ede/jkube-kit/helm/src/main/java/org/eclipse/jkube/kit/resource/helm/HelmService.java#L128

@manusa
Copy link
Member

manusa commented Feb 9, 2024

So the problem is that at that point the outputDir contains the previous tar file, right?

What's outputDir in this context?

Does it make sense that the tar file is in this outputDir?

@rohanKanojia
Copy link
Member Author

rohanKanojia commented Feb 9, 2024

outputDir is the location where helm chart is being generated. build/jkube/helm/projectName/kubernetes

In #2036 we had changed the location where tar file would be generated from target / build to build/jkube/helm/projectName/kubernetes to avoid collision between K8s and OpenShift helm chart
tarballs.

@manusa
Copy link
Member

manusa commented Feb 9, 2024

In #2036 we had changed the location where tar file would be generated from target / build to build/jkube/helm/projectName/kubernetes

So if we change again the output location for the tarball we'll incur in a new breaking change then.

We can do the suggested change but being more explicit:

  • Store the result of FileUtil.listFilesAndDirsRecursivelyInDirectory(outputDir) in a variable.
  • Filter out the maybe existing tarBall file from the list of returned files
  • Crate a new test for this specific scenario
  • Add a comment explaining why this is needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants