feat: add arm64 support

[mayankshah1607]

Signed-off-by: Mayank Shah [email protected]

What type of PR is this?
/kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #138

Requirements:

• uses conventional commit messages
• includes documentation
• adds unit tests
• tested upgrade from previous version

Special notes for your reviewer:

Release note:

none

[k8s-ci-robot]

Hi @mayankshah1607. Thanks for your PR.

I'm waiting for a kubernetes-csi member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mayankshah1607
To complete the pull request process, please assign msau42 after the PR has been reviewed.
You can assign the PR to them by writing /assign @msau42 in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[mayankshah1607]

@andyzhangx We need to set the registry details here. Could you share information regarding the registry / credentials? Thanks

[andyzhangx]

make smb-container is here:

csi-driver-smb/Makefile

Lines 112 to 121 in e9c2dd3

	.PHONY: smb-container
	smb-container:
	docker buildx rm container-builder || true
	docker buildx create --use --name=container-builder
	ifdef CI
	make smb smb-windows
	docker buildx build --no-cache --build-arg LDFLAGS=${LDFLAGS} -t $(IMAGE_TAG)-linux-amd64 -f ./pkg/smbplugin/Dockerfile --platform="linux/amd64" --push .
	docker buildx build --no-cache --build-arg LDFLAGS=${LDFLAGS} -t $(IMAGE_TAG)-windows-1809-amd64 -f ./pkg/smbplugin/Windows.Dockerfile --platform="windows/amd64" --push .
	docker manifest create $(IMAGE_TAG) $(IMAGE_TAG)-linux-amd64 $(IMAGE_TAG)-windows-1809-amd64
	docker manifest inspect $(IMAGE_TAG)

I think you need to add one more command to build ARM64 container?

[coveralls]

Pull Request Test Coverage Report for Build 354236700

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 87.658%

---

Totals
Change from base Build 353928479:	0.0%
Covered Lines:	625
Relevant Lines:	713

---

💛 - Coveralls

[andyzhangx]

/ok-to-test

[mayankshah1607]

@andyzhangx I think this would do. But is make smb-container being called in the CI? I.e, the post image job?

[andyzhangx]

let's wait for the test result, I have credentials to push image to Azure Container Registry.

[andyzhangx]

I think it's better move L120 to L119

[msau42]

Our Kubernetes release pipeline supprts multiarch now including arm. But you need to setup the cloudbuild.yaml files like in the other csi repos

[andyzhangx]

there is error:

docker buildx build --no-cache --build-arg LDFLAGS="-X github.com/kubernetes-csi/csi-driver-smb/pkg/smb.driverVersion=e2e-25526bb608f502e89d3f6974ad9e94bce9ae1855 -X github.com/kubernetes-csi/csi-driver-smb/pkg/smb.gitCommit=25526bb608f502e89d3f6974ad9e94bce9ae1855 -X github.com/kubernetes-csi/csi-driver-smb/pkg/smb.buildDate=2020-10-30T14:41:35Z -s -w -extldflags '-static'" -t k8sprow.azurecr.io/smb-csi:e2e-25526bb608f502e89d3f6974ad9e94bce9ae1855-linux-arm64 -f ./pkg/smbplugin/Dockerfile --platform="linux/arm64" --push .
time="2020-10-30T14:44:42Z" level=warning msg="invalid non-bool value for BUILDX_NO_DEFAULT_LOAD: "
#1 [internal] load .dockerignore
#1 transferring context: 2B done
#1 DONE 0.0s

#2 [internal] load build definition from Dockerfile
#2 transferring dockerfile: 32B done
#2 DONE 0.0s

#3 [internal] load metadata for k8s.gcr.io/build-image/debian-base-amd64:v2...
#3 DONE 0.1s

#4 [internal] load metadata for docker.io/library/golang:1.13.10-alpine3.10
#4 DONE 1.4s
failed to solve: rpc error: code = Unknown desc = failed to load LLB: runtime execution on platform linux/arm64 not supported
make[2]: *** [Makefile:118: smb-container] Error 1
make[2]: Leaving directory '/home/prow/go/src/sigs.k8s.io/csi-driver-smb'
make[1]: *** [Makefile:74: e2e-bootstrap] Error 2
make[1]: Leaving directory '/home/prow/go/src/sigs.k8s.io/csi-driver-smb'
Failure [131.374 seconds]

not sure whether it's container image build error or Azure Container Registry not supported issue, need to find out later.

[mayankshah1607]

Related - docker/buildx#138
Do we have qemu installed in the machine running the job?

[kferrone]

Any luck? I've been following along

[mayankshah1607]

It looks like the base image being used for the failing jobs does not have qemu installed, which is a requirement for building arm64 images (as per this comment) .
Is it feasible modify the base image and have qemu installed, so that buildx can successfully build arm64 images?

/Cc @andyzhangx @msau42

[andyzhangx]

It looks like the base image being used for the failing jobs does not have qemu installed, which is a requirement for building arm64 images (as per this comment) .
Is it feasible modify the base image and have qemu installed, so that buildx can successfully build arm64 images?

/Cc @andyzhangx @msau42

since it's just a testing image, I think it's ok, could you work on a PR to install qemu? thanks.

[andyzhangx]

Any luck? I've been following along

@kferrone I have already built the master branch image with ARM64 support with this PR, you could try now:

{
   "schemaVersion": 2,
   "mediaType": "application/vnd.docker.distribution.manifest.list.v2+json",
   "manifests": [
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 1163,
         "digest": "sha256:2d6616d944e5382175c824c3ff288723e91e736b6318dbd33f2a96cd6082371a",
         "platform": {
            "architecture": "amd64",
            "os": "linux"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 1163,
         "digest": "sha256:6580abb9e16b557a10dd05a5ecc40f2b40c8d2874a50f6d0878dff118308a10c",
         "platform": {
            "architecture": "arm",
            "os": "linux",
            "variant": "v7"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 950,
         "digest": "sha256:8f2bad9a5772a4f77774a707804a0f85f60b728723d8e56c770d01d5b500038e",
         "platform": {
            "architecture": "amd64",
            "os": "windows"
         }
      }
   ]
}

In the meantime, we will figure out how to make CI workflow work for ARM image

[andyzhangx]

It looks like the base image being used for the failing jobs does not have qemu installed, which is a requirement for building arm64 images (as per this comment) .
Is it feasible modify the base image and have qemu installed, so that buildx can successfully build arm64 images?
/Cc @andyzhangx @msau42

since it's just a testing image, I think it's ok, could you work on a PR to install qemu? thanks.

@mayankshah1607 I have pushed some changes in Makefile, it could build binary and docker image with ARM64 support now, following guide: https://www.docker.com/blog/multi-platform-docker-builds/

• docker buildx setting to support ARM64

export DOCKER_CLI_EXPERIMENTAL=enabled
docker run --privileged --rm docker/binfmt:a7996909642ee92942dcd6cff44b9b95f08dad64

[mayankshah1607]

@andyzhangx Tried adding the following to the Makefile -

docker run --privileged --rm docker/binfmt:a7996909642ee92942dcd6cff44b9b95f08dad64

The error still remains. It doesn't look like the handlers are getting registered. Could it be because this is not meant to be run as DinD? In that case we could try to run ARM64 build and push separately on Github Actions

[andyzhangx]

@andyzhangx Tried adding the following to the Makefile -

docker run --privileged --rm docker/binfmt:a7996909642ee92942dcd6cff44b9b95f08dad64

The error still remains. It doesn't look like the handlers are getting registered. Could it be because this is not meant to be run as DinD? In that case we could try to run ARM64 build and push separately on Github Actions

it could be, I think we can add a build flag in Makefile and then only run arm64 binary build and docker build image verification in GH action.

And also we really need a GH action for arm64 test.

[mayankshah1607]

@andyzhangx It works now. I added the following line instead:

docker run --rm --privileged multiarch/qemu-user-static --reset -p yes

[andyzhangx]

could you rebase this PR @mayankshah1607 thanks.

[k8s-ci-robot]

@mayankshah1607: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[mayankshah1607]

Following up with the discussion on Slack regarding multi arch builds, we should move this to build in GCB, and not Prow directly.

the prow jobs share a kernel and binfmt_misc is global / non-namespaced

Should we add the relevant build commands in a cloudbuild.yaml in this repo?

[andyzhangx]

Following up with the discussion on Slack regarding multi arch builds, we should move this to build in GCB, and not Prow directly.

the prow jobs share a kernel and binfmt_misc is global / non-namespaced

Should we add the relevant build commands in a cloudbuild.yaml in this repo?

@mayankshah1607 since it already works, let's merge this PR first. I believe it would also benefit other CSI drivers too.

[andyzhangx]

and then continue with #28

[mayankshah1607]

Had some issues rebasing this branch after #141 was merged. Opening a new PR #144 and closing this one

[BenTheElder]

re: dind ... Prow is dind, but see: https://kubernetes.slack.com/archives/C09QZ4DQB/p1604941946039700?thread_ts=1604921217.035400&cid=C09QZ4DQB
GCB is not dind. (it is docker socket mounted to a container though, which is different, containers you create will be on the host adjacent to the build env)