Inline RPM dependencies in container image build #1514
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Supersedes #1510 |
|
@tkan145 ok, this now looks like it has more or less the same failures as master. Could you please take another look ? (I've basically just created a new branch and created a new PR from that and deleted the old branch) |
|
@gsaslis luarocks commit is merged. Can you do another rebase? 🤞 |
Signed-off-by: Yorgos Saslis <[email protected]>
Signed-off-by: Yorgos Saslis <[email protected]>
Signed-off-by: Yorgos Saslis <[email protected]>
Signed-off-by: Yorgos Saslis <[email protected]>
the RPM builds fine without this package. Finding the `rpmdevtools` package is a problem anyway because it's part of the entitled content - and not available in UBI repos. Signed-off-by: Yorgos Saslis <[email protected]>
- all RPM builds happen in a multi-stage build - this is just the first version of the Containerfile that I was able to get a passing build on, with `podman build -f Containerfile .` - there are still several improvements to be implemented in the multi-stage build, to reduce some duplication. The previous build system didn't allow reusing previous build stages in subsequent ones (i.e. it didn't allow `FROM rpm-builder AS openresty`, etc.), so that still needs some clearing up. - we should probably only keep one of `Containerfile` and `Dockerfile`. I just didn't overwrite the `Dockerfile` in one go to allow for a better chance of review. Open issue: the jaegertracing-cpp-client RPM is currently commented out. That hadn't previously been ported to its own build stage and I don't know if it's worth doing that now. Hopefully, support for opentracing can be dropped in the next release (considering we now have support for opentelemetry). Signed-off-by: Yorgos Saslis <[email protected]>
Signed-off-by: Yorgos Saslis <[email protected]>
Signed-off-by: Yorgos Saslis <[email protected]>
gsaslis
force-pushed
the
better_build
branch
from
1b7b6a2 to
636b2a2
Compare
|
@tkan145 Looking better! just busted and prove failing now. |
|
nice! all green! ✅ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR introduces a radically different - and much more wholistic - approach to producing the apicast container image.
The current build relies on RPM packages that must have been built and pushed in a repository, so they can be pulled and installed into this image. However, this poses several issues with regards to how those RPMs are built, how easy they are to update, etc. etc.
The approach here is not entirely new - it comes from the product build, which has already adopted this multi-stage approach in building RPMs within the container image and then installing them in the final produced image.
It should be noted that - eventually - we could remove the RPM packaging altogether and essentially convert the RPM spec files into Containerfile instructions. This would further simplify the build steps. In this PR, we are only taking one first step towards that.
Some notes:
podman build -f Containerfile .FROM rpm-builder AS openresty, etc.), so that still needs some clearing up.ContainerfileandDockerfile. I just didn't overwrite theDockerfilein one go to allow for a better chance of review.Open issue: the jaegertracing-cpp-client RPM is currently commented out. That hadn't previously been ported to its own build stage and I don't know if it's worth doing that now. Luckily, support for opentracing is planned to be dropped in the next 3scale 2.16 release (considering we now have support for opentelemetry), so those lines could probably be removed altogether.
@tkan145 I am creating this as a draft PR, so we can slowly start reviewing it.
(Note: while I was able to build a container image, I haven’t tested it in any way yet. I’d also like your help in figuring out if there is anything important missing in the new Containerfile from the existing Dockerfile ).