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

chore(build): Use Camel K runtime Maven structural logging module #3376

Merged
merged 2 commits into from
Aug 24, 2022

Conversation

christophd
Copy link
Contributor

  • Load and extract structural logging artifacts from Camel K runtime module
  • Use logback configuration provided by this runtime module
  • Add logging artifacts during Dockerfile image build

IMPORTANT: This PR must wait for Camel K runtime module to be merged and released to Maven central (apache/camel-k-runtime#833) otherwise build scripts will fail because of the missing module.

As a workaround users can explicitly build the Camel K runtime modules locally and reference the SNAPSHOT version as follows:

make RUNTIME_VERSION=1.14.0-SNAPSHOT images

or use the local Camel K runtime repository

make CAMEL_K_RUNTIME_DIR=$(pwd)/../camel-k-runtime images

This will build the Camel K image with the SNAPSHOT runtime modules and its Maven structural logging settings.

Release Note

Chore(build): Use Camel K runtime Maven structural logging module

@christophd
Copy link
Contributor Author

FYI @squakez

Copy link
Contributor

@squakez squakez left a comment

Choose a reason for hiding this comment

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

Nice work!

@squakez squakez added the status/wip Work in progress label Jun 20, 2022
@squakez squakez added this to the 1.10.0 milestone Jun 20, 2022
@tadayosi tadayosi mentioned this pull request Aug 22, 2022
@oscerd
Copy link
Contributor

oscerd commented Aug 23, 2022

@christophd can we rebase this? we are now using the next staging release for camel-k-runtime.

@christophd
Copy link
Contributor Author

@oscerd sure will do right now

@christophd
Copy link
Contributor Author

@oscerd I had to add another commit to account for the Camel K staging repository when loading the Camel K runtime Maven logging overlay artifacts

Tried to also retain the functionality to specify a local Camel K runtime codebase to use this instead of loading from the internet.

@squakez @tadayosi can you please also have another look at the changes made?

@oscerd
Copy link
Contributor

oscerd commented Aug 23, 2022

How do we deal with this if we'll have to release artifacts together? Once we release the runtime the staging repo will go away.. while this is managed for camel k release process, I'm not sure this will be fine for this feature. It could make it unusable

@christophd
Copy link
Contributor Author

not sure but I think we need Camel K runtime to be fully released to maven central before kicking off a release for Camel K right? So we can assume the Camel K runtime bits to always be available on central?

The staging repo support is only for temporary builds like in this PR and should be given as an argument to the Make file call explicitly?

@oscerd
Copy link
Contributor

oscerd commented Aug 23, 2022

No, the release is done in one shot with the runtime still in staging. If the code remain there during the release we'll have a reference to a repository that doesn't exist. So we should remove it before releasing.

@oscerd
Copy link
Contributor

oscerd commented Aug 23, 2022

So my suggestion is to remove the staging reference for this specific feature before the release start.

@squakez
Copy link
Contributor

squakez commented Aug 23, 2022

I think we should pick the artifacts from staging, when the reference is present, otherwise it must take it from central. Or am I missing something?

@oscerd
Copy link
Contributor

oscerd commented Aug 23, 2022

During the release, in the beginning there will be some steps using the staging repo, but before the end of the release once the catalog has been generated and the resources generated, the reference to the staging repository will be removed, so in the end there won't any reference to the staging repository and the final artifacts/image etc. The release will take for granted the promotion of the artifacts coming from staging repository to central. In this case, we'll still have reference to the staging repo in the final artifacts and we shouldn't. Hope this makes thing a bit more clear. We are not in the situation in which: k-runtime will be released to central before starting the release of camel-k, the release vote will be one for all the components (camel-kamelets, camel-k-runtime and camel-k): for camel-kamelets we are pointing a tag, so no problem, for k-runtime we are managaing the problem through Makefile subsequent steps, for this overlay feature we are not managing the switching from staging to central at all.

@christophd
Copy link
Contributor Author

@oscerd so it basically follows the steps described in here, right?: https://github.com/apache/camel-k/blob/main/release.adoc

@oscerd
Copy link
Contributor

oscerd commented Aug 23, 2022

Yes

@christophd
Copy link
Contributor Author

@oscerd so in this process when calling make release-staging the Camel K runtime staging repository is still available, right? I think this is all we need to reference the Maven overlay artifacts in order to add these to the resulting Camel K Docker image that gets pushed to the staging account.

Would that be ok?

@oscerd
Copy link
Contributor

oscerd commented Aug 23, 2022

The staging repository si be available until the release vote ends, so it will be there until the release passed. But the reference will be removed in next steps of releasing

@christophd
Copy link
Contributor Author

From my understanding the maven-overlay is only used to build the Docker image. Once this is done via make release-staging and the reference to staging gets removed the release process should not build the image once again. I think we are fine with that are we?

@oscerd
Copy link
Contributor

oscerd commented Aug 24, 2022

I think so.

@tadayosi tadayosi removed the status/wip Work in progress label Aug 24, 2022
Copy link
Member

@tadayosi tadayosi left a comment

Choose a reason for hiding this comment

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

Only small cosmetic issues.

rootdir=$(realpath ${location}/../)

if [ "$#" -lt 2 ]; then
echo "usage: $0 <output directory> <Camel K runtime version> [<staging repository>] [<local Camel K runtime project directory>]"
Copy link
Member

Choose a reason for hiding this comment

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

it's a small issue but the usage message doesn't match the actual code. Here <output directory> comes first but actually it should be $2 arg.

Maybe I guess the usage message is the intended one and the code output_dir=$2 is wrong? Is it really meant to be :

output_dir=$1
runtime_version=$2

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, the usage message is wrong. many thanks!


if [ -n "$staging_repo" ]; then
if [[ $staging_repo == http:* ]] || [[ $staging_repo == https:* ]]; then
echo "<settings xmlns=\"http://maven.apache.org/SETTINGS/1.0.0\" xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\"" \
Copy link
Member

Choose a reason for hiding this comment

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

I'd use bash heredocument to avoid \ at the end of lines but maybe it's a matter of taste.

cat <<EOF > ${rootdir}/settings.xml
...
EOF

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point. thx!

@tadayosi
Copy link
Member

@oscerd Not this time but IMO in the future we should separate the releasing process of Camel K runtime from the rest of Camel K and release the runtime more frequently because of #3481 to let the runtime catch up with latest LTS releases of Camel / Camel Quarkus. We can vote each release separately.

If I'm not mistaken architecturally Camel K and Camel K runtime are designed to be decoupled so I think it's even encouraged to do so.

@oscerd
Copy link
Contributor

oscerd commented Aug 24, 2022

We can do that, but instead of taking 72 hours to release, it would take up to one week if everything goes well, if not, maybe more. I don't have particular point of view on this kind of stuff, but camel-k-runtime doesn't have any reason to live without camel-k, so, releasing and voting it as standalone project doesn't make sense.

@oscerd
Copy link
Contributor

oscerd commented Aug 24, 2022

@tadayosi
Copy link
Member

but camel-k-runtime doesn't have any reason to live without camel-k

Yes, right, but my point is that if we release camel-k-runtime separately users could serve themselves for upgrading the runtime to a more secure one if they are concerned with any CVEs. Right now they have to wait for a main Camel K release to receive the latest patches for the runtime.

@oscerd
Copy link
Contributor

oscerd commented Aug 24, 2022

In the last release of camel-k-runtime we just released because of camel-k, but yeah, if there is security concern or CVE to fix we could release it alone.

- Load and extract structural logging artifacts from Camel K runtime module
- Use logback configuration provided by this runtime module
- Add logging artifacts during Dockerfile image build
@christophd christophd changed the title [WIP] chore(build): Use Camel K runtime Maven structural logging module chore(build): Use Camel K runtime Maven structural logging module Aug 24, 2022
@christophd
Copy link
Contributor Author

removed the WIP marker as this is ready to be merged IMO

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.

4 participants