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

Add make targets for darwin/arm64 binaries #10202

Merged
merged 34 commits into from
Jan 27, 2021

Conversation

ilya-zuyev
Copy link
Contributor

@ilya-zuyev ilya-zuyev commented Jan 21, 2021

This PR adds Makefile targets to build minikube-darwin-arm64 and e2e-darwin-arm64 binaries

develop@Devs-Mac-mini --- z/minikube ‹ilyaz/add_darwin_arm64_binary› » make minikube-darwin-arm64           
GOOS="darwin" GOARCH="arm64" \
        go build -tags "go_getter_nos3 go_getter_nogcs" -ldflags="-X k8s.io/minikube/pkg/version.version=v1.16.0 -X k8s.io/minikube/pkg/version.isoVersion=v1.16.0 -X k8s.io/minikube/pkg/version.isoPath=minikube/iso -X k8s.io/minikube/pkg/version.gitCommitID="6408c29f0a16e1fa92cf6071ab27c83f643c0c02" -X k8s.io/minikube/pkg/version.storageProvisionerVersion=v4" -a -o out/minikube-darwin-arm64 k8s.io/minikube/cmd/minikube

develop@Devs-Mac-mini --- z/minikube ‹ilyaz/add_darwin_arm64_binary› » lipo -archs out/minikube-darwin-arm64
arm64
develop@Devs-Mac-mini --- z/minikube ‹ilyaz/add_darwin_arm64_binary› » make e2e-darwin-arm64
GOOS="darwin" GOARCH="arm64" go test -ldflags="-X k8s.io/minikube/pkg/version.version=v1.16.0 -X k8s.io/minikube/pkg/version.isoVersion=v1.16.0 -X k8s.io/minikube/pkg/version.isoPath=minikube/iso -X k8s.io/minikube/pkg/version.gitCommitID="6408c29f0a16e1fa92cf6071ab27c83f643c0c02" -X k8s.io/minikube/pkg/version.storageProvisionerVersion=v4" -c k8s.io/minikube/test/integration --tags="integration go_getter_nos3 go_getter_nogcs" -o out/e2e-darwin-arm64

develop@Devs-Mac-mini --- z/minikube ‹ilyaz/add_darwin_arm64_binary› » lipo -archs out/e2e-darwin-arm64     
arm64

Also this PR adds a rule for "fat" darwin binary minikube-darwin containing both x86 and arm64 sections

develop@Devs-Mac-mini --- z/minikube ‹ilyaz/add_darwin_arm64_binary› » make minikube-darwin                                                                 
GOOS="darwin" GOARCH="amd64" \
        go build -tags "go_getter_nos3 go_getter_nogcs" -ldflags="-X k8s.io/minikube/pkg/version.version=v1.16.0 -X k8s.io/minikube/pkg/version.isoVersion=v1.16.0 -X k8s.io/minikube/pkg/version.isoPath=minikube/iso -X k8s.io/minikube/pkg/version.gitCommitID="855d55e4db2d8fdad36f982d36cb7b499260f944" -X k8s.io/minikube/pkg/version.storageProvisionerVersion=v4" -a -o out/minikube-darwin-amd64 k8s.io/minikube/cmd/minikube
GOOS="darwin" GOARCH="arm64" \
        go build -tags "go_getter_nos3 go_getter_nogcs" -ldflags="-X k8s.io/minikube/pkg/version.version=v1.16.0 -X k8s.io/minikube/pkg/version.isoVersion=v1.16.0 -X k8s.io/minikube/pkg/version.isoPath=minikube/iso -X k8s.io/minikube/pkg/version.gitCommitID="855d55e4db2d8fdad36f982d36cb7b499260f944" -X k8s.io/minikube/pkg/version.storageProvisionerVersion=v4" -a -o out/minikube-darwin-arm64 k8s.io/minikube/cmd/minikube
lipo -create out/minikube-darwin-amd64 out/minikube-darwin-arm64 -output out/minikube-darwin
develop@Devs-Mac-mini --- z/minikube ‹ilyaz/add_darwin_arm64_binary› » 

