-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
✨ Update Makefile for go/v3 to support darwin/arm64 #2706
Closed
everettraven
wants to merge
6
commits into
kubernetes-sigs:master
from
everettraven:feat/temp-arm-support
Closed
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
baad2ba
add scripts and modify makefile
everettraven b6d8337
add scripts
everettraven 030b4be
update kustomize build script
everettraven eb48582
update testdata
everettraven 9e9a7da
add license header
everettraven 9f9e6d0
auto build etcd with envtest if platform is darwin/arm64.
everettraven File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| #!/bin/bash | ||
|
|
||
| # Copyright 2022 The Kubernetes Authors. | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
|
|
||
| OS=$(uname -s | awk '{ print tolower($0) }') | ||
| ARCH=$(uname -m) | ||
| echo "Building etcd locally for $OS/$ARCH" | ||
|
|
||
| TEMP_DIR=$(mktemp -d) | ||
| cd $TEMP_DIR | ||
| git clone https://github.com/etcd-io/etcd.git | ||
| cd etcd | ||
| make build | ||
| if [ -d $HOME/bin ] | ||
| then | ||
| mv ./bin/etcd $HOME/bin/etcd | ||
| else | ||
| mkdir $HOME/bin | ||
| mv ./bin/etcd $HOME/bin/etcd | ||
| fi | ||
| cd ../../ | ||
| rm -rf $TEMP_DIR |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| #!/bin/bash | ||
|
|
||
| # Copyright 2022 The Kubernetes Authors. | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| KUSTOMIZE_VERSION=$1 | ||
| INSTALL_PATH=$2 | ||
| if [ ! -f $INSTALL_PATH/kustomize ] | ||
| then | ||
|
|
||
| OS=$(uname -s | awk '{ print tolower($0) }') | ||
| ARCH=$(uname -m) | ||
|
|
||
| echo "Building kustomize $KUSTOMIZE_VERSION locally for $OS/$ARCH" | ||
|
|
||
| TEMP_DIR=$(mktemp -d) | ||
| cd $TEMP_DIR | ||
| git clone --depth 1 --branch kustomize/$KUSTOMIZE_VERSION https://github.com/kubernetes-sigs/kustomize.git | ||
| cd kustomize/kustomize | ||
| GOBIN=$INSTALL_PATH go install . | ||
| cd ../../.. | ||
| rm -rf $TEMP_DIR | ||
| fi |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is hard to keep maintained and not sustainable ( Spaghetti Code )
Also, we cannot do that. We cannot add a script from your repo in the default scaffold.
About etcd bin
The etcd bin is used for ENV TEST and is shipped via the kubebuilder-tools, see:
Also, we are close to having that bin from the project:
As soon we have the binary there we can trigger the job in the cloud to build the kubebuilder-tools for darwin/arm64. To better understand it check the issue: #2664
About kustomize bin
You can follow up: kubernetes-sigs/kustomize#4612
Users will be able to use it with kustomize/v2 and go/v4-alpha, see: #2552
I am not sure if we should assume the decomposability to build kustomize v2 for darwin/arm64 to make it work with go/v3 stable. That will add this kind of complexity to the Makefile and also we would need to properly build it in the cloud as we do with the kubebuilder-tools instead. It seems out of the scope from Kubebuilder itself.
However, we could doc the script for example to allow users to build it by themselves and doc as an option. But the script also needs to leave in the repo or some official place and not in our fork. We would like to have a FAQ section such as we have in SDK for the docs, so I think we could add the info there and link the script. WDYT?
To know more about what is required to support apple silicon and what was done or not so far see: #1932
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/hold
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. I think at least adding some docs for a workaround would be good so that users can begin using Kubebuilder/Operator SDK to develop operators on apple silicon.
Just as an aside, the goal of this PR is to add those scripts to Kubebuilder as well so that the scripts that are run in the makefile are from the official Kubebuilder repo, I just left it pointing to my fork so we could test that it actually works since Kubebuilder doesn't have the scripts in place yet and would modify it to point to Kubebuilder prior to merging.
If this isn't a direction that we want to explore that is fine and I can close this PR, just thought I would give it a shot at trying to provide a solution that would allow development with go/v3 on Apple Silicon. Thanks for the feedback!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we have some options to address these ones, follow some ideas/suggestion:
Regards the bins used in kubebuilder-tools/env test:
a) We could Improve the kubebuilder-tools scripts for if the bin does not exist we build them for the arch informed so that it can work in the cloud and can be used by the authors. Also, we can improve the doc: https://book.kubebuilder.io/reference/artifacts.html and let the users know about that and that they can use it to build for any platform
Regards kustomize
Regards SDK
After we add the changes here in SDK we can use/ link and redirect users to not need to duplicate the content and be required to keep maintained two places
WDYT? Is that make sense?
Could you shape your scripts so we provide them to the users via the FAQ? Would you like to contribute with the kubebuilder-tools scripts to make them be able to build the project when the bin is not found?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of that makes sense to me. I may need a bit more clarification as I start to do some of those things. I would be happy to help out in any way that I can.
As I have time, I can begin working on updating the kubebuilder-tools scripts to build the tool if necessary. Then I think we can potentially discuss more regarding kustomize and and SDK as work on those solutions progresses.
I will go ahead and close this PR. Thanks @camilamacedo86 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, if you wish we can track issues with that as well.
and by sure .. feel free to ping if you need any help