You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "tdiesler (via GitHub)" <gi...@apache.org> on 2024/03/13 07:32:25 UTC
[PR] Integration builds fail with arm64 base image [camel-k]
tdiesler opened a new pull request, #5246:
URL: https://github.com/apache/camel-k/pull/5246
CrossRef: #5238
--
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
Re: [PR] Integration builds fail with arm64 base image [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #5246:
URL: https://github.com/apache/camel-k/pull/5246#discussion_r1522723666
##########
pkg/util/defaults/defaults.go:
##########
@@ -44,5 +44,10 @@ const (
installDefaultKamelets = true
)
+// Maps base image variants to their respective digests
+var baseImageVariantMapping = map[string]string{
Review Comment:
We cannot hardcode configuration values. Beside, this file should not be edited as it is regenerated automatically at every build execution.
--
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
Re: [PR] Integration builds fail with arm64 base image [camel-k]
Posted by "tdiesler (via GitHub)" <gi...@apache.org>.
tdiesler commented on code in PR #5246:
URL: https://github.com/apache/camel-k/pull/5246#discussion_r1522987162
##########
pkg/util/defaults/defaults.go:
##########
@@ -44,5 +44,10 @@ const (
installDefaultKamelets = true
)
+// Maps base image variants to their respective digests
+var baseImageVariantMapping = map[string]string{
Review Comment:
What you see here is the result of Makefile code generation. Perhaps this file should be in .gitignore
In Makefile we get the digest like this ...
```
ARM64_DIGEST := $(shell docker manifest inspect eclipse-temurin:17 | jq -r '.manifests[] | select(.platform.architecture == "arm64") | .digest')
```
True, this approach fixes the base image digest at build time instead of dynamically loading the correct variant at maven build time - I could not find the source for that. We could still get the CLI fix into the code base for now and have a separate issue for (OLM, Helm, Kustomize)
Alternatively, we could do the above lookup (every time) when the base image is actually needed, which I assume would then work for all install methods
--
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
Re: [PR] Integration builds fail with arm64 base image [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5246:
URL: https://github.com/apache/camel-k/pull/5246#issuecomment-2011627961
Superseded by #5262
--
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
Re: [PR] Integration builds fail with arm64 base image [camel-k]
Posted by "tdiesler (via GitHub)" <gi...@apache.org>.
tdiesler commented on PR #5246:
URL: https://github.com/apache/camel-k/pull/5246#issuecomment-1997538456
In this second attempt, we simply tell Jib to consider multiple platforms for the FROM image
```
-Djib.from.platforms=linux/amd64,linux/arm64 -Djib.from.image=eclipse-temurin:17
```
I also upgraded to jib-3.4.1
--
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
Re: [PR] Integration builds fail with arm64 base image [camel-k]
Posted by "tdiesler (via GitHub)" <gi...@apache.org>.
tdiesler commented on code in PR #5246:
URL: https://github.com/apache/camel-k/pull/5246#discussion_r1522987162
##########
pkg/util/defaults/defaults.go:
##########
@@ -44,5 +44,10 @@ const (
installDefaultKamelets = true
)
+// Maps base image variants to their respective digests
+var baseImageVariantMapping = map[string]string{
Review Comment:
What you see here is the result of Makefile code generation. Perhaps this file should be in .gitignore
In Makefile we get the digest like this ...
```
ARM64_DIGEST := $(shell docker manifest inspect eclipse-temurin:17 | jq -r '.manifests[] | select(.platform.architecture == "arm64") | .digest')
```
True, this approach fixes the base image digest at build time instead of dynamically loading the correct variant at maven build time - I could not find the source for that. We could still get CLI fix into the code base for now and have a separate issue for (OLM, Helm, Kustomize)
Alternatively, we could do the above lookup (every time) when the base image is actually needed, which I assume would then work for all install methods
--
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
Re: [PR] Integration builds fail with arm64 base image [camel-k]
Posted by "tdiesler (via GitHub)" <gi...@apache.org>.
tdiesler commented on code in PR #5246:
URL: https://github.com/apache/camel-k/pull/5246#discussion_r1522987162
##########
pkg/util/defaults/defaults.go:
##########
@@ -44,5 +44,10 @@ const (
installDefaultKamelets = true
)
+// Maps base image variants to their respective digests
+var baseImageVariantMapping = map[string]string{
Review Comment:
What you see here is the result of Makefile code generation. Perhaps this file should be in .gitignore
In Makefile we get the digest like this ...
```
ARM64_DIGEST := $(shell docker manifest inspect eclipse-temurin:17 | jq -r '.manifests[] | select(.platform.architecture == "arm64") | .digest')
```
True, this approach fixes the base image digest at build time instead of dynamically loading the correct variant at maven build time - I could not find the source for that. Perhaps we can still get this into the code base for now and have a separate issue for (OLM, Helm, Kustomize)
Alternatively, we could do the above lookup (every time) when the base image is actually needed, which I assume would then work for all install methods
--
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
Re: [PR] Integration builds fail with arm64 base image [camel-k]
Posted by "tdiesler (via GitHub)" <gi...@apache.org>.
tdiesler commented on code in PR #5246:
URL: https://github.com/apache/camel-k/pull/5246#discussion_r1522987162
##########
pkg/util/defaults/defaults.go:
##########
@@ -44,5 +44,10 @@ const (
installDefaultKamelets = true
)
+// Maps base image variants to their respective digests
+var baseImageVariantMapping = map[string]string{
Review Comment:
What you see here is the result of Makefile code generation. Perhaps this file should be in .gitignore
In Makefile we get the digest like this ...
```
ARM64_DIGEST := $(shell docker manifest inspect eclipse-temurin:17 | jq -r '.manifests[] | select(.platform.architecture == "arm64") | .digest')
```
True, this approach fixes the base image digest at build time instead of dynamically loading the correct variant at maven build time - I could not find the source for that. Perhaps we can still get this into the code base for now and have a separate issues for (OLM, Helm, Kustomize)
Alternatively, we could do the above lookup (every time) when the base image is actually needed, which I assume would then work for all install methods
--
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
Re: [PR] Integration builds fail with arm64 base image [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5246:
URL: https://github.com/apache/camel-k/pull/5246#discussion_r1524578978
##########
pkg/util/defaults/defaults.go:
##########
@@ -44,5 +44,10 @@ const (
installDefaultKamelets = true
)
+// Maps base image variants to their respective digests
+var baseImageVariantMapping = map[string]string{
Review Comment:
This may hijack a little bit the issue but this comment about runtime lookup of the right base image make me thinking if we should do it for every architecture and having the `IntegrationPlatform` reflecting the images in its status.
The reason for that is something we never have discussed but it should be possible to have a mixed kubernetes set-up where some nodes are amd64 and some arm64 so you may have some kits working for some nodes and not for some others.
--
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
Re: [PR] Integration builds fail with arm64 base image [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5246:
URL: https://github.com/apache/camel-k/pull/5246#issuecomment-1999829551
Ref #4149
--
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
Re: [PR] Integration builds fail with arm64 base image [camel-k]
Posted by "tdiesler (via GitHub)" <gi...@apache.org>.
tdiesler commented on PR #5246:
URL: https://github.com/apache/camel-k/pull/5246#issuecomment-2003478952
> Thanks, I think this is going in the right direction. I have a question though. What does that parameter exactly do? Does it create a manifest and also the 2 different container architectures? I have the feeling this is one of those thing we want to parametrize via some user variable (could be done into the builder trait). Also, we should work (in a separate PR) to support this in the S2I if that strategy is not already taking care of this aspect.
>
> Beside that, we do need to include some documentation to explain all this stuff.
Pls see: https://github.com/apache/camel-k/issues/5238#issuecomment-2003459798
--
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
Re: [PR] Integration builds fail with arm64 base image [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5246:
URL: https://github.com/apache/camel-k/pull/5246#issuecomment-2009270960
Unfortunately this is not yet good to be merged. We have an upgrade check failure. It seems it's because the previous base image was not built with the multi platform, confirming my assumption that we must provide the platform value as a parameter.
```
dump.go:337: > {"level":"info","ts":"2024-03-19T08:39:07Z","logger":"camel-k.maven.build","msg":"Using base image with digest: sha256:3a874c26391e63a32e7dcf668b9909a9d8173d998083b7a85b7c5fa5c23c4546"}
dump.go:337: > {"level":"info","ts":"2024-03-19T08:39:07Z","logger":"camel-k.maven.build","msg":""}
dump.go:337: > {"level":"info","ts":"2024-03-19T08:39:07Z","logger":"camel-k.maven.build","msg":"------------------------------------------------------------------------"}
dump.go:337: > {"level":"info","ts":"2024-03-19T08:39:07Z","logger":"camel-k.maven.build","msg":"BUILD FAILURE"}
dump.go:337: > {"level":"info","ts":"2024-03-19T08:39:07Z","logger":"camel-k.maven.build","msg":"------------------------------------------------------------------------"}
dump.go:337: > {"level":"info","ts":"2024-03-19T08:39:07Z","logger":"camel-k.maven.build","msg":"Total time: 2.208 s (Wall Clock)"}
dump.go:337: > {"level":"info","ts":"2024-03-19T08:39:07Z","logger":"camel-k.maven.build","msg":"Finished at: 2024-03-19T08:39:07Z"}
dump.go:337: > {"level":"info","ts":"2024-03-19T08:39:07Z","logger":"camel-k.maven.build","msg":"------------------------------------------------------------------------"}
dump.go:337: > {"level":"error","ts":"2024-03-19T08:39:07Z","logger":"camel-k.maven.build","msg":"Failed to execute goal com.google.cloud.tools:jib-maven-plugin:3.4.1:build (default-cli) on project camel-k-integration: cannot build for multiple platforms since the base image 'kind-registry:5000/test-8caec862-1309-4695-81f8-9c6dabddf705/camel-k-kit-cnskqin2fd5c73csicrg@sha256:3a874c26391e63a32e7dcf668b9909a9d8173d998083b7a85b7c5fa5c23c4546' is not a manifest list. -> [Help 1]","stacktrace":"github.com/apache/camel-k/v2/pkg/util/log.Logger.Error\n\tgithub.com/apache/camel-k/v2/pkg/util/log/log.go:81\ngithub.com/apache/camel-k/v2/pkg/util/maven.normalizeLog\n\tgithub.com/apache/camel-k/v2/pkg/util/maven/maven_log.go:83\ngithub.com/apache/camel-k/v2/pkg/util/maven.MavenLogHandler\n\tgithub.com/apache/camel-k/v2/pkg/util/maven/maven_log.go:54\ngithub.com/apache/camel-k/v2/pkg/util.scan\n\tgithub.com/apache/camel-k/v2/pkg/util/command.go:76\ngithub.com/apache/camel-k/v2/pkg/ut
il.RunAndLog.func1\n\tgithub.co
```
--
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
Re: [PR] Integration builds fail with arm64 base image [camel-k]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5246:
URL: https://github.com/apache/camel-k/pull/5246#issuecomment-1993739690
:warning: Unit test coverage report - coverage decreased from 37.2% to 37.1% (**-0.1%**)
--
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
Re: [PR] Integration builds fail with arm64 base image [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez closed pull request #5246: Integration builds fail with arm64 base image
URL: https://github.com/apache/camel-k/pull/5246
--
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