You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2022/05/27 09:43:03 UTC

[GitHub] [camel-k] robertonav20 opened a new pull request, #3309: MultiArchitecture support

robertonav20 opened a new pull request, #3309:
URL: https://github.com/apache/camel-k/pull/3309

   <!-- Description -->
   The pr introduces:
   - Add manual command to build kamel operator to support arm architecture
   - Add Dockerfile.arch to support arm architecture with graalvm image for now quarkus image is unavailable
   - Update buildah version to 1.23.3 to support arm architecture
   
   PS. to enable this behavior just use --build-publish-strategy=Buildah inside install command
   


-- 
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 #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1143135783

   Examples are in https://github.com/apache/camel-k/tree/main/examples
   Docs in https://github.com/apache/camel-k/tree/main/docs (you may need to create a new entry in https://github.com/apache/camel-k/tree/main/docs/modules/ROOT/pages/configuration likely). You can add the page and I'll take care of linking it correctly later if needed.
   
   By the way, we've fixed a few things in the ci recently. I think you can rebase with `main` to check the result of ci actions again.


-- 
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] robertonav20 commented on pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1145978906

   The trait builder create buildahTask correctly
   
   Below the screenshot with the log 
   ![image](https://user-images.githubusercontent.com/6991920/171865794-ae21b118-c050-4bc9-8a62-0eb92ebbb598.png)
   ![image](https://user-images.githubusercontent.com/6991920/171864988-db8d0cee-3bd1-40f8-ae46-78b3477b5a67.png)
   
   But when the addBuildahTaskToPod receive the task the attribute platform is empty
   ![image](https://user-images.githubusercontent.com/6991920/171865600-a88d5b11-cb09-4add-a805-1badfad7c5b7.png)
   
   So i think there is a problem with mapping after trait builder 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


[GitHub] [camel-k] squakez commented on a diff in pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
squakez commented on code in PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#discussion_r885448043


##########
go.mod:
##########
@@ -8,6 +8,7 @@ require (
 	github.com/apache/camel-k/pkg/client/camel v0.0.0
 	github.com/apache/camel-k/pkg/kamelet/repository v0.0.0
 	github.com/container-tools/spectrum v0.3.8
+	github.com/containerd/containerd v1.5.8

Review Comment:
   Should be not needed any longer
   



-- 
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] robertonav20 commented on pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1156057768

   Here you can find the build yaml
   
   ```
   apiVersion: v1
   items:
   - apiVersion: camel.apache.org/v1
     kind: Build
     metadata:
       creationTimestamp: "2022-06-15T06:49:27Z"
       generation: 1
       labels:
         camel.apache.org/created.by.kind: Integration
         camel.apache.org/created.by.name: test
         camel.apache.org/created.by.namespace: default
         camel.apache.org/created.by.version: "531775"
         camel.apache.org/kit.layout: fast-jar
       name: kit-cako1tqr0ppc73e4mi00
       namespace: default
       ownerReferences:
       - apiVersion: camel.apache.org/v1
         blockOwnerDeletion: true
         controller: true
         kind: IntegrationKit
         name: kit-cako1tqr0ppc73e4mi00
         uid: a2504e35-e381-495a-8958-6277545023b0
       resourceVersion: "531806"
       uid: d7c510d1-5b03-46ba-acb5-0dcc8898bca2
     spec:
       strategy: pod
       tasks:
       - builder:
           baseImage: docker.io/adoptopenjdk/openjdk11:slim
           dependencies:
           - camel:log
           - camel:timer
           - mvn:org.apache.camel.k:camel-k-runtime
           - mvn:org.apache.camel.quarkus:camel-quarkus-groovy-dsl
           maven:
             cliOptions:
             - -V
             - --no-transfer-progress
             - -Dstyle.color=never
             localRepository: /tmp/artifacts/m2
             properties:
               quarkus.package.type: fast-jar
             settings: {}
           name: builder
           runtime:
             applicationClass: io.quarkus.bootstrap.runner.QuarkusEntryPoint
             capabilities:
               circuit-breaker:
                 dependencies:
                 - artifactId: camel-quarkus-microprofile-fault-tolerance
                   groupId: org.apache.camel.quarkus
               cron:
                 dependencies:
                 - artifactId: camel-k-cron
                   groupId: org.apache.camel.k
               health:
                 dependencies:
                 - artifactId: camel-quarkus-microprofile-health
                   groupId: org.apache.camel.quarkus
               master:
                 dependencies:
                 - artifactId: camel-k-master
                   groupId: org.apache.camel.k
               platform-http:
                 dependencies:
                 - artifactId: camel-quarkus-platform-http
                   groupId: org.apache.camel.quarkus
               rest:
                 dependencies:
                 - artifactId: camel-quarkus-platform-http
                   groupId: org.apache.camel.quarkus
                 - artifactId: camel-quarkus-rest
                   groupId: org.apache.camel.quarkus
               tracing:
                 dependencies:
                 - artifactId: camel-quarkus-opentracing
                   groupId: org.apache.camel.quarkus
             dependencies:
             - artifactId: camel-k-runtime
               groupId: org.apache.camel.k
             metadata:
               camel-quarkus.version: 2.8.0
               camel.version: 3.16.0
               quarkus.version: 2.8.0.Final
             provider: quarkus
             version: 1.13.0
           steps:
           - github.com/apache/camel-k/pkg/builder/LoadCamelQuarkusCatalog
           - github.com/apache/camel-k/pkg/builder/CleanUpBuildDir
           - github.com/apache/camel-k/pkg/builder/GenerateJavaKeystore
           - github.com/apache/camel-k/pkg/builder/GenerateQuarkusProject
           - github.com/apache/camel-k/pkg/builder/GenerateProjectSettings
           - github.com/apache/camel-k/pkg/builder/InjectDependencies
           - github.com/apache/camel-k/pkg/builder/SanitizeDependencies
           - github.com/apache/camel-k/pkg/builder/BuildQuarkusRunner
           - github.com/apache/camel-k/pkg/builder/ComputeQuarkusDependencies
           - github.com/apache/camel-k/pkg/builder/IncrementalImageContext
           - github.com/apache/camel-k/pkg/builder/JvmDockerfile
       - buildah:
           image: ghcr.io/robertonav20/default/camel-k-kit-cako1tqr0ppc73e4mi00:531781
           name: buildah
           registry:
             address: ghcr.io/robertonav20
             secret: camel-k-registry-secret
       timeout: 5m0s
     status:
       phase: Running
       startedAt: "2022-06-15T06:49:27Z"
   kind: List
   metadata:
     resourceVersion: ""
     selfLink: ""
   ```


-- 
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] robertonav20 commented on pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1143708516

   I made some fix after our changes and now all changes works except the PublishStrategyOptions, if i add the new constant BuildahPlatform inside IntegrationPlatform like this with `kubectl edit ip`
   
   ```
   status:
     build:
       PublishStrategyOptions:
         BuildahPlatform: linux/arm/v8
   ```
   But buildah don't reiceve this property because i add some logs inside builder.go... i don't know if i made the correct steps...
   
   Have you some ideas?


-- 
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] robertonav20 commented on pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1157992705

   Ok, is the only error?


-- 
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 #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1154085707

   @robertonav20 have you managed to sort those problems out? If tests pass and you confirm it's good to go, I will merge.


-- 
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] robertonav20 commented on pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1144852118

   Yes, i made as you describe below the yaml
   ```
   apiVersion: camel.apache.org/v1
   kind: IntegrationPlatform
   metadata:
     annotations:
       kubectl.kubernetes.io/last-applied-configuration: |
         {"apiVersion":"camel.apache.org/v1","kind":"IntegrationPlatform","metadata":{"annotations":{},"creationTimestamp":"2022-05-20T16:43:42Z","generation":1,"labels":{"app":"camel-k"},"managedFields":[{"apiVersion":"camel.apache.org/v1","fieldsType":"FieldsV1","fieldsV1":{"f:metadata":{"f:labels":{".":{},"f:app":{}  creationTimestamp: "2022-06-01T13:03:33Z"
     generation: 2
     labels:
       app: camel-k
     name: camel-k
     namespace: default
     resourceVersion: "330380"
     uid: 4793fd1d-cafd-4a8e-99cc-5e2cd2edef29
   spec:
     build:
       PublishStrategyOptions:
         BuildahPlatform: linux/arm/v8
       maven:
         settings: {}
       publishStrategy: Buildah
       registry:
         address: ghcr.io/robertonav20
         secret: camel-k-registry-secret
     kamelet: {}
     resources: {}
   status:
     build:
       PublishStrategyOptions:
         BuildahPlatform: linux/arm/v8
         KanikoPersistentVolumeClaim: camel-k
       baseImage: docker.io/adoptopenjdk/openjdk11:slim
       buildStrategy: pod
       maven:
         cliOptions:
         - -V
         - --no-transfer-progress
         - -Dstyle.color=never
         localRepository: /tmp/artifacts/m2
       publishStrategy: Buildah
       registry:
         address: ghcr.io/robertonav20
         secret: camel-k-registry-secret
       runtimeVersion: 1.13.0
       timeout: 5m0s
     cluster: Kubernetes
     info:
       buildahVersion: 1.23.3
       gitCommit: ae3509cbbffdf6bdb5d2124bdc7f09c9fd1cc1f8
       goOS: linux
       goVersion: go1.18.1
     kamelet:
       repositories:
       - uri: none
     phase: Ready
     version: 1.10.0-SNAPSHOT
   ```
   
   In addition i add this logs inside builder.go to trace if the buildahTask is builded correctly
   ```
   if platform, found = e.Platform.Status.Build.PublishStrategyOptions[builder.BuildahPlatform]; !found {
   	platform = ""
   	t.L.Infof("Attribute platform for buildah not found, default from host will be used!")
   } else {
   	t.L.Infof("User defined %s platform, will be used from buildah!", platform)
   }
   ```
   
   I don't see both logs, so i think the trait builder can't be executed


-- 
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] robertonav20 commented on pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1152316035

   Hi @squakez , how we can proceed with pr?


-- 
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] robertonav20 commented on pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1157636170

   I rebased the branch, can you restart the tests?


-- 
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 #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1160055307

   Restarted. I have seen in your branch, only one test is failing (which is a flaky one): https://github.com/robertonav20/camel-k/runs/6948033955?check_suite_focus=true If that is the case here as well, I will squash and merge accordingly. :crossed_fingers: 


-- 
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] robertonav20 commented on pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1142103313

   Hi @squakez, how i share the documentation? Have you a package inside the repository or other?
   
   Thanks in advance


-- 
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] robertonav20 commented on pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1154101416

   @squakez no unfortunately, i just made an analysis to understand the problem... you can see the details in the previous comment.
   
   By the way i tested correctly the integration kit image without that param architecture


-- 
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 #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1156220019

   The problem is here:
   ```
       - buildah:
           image: ghcr.io/robertonav20/default/camel-k-kit-cako1tqr0ppc73e4mi00:531781
           name: buildah
           registry:
             address: ghcr.io/robertonav20
             secret: camel-k-registry-secret
   ```
   The Buildah configuration is missing the platform. Ie, when I run it, I can see:
   ```
       - buildah:
           image: localhost:5000/operator-test/camel-k-kit-cakq678cibldo863h53g:2284561
           name: buildah
           platform: linux/arm/v8
           registry:
             address: localhost:5000
   ```
   I still think it does not work for you because CRDs are not correctly applied. I will rerun the failing tests, and, if they are green and you agree we can merge this. Then, you will be able to try again as soon as it is merged via nightly build installation in order to test it clean.


-- 
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] robertonav20 commented on pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1160261680

   I rebased the branch with last commit, can you restart the tests ? If they complete successfully we can merge


-- 
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] robertonav20 commented on pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1159421846

   i think which our problem is the generated crd, because the tests starts to fail from that change... i relaunch the generation of  crds, if fail again can you start them? maybe i have something wrong on my pc
   


-- 
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 #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1144980673

   @robertonav20 thanks for the installation instructions. I'll have a look and test to see if I can help troubleshooting. I'll keep you posted.


-- 
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] robertonav20 commented on a diff in pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on code in PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#discussion_r885316389


##########
pkg/trait/builder.go:
##########
@@ -101,7 +102,14 @@ func (t *builderTrait) Apply(e *Environment) error {
 		}})
 
 	case v1.IntegrationPlatformBuildPublishStrategyBuildah:
+		var platform string
+		var found bool
+		if platform, found = e.Platform.Status.Build.PublishStrategyOptions[builder.BuildahPlatform]; !found {
+			platform = platforms.DefaultSpec().OS + "/" + platforms.DefaultSpec().Architecture + "/" + platforms.DefaultSpec().Variant

Review Comment:
   Yeah, if the PublishStrategyOptions haven't this value we proceed with default value retrieved from Node architecture.
   Buildah as default use the architecture of Node if you don't specify platform attribute



-- 
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] robertonav20 commented on pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1155551774

   @squakez i tested again with your instructions but i get the same result
   
   ```
   {"level":"info","ts":1655230597.095445,"logger":"camel-k.controller.integrationkit","msg":"Invoking action build","request-namespace":"default","request-name":"kit-cakd11275mqc73ao1qm0","api-version":"camel.apache.org/v1","kind":"IntegrationKit","ns":"default","name":"kit-cakd11275mqc73ao1qm0"}
   {"level":"info","ts":1655230597.0983949,"logger":"camel-k.traits","msg":"User defined linux/arm/v8 platform, will be used from buildah!","trait":"builder"}
   
   buildah bud --storage-driver=vfs --pull-always -f Dockerfile -t ghcr.io/robertonav20/default/camel-k-kit-cakd11275mqc73ao1qm0:524604 . && buildah push --storage-driver=vfs --digestfile=/dev/termination-log ghcr.io/robertonav20/default/camel-k-kit-cakd11275mqc73ao1qm0:524604 docker://ghcr.io/robertonav20/default/camel-k-kit-cakd11275mqc73ao1qm0:524604
   ```
   I don't know how to help :(


-- 
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 #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1157632321

   > The test are failed again, can i help with this?
   > 
   > For me we can proceed with pr, i will test the feature with the night build
   
   Can you please rebase this PR against the latest `main`? I am not entirely sure why all those tests are failing. We've fixed a few things lately, so, hopefully they are not related to these changes.


-- 
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 #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1157787131

   This test is suspiciously failing every time:
   ```
   --- FAIL: TestKitTimerToLogFullBuild (617.74s)
   ```
   It seems it ends up always in timeout. I think it is related to the PR. Can you please have a look?


-- 
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] robertonav20 commented on pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1157621220

   The test are failed again, can ihelp with this?


-- 
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 #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
squakez merged PR #3309:
URL: https://github.com/apache/camel-k/pull/3309


-- 
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 #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1144561785

   @robertonav20 I think you need to edit the value in the `.spec`, not directly the `.status`. The operator will be in charge to reconcile the spec into the status. Try changing that, and after a few seconds, check that the status has been reconciled with the value provided in the spec (ie, `kubectl get ip -o yaml`).


-- 
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] robertonav20 commented on pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1145938511

   Yes i know, i have a raspberry with a k3s cluster i used to run the custom image.... i don't understand because the trait builder isn't executed maybe depends from some missing configuration or others. I will try to find the problem


-- 
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] robertonav20 commented on pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1160426298

   Nice! Thanks to you for your guide!


-- 
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 #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1160416100

   Merged. Thanks for the contribution! I think we can have a blog announcing this experimental feature, feel free to add one into [camel-website](https://github.com/apache/camel-website). I guess we can reuse part of the documentation you already wrote, maybe with some snapshot taken from your tests (and some performance measurement). 


-- 
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] robertonav20 commented on pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1143215648

   Ok Good! I will add documentation and some check before merge pr


-- 
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 a diff in pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
squakez commented on code in PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#discussion_r885444988


##########
pkg/controller/build/build_pod.go:
##########
@@ -238,18 +238,34 @@ func addBuildTaskToPod(build *v1.Build, taskName string, pod *corev1.Pod) {
 }
 
 func addBuildahTaskToPod(ctx context.Context, c ctrl.Reader, build *v1.Build, task *v1.BuildahTask, pod *corev1.Pod) error {
-	bud := []string{
-		"buildah",
-		"bud",
-		"--storage-driver=vfs",
-		"--platform",
-		task.Platform,
-		"--pull-always",
-		"-f",
-		"Dockerfile",
-		"-t",
-		task.Image,
-		".",
+	bud := []string{}
+
+	if task.Platform != "" {

Review Comment:
   I think we can do something better for readability. Pseudo code follows:
   ```
   	bud = []string{
   			"buildah",
   			"bud",
   			"--storage-driver=vfs",
   			"--pull-always",
   			"-f",
   			"Dockerfile",
   			"-t",
   			task.Image,
   			".",
   }
   if platform != "" {
       bud = append(bud, []string{
            		task.Platform,
   			"--pull-always",
   			"-f",
       }
   }
   ```



-- 
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 #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1154923665

   I wonder if you have some older build that is reused. Try to do a `kamel reset` in order to clean any previous build/kit. Then, once you've re-started an Integration, check the configuration of the Build via `kubectl get build -o yaml`. Look for the `buildah` configuration to see if the platform parameter is there or not. Feel free to report any output so we can help troubleshooting.


-- 
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] robertonav20 commented on pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1155119948

   Ok thanks... i will do these steps in some few days, anyway the tests are failed so how you proceed?


-- 
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 #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1145662081

   Problem found. As we've changed the API definition, we need to regenerate the CRDs and apply them as well. You need to include in this PR the changes coming from `make generate` and locally you must update the CRDs you will find in `/config/crds/bases/camel.apache.org_builds.yaml` ie `kubectl apply -f /config/crds/bases/camel.apache.org_builds.yaml`.


-- 
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] robertonav20 commented on pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1145913220

   I made as you suggest but i have the same problem.... i don't see both logs and the buildah bud command haven't platform attribute :(
   
   I commit and push the new crd


-- 
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 a diff in pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
squakez commented on code in PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#discussion_r885267026


##########
pkg/apis/camel/v1/build_types.go:
##########
@@ -107,6 +107,8 @@ type PublishTask struct {
 type BuildahTask struct {
 	BaseTask    `json:",inline"`
 	PublishTask `json:",inline"`
+	// The platform of build image
+	Platform string `json:"baseImage,omitempty"`

Review Comment:
   I think it should be `json:"platform,omitempty"`



##########
pkg/util/defaults/defaults.go:
##########
@@ -29,13 +29,13 @@ const (
 	DefaultRuntimeVersion = "1.13.0"
 
 	// BuildahVersion --
-	BuildahVersion = "1.14.0"
+	BuildahVersion = "1.23.3"
 
 	// KanikoVersion --
 	KanikoVersion = "0.17.1"
 
 	// baseImage --
-	baseImage = "adoptopenjdk/openjdk11:slim"
+	baseImage = "docker.io/adoptopenjdk/openjdk11:slim"

Review Comment:
   Is this needed by the new Buildah?



##########
script/Makefile:
##########
@@ -19,16 +19,17 @@ OPERATOR_VERSION := $(subst -SNAPSHOT,,$(VERSION))
 LAST_RELEASED_IMAGE_NAME := camel-k-operator
 LAST_RELEASED_VERSION := 1.9.1
 RUNTIME_VERSION := 1.13.0
-BUILDAH_VERSION := 1.14.0
+BUILDAH_VERSION := 1.23.3
 KANIKO_VERSION := 0.17.1
 INSTALL_DEFAULT_KAMELETS := true
 CONTROLLER_GEN_VERSION := v0.6.1
 OPERATOR_SDK_VERSION := v1.14.0
 KUSTOMIZE_VERSION := v4.1.2
 OPM_VERSION := v1.21.0
-BASE_IMAGE := adoptopenjdk/openjdk11:slim
+BASE_IMAGE := docker.io/adoptopenjdk/openjdk11:slim
 LOCAL_REPOSITORY := /tmp/artifacts/m2
 IMAGE_NAME := docker.io/apache/camel-k
+IMAGE_TARGET_PLATFORM := linux/amd64

Review Comment:
   Better move as part of the `images-arch` procedure



##########
pkg/trait/builder.go:
##########
@@ -101,7 +102,14 @@ func (t *builderTrait) Apply(e *Environment) error {
 		}})
 
 	case v1.IntegrationPlatformBuildPublishStrategyBuildah:
+		var platform string
+		var found bool
+		if platform, found = e.Platform.Status.Build.PublishStrategyOptions[builder.BuildahPlatform]; !found {
+			platform = platforms.DefaultSpec().OS + "/" + platforms.DefaultSpec().Architecture + "/" + platforms.DefaultSpec().Variant

Review Comment:
   We're still forcing the build within the architecture where the Camel K operator Pod Node is running. Since we now have the platform as a parameter we should let the user manually provide that information without involving any `platforms` dependency.



##########
script/Makefile:
##########
@@ -90,6 +91,15 @@ TEST_PREBUILD = build
 #GOLDFLAGS += -X github.com/apache/camel-k/pkg/util/olm.DefaultStartingCSV=
 #GOLDFLAGS += -X github.com/apache/camel-k/pkg/util/olm.DefaultGlobalNamespace=openshift-operators
 
+# Define target platform for docker build

Review Comment:
   We better move this check inside the `images-arch` to make sure it will be exclusively used when it is called by that procedure.



##########
pkg/controller/build/build_pod.go:
##########
@@ -243,6 +242,9 @@ func addBuildahTaskToPod(ctx context.Context, c ctrl.Reader, build *v1.Build, ta
 		"buildah",
 		"bud",
 		"--storage-driver=vfs",
+		"--platform",

Review Comment:
   We better add this to the slice conditionally. Ie, if there is a platform, then include this into the slice.



##########
script/Makefile:
##########
@@ -547,3 +581,9 @@ endif
 # Build the bundle image.
 bundle-build: bundle
 	cd bundle && docker build -f Dockerfile -t $(BUNDLE_IMAGE_NAME) .
+
+# Build the bundle image.
+bundle-build-arch: bundle

Review Comment:
   It seems it is not used by any other makefile part



-- 
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] robertonav20 commented on a diff in pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on code in PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#discussion_r885403069


##########
pkg/controller/build/build_pod.go:
##########
@@ -243,6 +242,9 @@ func addBuildahTaskToPod(ctx context.Context, c ctrl.Reader, build *v1.Build, ta
 		"buildah",
 		"bud",
 		"--storage-driver=vfs",
+		"--platform",

Review Comment:
   As you suggest add platform args if platform attribute exists...



-- 
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 #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1156002895

   I am curious to see what do you have in the Build configuration. Can you please post the result of `kubectl get build -o yaml`?


-- 
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 #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1160264616

   > I rebased the branch with last commit, can you restart the tests ? If they complete successfully we can merge
   
   Done. FYI, the previous test execution passed correctly, so it looks in good shape now.


-- 
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 #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1158602479

   No, there are several. However, I'd focus on this one as it appears on the builder, which is the part changed in the PR. Hopefully, once we fix them, the rest will clear.


-- 
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 #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1145927500

   @robertonav20 in order to test those changes, you'll need your operator running locally: https://camel.apache.org/camel-k/1.9.x/contributing/local-development.html#local-operator or wait these changes available in the nightly build (once merged). I've managed to have it run, so, if the ci checks are green, I'll merge this PR.


-- 
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] robertonav20 commented on a diff in pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on code in PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#discussion_r885368082


##########
pkg/trait/builder.go:
##########
@@ -101,7 +102,14 @@ func (t *builderTrait) Apply(e *Environment) error {
 		}})
 
 	case v1.IntegrationPlatformBuildPublishStrategyBuildah:
+		var platform string
+		var found bool
+		if platform, found = e.Platform.Status.Build.PublishStrategyOptions[builder.BuildahPlatform]; !found {
+			platform = platforms.DefaultSpec().OS + "/" + platforms.DefaultSpec().Architecture + "/" + platforms.DefaultSpec().Variant

Review Comment:
   I propose another solution... we can invert the logic in this way.
   
   The platform attribute will be initialized and mapped inside buildah if exists, otherwise platform will be ignored



-- 
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] robertonav20 commented on a diff in pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on code in PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#discussion_r885313676


##########
pkg/util/defaults/defaults.go:
##########
@@ -29,13 +29,13 @@ const (
 	DefaultRuntimeVersion = "1.13.0"
 
 	// BuildahVersion --
-	BuildahVersion = "1.14.0"
+	BuildahVersion = "1.23.3"
 
 	// KanikoVersion --
 	KanikoVersion = "0.17.1"
 
 	// baseImage --
-	baseImage = "adoptopenjdk/openjdk11:slim"
+	baseImage = "docker.io/adoptopenjdk/openjdk11:slim"

Review Comment:
   No, i agree with you so i proceed with revert this modify



-- 
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 a diff in pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
squakez commented on code in PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#discussion_r885328491


##########
pkg/trait/builder.go:
##########
@@ -101,7 +102,14 @@ func (t *builderTrait) Apply(e *Environment) error {
 		}})
 
 	case v1.IntegrationPlatformBuildPublishStrategyBuildah:
+		var platform string
+		var found bool
+		if platform, found = e.Platform.Status.Build.PublishStrategyOptions[builder.BuildahPlatform]; !found {
+			platform = platforms.DefaultSpec().OS + "/" + platforms.DefaultSpec().Architecture + "/" + platforms.DefaultSpec().Variant

Review Comment:
   My main concern is we introduce a dependency (with all its maintenance burden) just to get some OS info. Could we try instead using something already available in the Go language (ie, https://pkg.go.dev/runtime#GOOS).



-- 
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] robertonav20 commented on pull request #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
robertonav20 commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1141870770

   I prefer arch against arm64 because i think in the future camel k supports other architectures... i don't know if this feature is part of camel-k roadmap


-- 
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 #3309: MultiArchitecture support

Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3309:
URL: https://github.com/apache/camel-k/pull/3309#issuecomment-1152320212

   > Hi @squakez , how we can proceed with pr?
   
   I'm not able to follow up with it this week. If nobody else is having a look in the while, I'll check it again during next week.


-- 
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