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

arm64 release binaries for CI and Krew #1148

Merged
merged 9 commits into from Apr 23, 2023
Merged

arm64 release binaries for CI and Krew #1148

merged 9 commits into from Apr 23, 2023

Conversation

HollowMan6
Copy link
Contributor

@HollowMan6 HollowMan6 commented Mar 14, 2023

Resolve #195 #1112

Overview

Add arm64 release binaries (both Linux and macOS) to release CI: kubescape-arm64-<system>, as well as Krew.

macOS arm64 tests are not available in the CI.

Binaries to try out: https://github.com/HollowMan6/kubescape/releases/tag/v2.2.5

Makefile Show resolved Hide resolved
@craigbox
Copy link
Contributor

craigbox commented Mar 21, 2023

$ file ./kubescape-arm64-macos-latest  version
./kubescape-arm64-macos-latest: Mach-O 64-bit executable arm64

$ ./kubescape-arm64-macos-latest version
Your current version is: v2.2.5 [git enabled in build: true]

Spot test with a cluster scan worked fine.

@HollowMan6
Copy link
Contributor Author

Just setup a mac VM on my intel machine, I can't get kubescape-arm64-macos-latest running on that, so this should be m1 specific and I think everything should be OK.

Screenshot from 2023-03-20 21-48-48

@craigbox
Copy link
Contributor

Is it painfully slow to build with qemu?
Does it only build on a release, and not on every PR?

@HollowMan6
Copy link
Contributor Author

HollowMan6 commented Mar 21, 2023

Is it painfully slow to build with qemu?

It's very slow indeed. But I guess we have no choice because there's no cross build option available for libgit2 for linux (not like the macOS one) unfortunately.

Does it only build on a release, and not on every PR?

I will try to merge these code into b-binary-build-and-e2e-tests.yaml

@craigbox
Copy link
Contributor

Is it painfully slow to build with qemu?

It's very slow indeed. But I guess we have no choice because there's no cross build option available for libgit2 for linux (not like the macOS one) unfortunately.

Does it only build on a release, and not on every PR?

I will try to merge these code into b-binary-build-and-e2e-tests.yaml

The question was more: almost nothing we do in Kubescape is architecture specific, and very little of it is OS specific. Is it worth doing tests on arm64 as well as Intel for Mac? Or for that matter, is it worth testing for Linux, Windows and Mac on every build? (Is it OK to do so because they happen in parallel and there is no cost?)

I want to optimize for "tests take the least amount of time to run". To that end, I wonder if testing just on one platform, one build is enough. If a PR looks like it touches something architecture or OS specific, a maintainer then could call for it to have the full suite of tests before merging.

Just looking at the time taken to complete the checks on this PR, I see wild variation in time taken: arm64 is quicker on Mac, but building arm64 for Ubuntu took almost two hours.

We could then make sure that the process of kicking off a release tests all combinations of OS and platform.

@matthyx @dwertent to tell me if this is even relevant.

@HollowMan6
Copy link
Contributor Author

Yeah, that's more about the design problem. I don't know if testing just on one platform, one build is enough for PRs, but I'm sure if any change is required regarding this, it would be easy for me to implement that change. Please let me know your answers and requirements.

@matthyx
Copy link
Contributor

matthyx commented Mar 22, 2023

@HollowMan6 I think it's enough to test each commit on our reference platform, which would be Linux amd64.
Then only during the release process we would run all tests on all platforms to qualify a release candidate as a release.

@HollowMan6
Copy link
Contributor Author

@HollowMan6 I think it's enough to test each commit on our reference platform, which would be Linux amd64. Then only during the release process we would run all tests on all platforms to qualify a release candidate as a release.

OK, I see, will implement in this way.

@HollowMan6
Copy link
Contributor Author

I think now this is ready to get merged. Please let me know if you have more feedback.

@matthyx
Copy link
Contributor

matthyx commented Mar 24, 2023

@dwertent LGTM, please have a look

install.sh Outdated Show resolved Hide resolved
Signed-off-by: Hollow Man <hollowman@opensuse.org>
Signed-off-by: Hollow Man <hollowman@opensuse.org>
Signed-off-by: Hollow Man <hollowman@opensuse.org>
Signed-off-by: Hollow Man <hollowman@opensuse.org>
Signed-off-by: Hollow Man <hollowman@opensuse.org>
Signed-off-by: Hollow Man <hollowman@opensuse.org>
Signed-off-by: Hollow Man <hollowman@opensuse.org>
Signed-off-by: Hollow Man <hollowman@opensuse.org>
@matthyx
Copy link
Contributor

matthyx commented Apr 17, 2023

cc @dwertent

install.sh Outdated Show resolved Hide resolved
Signed-off-by: Hollow Man <hollowman@opensuse.org>
Copy link
Contributor

@dwertent dwertent left a comment

Choose a reason for hiding this comment

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

Thank you. Well done :)

@dwertent dwertent merged commit d448de1 into kubescape:master Apr 23, 2023
11 checks passed
@YiscahLevySilas1
Copy link
Contributor

YiscahLevySilas1 commented Apr 27, 2023

@HollowMan6 Hi :) Do you know to explain why this workflow failed?
Is it possible we need to add sudo apt-get update before?

@HollowMan6
Copy link
Contributor Author

I guess so. Will open another PR to fix it.

@HollowMan6 Hi :) Do you know to explain why this workflow failed? Is it possible we need to add sudo apt-get update before?

@HollowMan6
Copy link
Contributor Author

PR opened at #1210

@YiscahLevySilas1
Copy link
Contributor

PR opened at #1210

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide ARM64 release binaries
5 participants