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

Consolidate image & executable build into exe-in-image #582

Closed
Jongy opened this issue Nov 6, 2022 · 3 comments
Closed

Consolidate image & executable build into exe-in-image #582

Jongy opened this issue Nov 6, 2022 · 3 comments
Assignees
Labels
build Image/executable build defined-and-prioritized Tickets that have fully defined the desired outcome & are prioritized to be developed.

Comments

@Jongy
Copy link
Contributor

Jongy commented Nov 6, 2022

We have 2 Dockerfiles (Dockerfile and Dockerfile.pyi) which result in an image and in an exe. Instead of maintaining 2 builds, we can remove the image build, and instead the image Dockerfile would become:

FROM scratch # or an almost-empty image
COPY build/.../gprofiler /gprofiler
ENTRYPOINT /gprofiler

Upsides:

  1. Only one build to maintain
  2. Smaller image size (100mb ~ as opposed to 300-400 now)

Downsides:

  1. Image build is faster for local testing & execution, building the exe takes longer so this makes development cycles a bit longer. We can support local execution with python -m gprofiler (haven't tried that in a long while).
@Jongy Jongy added the build Image/executable build label Nov 6, 2022
@Jongy
Copy link
Contributor Author

Jongy commented Nov 18, 2022

I think we'll go for this after #597 is merged and we're using a newer Python in the exe, I don't want to force all releases to use the Python 3.6 we have in the exe which has some bugs that we encounter in flaky tests.

Jongy added a commit that referenced this issue Dec 15, 2022
Follows intel/granulate-utils#107 to do the same for gProfiler.
Now the container build uses 3.8, so that's the minimal supported version.
If we ugprade the container to Ubuntu 22.04 or implement #582, the minimal supported version will be 3.10 :)
@mpozniak95 mpozniak95 self-assigned this Apr 14, 2023
@Jongy
Copy link
Contributor Author

Jongy commented Apr 14, 2023

Additional elaboration:

  1. Ideally, the base image should be empty (since staticx-ed binaries have no dependencies). If possible - the best would be an EMPTY image with the gprofiler binary in /. In Dokcer that's FROM scratch as the base of the Dockerfile.
  2. Personally, I use the container build when debugging/prototyping because it's faster to build than the pyinstaller+staticx. Once we remove the standard container build, I won't have that option anymore, and so, I'd like a replacement. One option is to be able to build the exe while skipping the staticx - this is considerably faster, while not enough for all profilers (PyPerf is not built statically at the moment, but I can live with that and use the full build when I want to test PyPerf stuff). Any other idea on how to remain with fast (less than 5 seconds) time for the build are acceptable 🙏
  3. ENV GPROFILER_IN_CONTAINER=1 should be kept.
  4. The build_..._container.sh scripts should be simple now and not contain any reference to other base images. In other words, only build_..._executable.sh should contain references to the base images CENTOS_VERSION etc.
  5. CI: Currently in testing CI we have 2 separate workflows, one that builds the container then runs container tests, and one that builds the executable then runs executable tests. I suggest that we structure them as one - first build the exe (build-executable job), then the test-executable which has needs: build-executable. We'll merge build-container into the same workflow, it will also have needs: build-executable, and it will build the image using the executable built earlier, then continue to the test job (which can be renamed to test-container). The same should be done for the deploy-* jobs. Eventually we should remain with 3 workflows - linters, to which the Python linting should move, build-on-windows which is disconnected from the others, and build-test-deploy which does the build, testing & deploying (if tag) for both x86_64 and aarch64.

@Jongy
Copy link
Contributor Author

Jongy commented May 8, 2023

More notes:

  • @mpozniak95 correctly noted that gProfiler uses shell - /bin/sh - for running commands, e.g in pgrep_maps. So we decided to go with alpine as a base image, not scratch.
  • As for the equivalent of copy_resources_from_image.sh, when gProfiler exe runs, it unpacks all resources. I suggest we can add a subcommand gprofiler extract-resources (like upload-file) which can exit brutally (for example sys._exit()) in a way that PyInstaller does not remove the resources. Alternatively, we can check how to prevent PyInstaller from removing the artifacts it extracts, then extract-resources can do that.

@Jongy Jongy added the defined-and-prioritized Tickets that have fully defined the desired outcome & are prioritized to be developed. label Jun 22, 2023
@Jongy Jongy closed this as completed in c7cc4a3 Aug 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Image/executable build defined-and-prioritized Tickets that have fully defined the desired outcome & are prioritized to be developed.
Projects
None yet
Development

No branches or pull requests

2 participants