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

Proposal to add libz to the tiny runtime image #79

Merged
merged 3 commits into from
Jul 12, 2021
Merged

Conversation

dmikusa
Copy link
Contributor

@dmikusa dmikusa commented Jun 14, 2021

Summary

At the moment, Java applications do not run on the tiny builder. It is possible to build them, however, if you try to run one you'll end up with an error libz.so.1: cannot open shared object file: No such file or directory.

Use Cases

  1. To support running Java apps on tiny
  2. Java represents a large part of the user base
  3. This can cut off 70M for Java images, which is substantial for many Java apps

Checklist

  • I have viewed, signed, and submitted the Contributor License Agreement.
  • I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
  • I have added an integration test, if necessary.
  • I have reviewed the styleguide for guidance on my code quality.
  • I'm happy with the commit history on this PR (I have rebased/squashed as needed).

@dmikusa dmikusa requested a review from a team as a code owner June 14, 2021 02:57
@ekcasey
Copy link
Member

ekcasey commented Jun 22, 2021

At the last working group @ryanmoran suggested that this should be a stacks sub-team RFC, which seems reasonable to me.

@dmikusa dmikusa requested a review from a team as a code owner June 22, 2021 15:11
@dmikusa
Copy link
Contributor Author

dmikusa commented Jun 22, 2021

I have moved this to the stacks subteam. @sophiewigmore @brayanhenao, your thoughts are appreciated.

Thanks

@sophiewigmore
Copy link
Member

I think the main issue I am struggling with here is is adding something to Tiny that isn't needed by ALL of our Paketo Buildpacks, so that there's a superfluous package hanging out in the run image in some use cases.

That being said, the fact that the addition is to enable Java, which makes up a non-trivial amount of Paketo use cases, as well as the small package size are compelling benefits. These benefits outweigh that one negative for me, so I am good with this.

@brayanhenao
Copy link
Member

@dmikusa-pivotal I did some tests adding the zlib1g to the tiny stack. The run image ended with a +0.1MB size.

Screen Shot 2021-06-22 at 5 22 03 PM

I agree with @sophiewigmore, It's worth it this this small size addition to the image compared to the fact that will enable the use of Tiny in Java.

@dmikusa
Copy link
Contributor Author

dmikusa commented Jun 23, 2021

Awesome, thanks to everyone for the feedback!

@fg-j
Copy link

fg-j commented Jun 23, 2021

Is this addition of this single package all that is required to run all Java workloads on Tiny?

@dmikusa
Copy link
Contributor Author

dmikusa commented Jun 23, 2021

@fg-j - Yes, that's my understanding. I haven't actually tried it though. If there's a desire to try before we make the change & someone can provide a modified tiny image (or instructions how to add it), I can do that & report back.

@fg-j
Copy link

fg-j commented Jun 23, 2021

Personally, I'd love to see an exploration of that, or some prior art or documentation that indicates this package is all we'd need for Java on Tiny, since it's so painful to remove a package from a stack once added, but I'll defer to the Stacks maintainers.

@dmikusa
Copy link
Contributor Author

dmikusa commented Jun 23, 2021

@brayanhenao Is the test image you created a tiny builder? Wondering if you could share & I could test this out? Thanks

@brayanhenao
Copy link
Member

@dmikusa-pivotal

Steps to reproduce the addition of a package to tiny:

  1. Clone https://github.com/paketo-buildpacks/stacks.
  2. Go to <path-to-clone>/stacks/tiny/dockerfile/run.
  3. Edit the file packagelist, adding the library zlib1g at the end, preserving the empty line at the end of the file.
  4. Go to <path-to-clone>/stacks/create-stack.
  5. Run the command go build.
  6. Execute the binary with the following parameters: ./create-stack
  • --build-destination <name-of-the-tiny-build-docker-image> : Is the name that the program will give to the build image and will be stored local Daemon.
  • --run-destination <name-of-the-tiny-run-docker-image> : Is the name that the program will give to the run image and will be stored local Daemon.
  • --stack tiny : Is the name of the stack, in this case is Tiny.
  • --stacks-dir <full-path-to-stacks-repo-folder> : Is the full path to the stacks folder. Eg: /Users/bhenao/workspace/paketo-buildpacks/stacks
  • --version <version-of-the-image-tag> : Version of the tag. The images will have this tag format image-name:-.

Eg: ./create-stack --build-destination bahctiny-build --run-destination bahctiny-run --stack tiny --stacks-dir /Users/bhenao/workspace/paketo-buildpacks/stacks --version 1

@dmikusa
Copy link
Contributor Author

dmikusa commented Jun 23, 2021

Thanks @brayanhenao

I was able to build the tiny stack image with the additional library, create a builder from it, update the Java buildpacks to allow the tiny stack and package all those up. Then run against the maven sample & it builds. There is a small issue with it running, not the JVM itself. That is OK, but the jvm kill agent that we install doesn't run. There appears to be a different library required for that to work.

Could not find agent library /layers/paketo-buildpacks_bellsoft-liberica/jvmkill/jvmkill-1.16.0-RELEASE.so in absolute path, with error: libgcc_s.so.1: cannot open shared object file: No such file or directory

I am going to do a little more testing to confirm that is the only issue. If that's the case, after talking with @ekcasey, we'd like to continue forward for this. We can find a workaround, such as disabling jvm kill agent, until we can recompile it without that dependency.

I'll update again tomorrow with the results of additional testing.

@dmikusa
Copy link
Contributor Author

dmikusa commented Jun 24, 2021

OK, as suspected. Removing the JVM Kill agent will allow Java apps to run on the tiny stack. I'd like to move forward with this RFC, regardless of the JVM Kill agent issues. I think that's a minor problem, and we can fix it later without any more stack modifications.

