You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "squakez (via GitHub)" <gi...@apache.org> on 2023/06/13 10:22:19 UTC
[GitHub] [camel-k] squakez opened a new pull request, #4483: feat: remove the need for a storage
squakez opened a new pull request, #4483:
URL: https://github.com/apache/camel-k/pull/4483
<!-- Description -->
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/
<!--
Enter your extended release note in the below block. If the PR requires
additional action from users switching to the new release, include the string
"action required". If no release note is required, write "NONE".
You can (optionally) mark this PR with labels "kind/bug" or "kind/feature" to make sure
the text is added to the right section of the release notes.
-->
**Release Note**
```release-note
feat: remove the need for a storage
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #4483: feat: remove the need for a storage
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #4483:
URL: https://github.com/apache/camel-k/pull/4483#issuecomment-1591284862
The fix addressing the concerns in previous comment should be available in https://github.com/container-tools/spectrum/pull/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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] gansheer commented on pull request #4483: feat: remove the need for a storage
Posted by "gansheer (via GitHub)" <gi...@apache.org>.
gansheer commented on PR #4483:
URL: https://github.com/apache/camel-k/pull/4483#issuecomment-1610890633
:partying_face:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #4483: feat: remove the need for a storage
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #4483:
URL: https://github.com/apache/camel-k/pull/4483#issuecomment-1610863110
The failing check did not fail on previous attempt, merging.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] gansheer commented on a diff in pull request #4483: feat: remove the need for a storage
Posted by "gansheer (via GitHub)" <gi...@apache.org>.
gansheer commented on code in PR #4483:
URL: https://github.com/apache/camel-k/pull/4483#discussion_r1232398674
##########
pkg/controller/catalog/initialize.go:
##########
@@ -285,8 +285,12 @@ func initializeS2i(ctx context.Context, c client.Client, ip *v1.IntegrationPlatf
return fmt.Errorf("cannot create tar archive: %w", err)
}
- err = tarEntries(archiveFile, "/usr/local/bin/kamel:/usr/local/bin/kamel",
- "/usr/share/maven/mvnw/:/usr/share/maven/mvnw/")
+ err = tarEntries(archiveFile,
+ "/usr/local/bin/kamel:/usr/local/bin/kamel",
+ "/usr/share/maven/mvnw/:/usr/share/maven/mvnw/",
+ // Required for snapshots dependencies in the runtimes
+ defaults.LocalRepository+":"+defaults.LocalRepository,
Review Comment:
This line is also missing from the Dockerfile definition line180:
```
dockerfile := string([]byte(`
FROM ` + catalog.Spec.GetQuarkusToolingImage() + `
ADD /usr/local/bin/kamel /usr/local/bin/kamel
ADD /usr/share/maven/mvnw/ /usr/share/maven/mvnw/
ADD ` + defaults.LocalRepository + ` ` + defaults.LocalRepository + `
`))
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #4483: feat: remove the need for a storage
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #4483:
URL: https://github.com/apache/camel-k/pull/4483#issuecomment-1610863573
@gansheer FYI, this is merged
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez merged pull request #4483: feat: remove the need for a storage
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez merged PR #4483:
URL: https://github.com/apache/camel-k/pull/4483
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #4483: feat: remove the need for a storage
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #4483:
URL: https://github.com/apache/camel-k/pull/4483#issuecomment-1604359020
Failures are likely due to https://github.com/apache/camel-k/issues/4511
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #4483: feat: remove the need for a storage
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #4483:
URL: https://github.com/apache/camel-k/pull/4483#issuecomment-1590816726
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] gansheer commented on a diff in pull request #4483: feat: remove the need for a storage
Posted by "gansheer (via GitHub)" <gi...@apache.org>.
gansheer commented on code in PR #4483:
URL: https://github.com/apache/camel-k/pull/4483#discussion_r1232398674
##########
pkg/controller/catalog/initialize.go:
##########
@@ -285,8 +285,12 @@ func initializeS2i(ctx context.Context, c client.Client, ip *v1.IntegrationPlatf
return fmt.Errorf("cannot create tar archive: %w", err)
}
- err = tarEntries(archiveFile, "/usr/local/bin/kamel:/usr/local/bin/kamel",
- "/usr/share/maven/mvnw/:/usr/share/maven/mvnw/")
+ err = tarEntries(archiveFile,
+ "/usr/local/bin/kamel:/usr/local/bin/kamel",
+ "/usr/share/maven/mvnw/:/usr/share/maven/mvnw/",
+ // Required for snapshots dependencies in the runtimes
+ defaults.LocalRepository+":"+defaults.LocalRepository,
Review Comment:
This modification is missing from the Dockerfile definition line180:
```
dockerfile := string([]byte(`
FROM ` + catalog.Spec.GetQuarkusToolingImage() + `
ADD /usr/local/bin/kamel /usr/local/bin/kamel
ADD /usr/share/maven/mvnw/ /usr/share/maven/mvnw/
ADD ` + defaults.LocalRepository + ` ` + defaults.LocalRepository + `
`))
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org