Add minimal docs on how to build and some minor build improvements #2
Conversation
copied from boilerplate Makefile that kubebuilder creates
this might not be the right long-term solution
Makefile
Outdated
|
|
||
| # find or download controller-gen | ||
| # download controller-gen if necessary | ||
| controller-gen: |
There was a problem hiding this comment.
Something along these lines used to be in here -- that's maybe how you found it.
I'm fairly wary of bloating the complexity of this Makefile with any logic beyond target dependencies and straightforward commands.
There are a number of dependencies required to run this package, what do you think about taking the assumption that controller-gen is already installed?
In CONTRIBUTING.md, we can document dependencies:
- kustomize
- kubectl
- controller-gen
See: https://github.com/kubeflow/kfserving/blob/master/docs/DEVELOPER_GUIDE.md#install-requirements
README.md
Outdated
| To build Karpenter from source, please first install the following: | ||
|
|
||
| 1. [go v1.14.4+](https://golang.org/dl/) | ||
| 2. [kubebuilder dependencies](https://book.kubebuilder.io/quick-start.html#prerequisites) |
There was a problem hiding this comment.
I don't think you actually need kubebuilder after the initial generation scripts.
README.md
Outdated
| ## Setting up Container Repository | ||
|
|
||
| ### Using AWS ECR |
There was a problem hiding this comment.
In my experience, if we're trying to pitch this as a k8s community project, staying vendor neutral is pretty important to other developers. I might remove this AWS section or move it to an AWS_DEVELOPER_GUIDE.md
| @@ -8,11 +8,11 @@ COPY go.mod go.mod | |||
| COPY go.sum go.sum | |||
|
|
|||
| # Build src | |||
| RUN go mod download | |||
| RUN GOPROXY=direct go mod download | |||
There was a problem hiding this comment.
Thinking about this -- do you need to put this line in our Makefile as well? AWSK8sTester has abstracted out the build logic to be shared between make and docker. We should consider this if it gets too complex.
There was a problem hiding this comment.
Yeah, I guess we could - at some point it gets a bit too complicated (and plus it's kind of specific to us, or other folks who might be working on this from inside a network where there are going to be proxies in front of stuff). Right now I'm just setting this in my .bashrc.
right now just has info on how to get the dependencies for building
instead document how to get it in project docs
| @@ -4,7 +4,7 @@ apiVersion: kustomize.config.k8s.io/v1beta1 | |||
| kind: Kustomization | |||
| images: | |||
| - name: controller | |||
| newName: 767520670908.dkr.ecr.us-west-2.amazonaws.com/karpenter | |||
| newName: 741206201142.dkr.ecr.us-west-2.amazonaws.com/karpenter | |||
There was a problem hiding this comment.
How about we use the scalability infrastructure repo for this: 197575167141
|
Snapshot successfully published to |
I ran into a few problems trying to get stuff to build - these seemed like the most obvious fixes. I'm definitely not wedded to them (I know
GOPROXY=directisn't always the greatest idea, but I'm not sure of another way to get things working easily when running under our VPN).