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

feat: remove the need for a storage #4483

Merged
merged 7 commits into from
Jun 28, 2023
Merged

Conversation

squakez
Copy link
Contributor

@squakez squakez commented Jun 13, 2023

While working on #4025 we have introduced the requirement of a storage as a PersistentVolumeClaim. However, I've realized this is actually nothing else than a Maven Proxy [1]. The presence of a storage add some complexity to the management of the operator. We also expect that any enterprise should already have a Maven Proxy, so, it makes sense to use it as a Maven best practice.

We are still able to use a PVC if the user wants to have this further optimization, and I'm adding some documentation to explain how to do that.

[1] https://camel.apache.org/blog/2023/06/camel-k-maven-proxy/

Release Note

feat: remove the need for a storage

@squakez
Copy link
Contributor Author

squakez commented Jun 14, 2023

@gansheer I had to temporary use root in this PR to be able to build. I'm testing with the RunAsNonRoot solution your proposed in the other PR, however, we still have some problem in the builder pod. The problem is that when we copy contents from Camel K operator into the container image later user by builder pod, the privileges for files are transferred with root. We need to find a way to instruct the builder container image that certain directories have a different set of privileges. This has to be done in Spectrum and S2I, right after we copy those contents.

@squakez squakez force-pushed the feat/remove_storage branch 2 times, most recently from 67c853b to 275f35e Compare June 14, 2023 12:04
@squakez
Copy link
Contributor Author

squakez commented Jun 14, 2023

The fix addressing the concerns in previous comment should be available in container-tools/spectrum#37. Once that got merged and we bump the spectrum dependency we should be able to move back the execution of builder pod with non-root privileges.

@squakez squakez force-pushed the feat/remove_storage branch 2 times, most recently from 157af02 to e1102e0 Compare June 21, 2023 08:05
@squakez squakez linked an issue Jun 21, 2023 that may be closed by this pull request
@squakez squakez marked this pull request as ready for review June 21, 2023 09:05
@squakez squakez force-pushed the feat/remove_storage branch 4 times, most recently from cb1dc29 to fc834ce Compare June 22, 2023 12:16
@squakez squakez added the trigger native test Use this label in PR when you want to trigger Quarkus Native tests label Jun 22, 2023
@squakez squakez force-pushed the feat/remove_storage branch 3 times, most recently from 43b0bfb to 4b44802 Compare June 23, 2023 10:18
@squakez
Copy link
Contributor Author

squakez commented Jun 23, 2023

Failures are likely due to #4511

@squakez squakez force-pushed the feat/remove_storage branch 3 times, most recently from 1947f91 to 50caa5a Compare June 26, 2023 12:44
@squakez squakez removed the trigger native test Use this label in PR when you want to trigger Quarkus Native tests label Jun 26, 2023
@squakez squakez force-pushed the feat/remove_storage branch 2 times, most recently from 6243cd4 to 3d6526b Compare June 26, 2023 15:40
@squakez squakez force-pushed the feat/remove_storage branch from 3d6526b to 4a8072d Compare June 27, 2023 07:04
@squakez squakez force-pushed the feat/remove_storage branch from e790715 to 728f1da Compare June 27, 2023 13:35
@squakez
Copy link
Contributor Author

squakez commented Jun 28, 2023

The failing check did not fail on previous attempt, merging.

@squakez squakez merged commit e45d26b into apache:main Jun 28, 2023
@squakez squakez deleted the feat/remove_storage branch June 28, 2023 06:47
@squakez
Copy link
Contributor Author

squakez commented Jun 28, 2023

@gansheer FYI, this is merged

@gansheer
Copy link
Contributor

🥳

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.

Use maven distribution available in the operator image
3 participants