Please let me know if there's any additional concerns with this PR.

@sophiewigmore
Copy link
Member

@dmikusa-pivotal I don't know a huge amount about the JVM Kill agent, but I'd like to be 100% sure that there won't be anymore stack modifications needed to enable this before we go through with it. Every java sample app I have played with (not many, to be fair) has pulled in the JVM Kill agent. If it's a big part of the ecosystem it worries me that we need to solution around it.
I'd like to understand more about the proposed workarounds and how viable they are as options, before we add something to the stack (just incase we end up having to backtrack and remove libz, I'd like to avoid that)

@dmikusa
Copy link
Contributor Author

dmikusa commented Jun 25, 2021

At the moment, the JVM Kill agent is always installed. I had to work around this by disabling it in a dev build to confirm that there are no additional libraries or impacts to running on the tiny stack. It's a hack at the moment, but just for the purposes of testing & validation.

With those additional tests, I didn't find any more situations where there were failures. I didn't exhaustively test, but I tested some common scenarios which were all fine. In addition, beyond the JVM itself and the JVM kill agent, I do not believe we're installing any native code. The rest is all Java byte code, which doesn't care about the stack or native libraries.

Given that the JVM kill agent doesn't work at the moment, there's a couple of options moving forward. None of these would require additional libraries installed into the stack.

  1. Add an option to disable the JVM kill agent. I will probably implement this regardless, as most modern JVMs have similar flags that don't require the additional agent. Users might want to go that route anyway since the JVM flags get you 50-75% of what the JVM kill agent does (depending on the features you use in the agent). This would mean that on Tiny, users would just select to disable, or more likely, automatically have the JVM kill agent disabled and have the JVM flags enabled instead.

  2. I'm optimistic that given some time, I can get the JVM kill agent running without the additional library. It doesn't make a lot of sense why it would need libgcc when the Rust library is being compiled with clang. I suppose anything's possible though and I haven't looked into this yet because it'll require a higher degree of effort.

Give the above info, my proposed plan is this:

  1. Get this RFC merged & implemented. This get's the tiny stack available with libz in it.
  2. Update the JVM buildpacks to support tiny (needs buildpack.toml modifications) & implement the feature to make JVM Kill agent optional. We'll do these together so that as soon as the buildpacks permit tiny it'll all just work.
  3. At some point, dig more into the JVM Kill agent and see if we can get it working on tiny. This doesn't need to happen right away because by step Add license 1 #2, we'll have users able to run on the tiny stack, with some very minor caveats, which I think are acceptable (after all you don't have to run on tiny).

That should meet the primary goal which is to get Java workloads able to run on tiny in a reasonable time frame.

@sophiewigmore
Copy link
Member

@dmikusa-pivotal thanks for the context, that plan seems reasonable to me. I didn't know that the JVM kill agent could be emulated with flags. I wonder if the use of those flags produces the same error as the one you saw with the agent? That could be an issue if so.
@brayanhenao do you have any concerns around this?

@dmikusa
Copy link
Contributor Author

dmikusa commented Jun 28, 2021

I didn't know that the JVM kill agent could be emulated with flags.

To be clear, it won't get you 100% of what the JVM kill agent does, maybe 50-75% though, depending on exactly what features of the JVM kill agent you use. It will get you the most used parts though, being able to have a heap dump written on exit.

I wonder if the use of those flags produces the same error as the one you saw with the agent?

No, they should not. They are native to the JVM and require no extra shared libraries.

@ekcasey ekcasey added the team/stacks Stacks Subteam RFC label Jun 29, 2021
* Assigns ID stacks/0002 to libz PR
* Adds README for stacks RFCs
* Removes rfcs subdir from text/stacks for consistency with other teams
* Removes redundant template

Signed-off-by: Emily Casey <ecasey@vmware.com>
@@ -45,6 +45,3 @@ Example:
## Unresolved Questions and Bikeshedding

Is there a clearer/more accurate label name we can use? Technically "stack" refers to a pair of images but we're only trying to represent the packages on a single image.


{{REMOVE THIS SECTION BEFORE RATIFICATION!}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ryanmoran Is this entire section intended to be removed? It seems we haven't been doing that consistently so I wasn't sure if that was still the intent.

@sophiewigmore and @brayanhenao I think we should at least remove these lines for now (cause they look kinda silly) and I can follow up with a PR to remove bikeshedding sections from every RFC if that is desired.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good with that @ekcasey

@sophiewigmore sophiewigmore merged commit 6bf4ef1 into main Jul 12, 2021
@sophiewigmore sophiewigmore deleted the add-libz branch July 12, 2021 21:33
brayanhenao added a commit to paketo-buildpacks/stacks that referenced this pull request Jul 26, 2021
* Add `zlib1g` package to the tiny stack.

Implements [Paketo Stacks RFC 0002](https://github.com/paketo-buildpacks/rfcs/blob/main/text/stacks/0002-add-libz.md) based on the conversation [here](paketo-buildpacks/rfcs#79 (comment)).

* Updated README to include zlib1g

Co-authored-by: Daniel Mikusa <dmikusa@vmware.com>
sophiewigmore pushed a commit to paketo-buildpacks/stacks that referenced this pull request Jul 26, 2021
* Add `zlib1g` package to the tiny stack.

Implements [Paketo Stacks RFC 0002](https://github.com/paketo-buildpacks/rfcs/blob/main/text/stacks/0002-add-libz.md) based on the conversation [here](paketo-buildpacks/rfcs#79 (comment)).

* Updated README to include zlib1g

Co-authored-by: Daniel Mikusa <dmikusa@vmware.com>

Co-authored-by: Daniel Mikusa <dmikusa@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team/stacks Stacks Subteam RFC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants