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

Konflux build 2.15 #1522

Merged
merged 9 commits into from
Jan 23, 2025

Conversation

tkan145
Copy link
Contributor

@tkan145 tkan145 commented Jan 20, 2025

What

Port #1514 so we can build 3scale 2.15 on Konflux. This does not include the jaeger-client-cpp rpm so we still need to figure out what is the best way to build it

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]>
@tkan145 tkan145 requested a review from a team as a code owner January 20, 2025 06:42
@tkan145 tkan145 requested a review from gsaslis January 20, 2025 06:42
Copy link
Member

@gsaslis gsaslis left a comment

Choose a reason for hiding this comment

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

@tkan145 One concern with this PR is whether merging this will affect the 2.15 build on the Continuous Productization as a Service (CPaaS).

It looks like only new files are being added here, so the build will probably not break, as we're not changing files that the product build depends on.

On the other hand, we'll now have these files in two places - one for CPaaS (in the midstream repo) and one for Konflux (here). This is something to keep in mind, if we need to make changes in either place, as there's no single source of truth and we'll have to port the changes across these 2 streams until we know we can ship 2.15 on konflux.

In all, the changes themselves look good to merge. And after merge, we should verify that both the CPaaS product build still works (taking care to not include it) in 2.15.2.

What I would recommend is that we change the target branch, so we have one for cpaas and one for konflux. I'll leave the branch name up to you and all we'd then need is to point the konflux build to the new branch. Once all is done, we can retire this other branch (or the cpaas one).

Does that make sense?

@tkan145 tkan145 changed the base branch from 3scale-2.15-stable to 3scale-2.15-stable-konflux January 23, 2025 04:34
@tkan145
Copy link
Contributor Author

tkan145 commented Jan 23, 2025

@gsaslis yes I understand your concern. Updated to use new branch

@tkan145 tkan145 merged commit 5cc4d33 into 3scale:3scale-2.15-stable-konflux Jan 23, 2025
14 checks passed
@tkan145 tkan145 deleted the konflux_build_2.15 branch January 23, 2025 04:36
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

Successfully merging this pull request may close these issues.

2 participants