develop@Devs-Mac-mini --- z/minikube ‹ilyaz/add_darwin_arm64_binary› » lipo -archs out/minikube-darwin
x86_64 arm64

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 21, 2021
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 21, 2021
Makefile Outdated
@@ -219,7 +228,8 @@ endif
.PHONY: e2e-linux-amd64 e2e-linux-arm64 e2e-darwin-amd64 e2e-windows-amd64.exe
e2e-linux-amd64: out/e2e-linux-amd64 ## Execute end-to-end testing for Linux 64bit
e2e-linux-arm64: out/e2e-linux-arm64 ## Execute end-to-end testing for Linux ARM 64bit
e2e-darwin-amd64: out/e2e-darwin-amd64 ## Execute end-to-end testing for Darwin 64bit
e2e-darwin-amd64: out/e2e-darwin-amd64 ## Execute end-to-end testing for Darwin x86 64bit
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets fix the comment for all e2e, I think the right comment should be

"build end2end binary for ..." (it doesnt execute the test)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Makefile Outdated
minikube-windows-amd64.exe: out/minikube-windows-amd64.exe ## Build Minikube for Windows 64bit

out/minikube-darwin: out/minikube-darwin-amd64 out/minikube-darwin-arm64
lipo -create $^ -output $@
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this lipo for ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a standard darwin to work with executables.
https://ss64.com/osx/lipo.html
Here it's used to repack binaries for amd64 and arm64 into single executable file

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this affect the binary size ? and would it not work without it ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a pun on "fat binaries", like https://en.wikipedia.org/wiki/Liposuction
(they later got renamed to "universal binaries", making it incomprehensible)

There is actually a similar project for Linux: https://icculus.org/fatelf/
But it never caught on, I guess not the same amount of closed binaries ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i.e. it makes the "files to download" list shorter, but the binaries bigger (twice)

if we did that for all of kubernetes (all arch), it would be 5x the current size

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it. removed

Makefile Outdated
minikube-windows-amd64.exe: out/minikube-windows-amd64.exe ## Build Minikube for Windows 64bit

out/minikube-darwin: out/minikube-darwin-amd64 out/minikube-darwin-arm64
lipo -create $^ -output $@
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a pun on "fat binaries", like https://en.wikipedia.org/wiki/Liposuction
(they later got renamed to "universal binaries", making it incomprehensible)

There is actually a similar project for Linux: https://icculus.org/fatelf/
But it never caught on, I guess not the same amount of closed binaries ?

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 24, 2021
@afbjorklund
Copy link
Collaborator

afbjorklund commented Jan 24, 2021

@ilya-zuyev :
I think you need to remove this new target from "make e2e-cross", until we have upgraded to go1.16 ?

# k8s.io/minikube/cmd/minikube
/usr/local/go/pkg/tool/linux_amd64/link: running gcc failed: exit status 1
/tmp/go-link-870443438/go.o: file not recognized: file format not recognized
collect2: error: ld returned 1 exit status

Note: it has not been released yet, see https://golang.org/doc/devel/release.html

See also https://github.com/kubernetes/kubernetes/blob/master/build/build-image/cross/VERSION

Makefile Outdated
@@ -378,7 +388,7 @@ darwin: minikube-darwin-amd64 ## Build minikube for Darwin 64bit
linux: minikube-linux-amd64 ## Build minikube for Linux 64bit

.PHONY: e2e-cross
e2e-cross: e2e-linux-amd64 e2e-linux-arm64 e2e-darwin-amd64 e2e-windows-amd64.exe ## End-to-end cross test
e2e-cross: e2e-linux-amd64 e2e-linux-arm64 e2e-darwin-amd64 e2e-darwin-arm64 e2e-windows-amd64.exe ## End-to-end cross test
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fails to build, so please revert this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, was testing on go1.16 beta. removed. thanks!

@k8s-ci-robot k8s-ci-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 24, 2021
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 25, 2021
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 26, 2021
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 26, 2021
@ilya-zuyev
Copy link
Contributor Author

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Jan 26, 2021
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 26, 2021
@@ -43,9 +43,10 @@ echo "Verifying ISO exists ..."
make verify-iso

# Build and upload
env BUILD_IN_DOCKER=y \
env BUILD_IN_DOCKER=y BUILD_IMAGE=golang:1.16beta1-buster \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we change in Makefile no need to change both places. so later we can revert only on file

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afbjorklund, ilya-zuyev, medyagh

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

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [afbjorklund,medyagh]

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

@medyagh medyagh merged commit fc330cb into kubernetes:master Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants