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

MultiArchitecture support #3309

Merged
merged 35 commits into from
Jun 20, 2022
Merged

MultiArchitecture support #3309

merged 35 commits into from
Jun 20, 2022

Conversation

robertonav20
Copy link
Contributor

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

ConsNavarraRoberto and others added 15 commits May 13, 2022 09:23
update docker file for camel-k operator
change makefile to build multiplatform image
add platform parameter to buildah task
add buildah go file inside builder package
add Architecture field inside publish task and mapping to buildah
platform attribute moved to BuildahTask
add images-arch images-dev-arch bundle-build-arch for manual build
@robertonav20 robertonav20 mentioned this pull request May 27, 2022
5 tasks
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.

Overall very nice accomplishment. There are a few things that may need some adjustment. Also, I'd rename everything you've specified as arch as arm64 instead, in order to understand it is referring to that particular architecture.

pkg/apis/camel/v1/build_types.go Outdated Show resolved Hide resolved
pkg/controller/build/build_pod.go Outdated Show resolved Hide resolved
pkg/trait/builder.go Outdated Show resolved Hide resolved
pkg/util/defaults/defaults.go Show resolved Hide resolved
script/Makefile Outdated Show resolved Hide resolved
script/Makefile Outdated Show resolved Hide resolved
script/Makefile Outdated Show resolved Hide resolved
ConsNavarraRoberto added 2 commits May 31, 2022 10:47
use platform attribute if exists
docker.io from defaults.go
@robertonav20
Copy link
Contributor Author

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

go.mod Outdated Show resolved Hide resolved
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. It would be nice to include some example or documentation so that everybody know how to use the new feature.

@robertonav20
Copy link
Contributor Author

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

Thanks in advance

@squakez
Copy link
Contributor

squakez commented Jun 1, 2022

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.

@squakez squakez added the kind/feature New feature or request label Jun 1, 2022
@robertonav20
Copy link
Contributor Author

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

@robertonav20
Copy link
Contributor Author

Hi @squakez , how we can proceed with pr?

@squakez
Copy link
Contributor

squakez commented Jun 10, 2022

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.

@squakez
Copy link
Contributor

squakez commented Jun 13, 2022

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

@robertonav20
Copy link
Contributor Author

@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

@squakez
Copy link
Contributor

squakez commented Jun 14, 2022

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.

@robertonav20
Copy link
Contributor Author

robertonav20 commented Jun 14, 2022

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

@robertonav20
Copy link
Contributor Author

robertonav20 commented Jun 14, 2022

@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 :(

But i think, this is an optional feature, so we can proceed with pr and open another task to fix him.
In addition i suggest to open another task to add "platform" field inside the image's manifest to respect OCI format

Like this

   "platform": {
      "architecture": "amd64",
      "os": "linux"
    }

@squakez
Copy link
Contributor

squakez commented Jun 15, 2022

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?

@robertonav20
Copy link
Contributor Author

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

@squakez
Copy link
Contributor

squakez commented Jun 15, 2022

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.

@robertonav20
Copy link
Contributor Author

robertonav20 commented Jun 16, 2022

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

@squakez
Copy link
Contributor

squakez commented Jun 16, 2022

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.

@robertonav20
Copy link
Contributor Author

I rebased the branch, can you restart the tests?

@squakez
Copy link
Contributor

squakez commented Jun 16, 2022

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?

@robertonav20
Copy link
Contributor Author

Ok, is the only error?

@squakez
Copy link
Contributor

squakez commented Jun 17, 2022

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.

@robertonav20
Copy link
Contributor Author

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

@squakez
Copy link
Contributor

squakez commented Jun 20, 2022

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

@robertonav20
Copy link
Contributor Author

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

@squakez
Copy link
Contributor

squakez commented Jun 20, 2022

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.

@squakez squakez merged commit d71cddf into apache:main Jun 20, 2022
@squakez
Copy link
Contributor

squakez commented Jun 20, 2022

Merged. Thanks for the contribution! I think we can have a blog announcing this experimental feature, feel free to add one into 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).

@robertonav20
Copy link
Contributor Author

Nice! Thanks to you for your guide!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants