Get repo into usable shape #62
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chuckha 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:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
approved
|
/assign @fabriziopandini |
|
What process are you using to get the CAPI CRD yaml in here? |
|
Thanks. We have https://github.com/kubernetes-sigs/cluster-api-provider-aws/blob/1148ea317d71ec1eb8a7910127219ce2a65526c1/hack/update-vendor.sh#L30-L36 and https://github.com/kubernetes-sigs/cluster-api-provider-aws/blob/1148ea317d71ec1eb8a7910127219ce2a65526c1/go.vendor in CAPI to do the same thing. Should we converge for consistency? |
|
@ncdc that copies the entire project where this only needs the CRDs. I don't think we need a vendor directory in this project |
|
my other argument is if we start importing cluster-api in all providers like this then why aren't we working in a mono-repo in the first place? (honestly, i'm kind of a fan of the monorepo idea anyway!) |
chuckha
force-pushed
the
capdctl2
branch
2 times, most recently
from
683f97e to
5d4eb19
Compare
| // Find the owner reference | ||
| var machineRef *v1.OwnerReference | ||
| for _, ref := range config.OwnerReferences { | ||
| if ref.Kind == machineKind { | ||
| if fmt.Sprintf("%s, Kind=%s", ref.APIVersion, ref.Kind) == machineKind { |
There was a problem hiding this comment.
Remove the .String() call from machineKind and you can do a normal == comparison.
There was a problem hiding this comment.
hmm, refs don't have a GroupVersionKind though, what am I comparing machineKind with? Wouldn't I have to build it up from the Ref parts anyway?
There was a problem hiding this comment.
My bad (I'll blame it on being sick 😄). I think you can use CAPI's util.HasOwner(config.OwnerReferences, v1alpha2.SchemeGroupVersion.String(), "Machine") instead of this loop. Will that do what you need?
There was a problem hiding this comment.
that seems like a heavy function for this when I just want to compare two strings, does
ref.Kind == machineKind.Kind && ref.APIVersion == machineKind.Version look ok to you? It passes the tests.
The only reason i'm a little hesitant on that function is that it doesn't make the code easier to read whereas i know what's going on quickly comparing a ref and a version.
third_party/forked/README.md
Outdated
| @@ -0,0 +1,3 @@ | |||
| # rerun-process-wrapper | |||
|
|
|||
| Forked from https://github.com/windmilleng/rerun-process-wrapper | |||
There was a problem hiding this comment.
Can you reach out to them to see about a LICENSE for this code?
There was a problem hiding this comment.
Yeah, already done. I suggested Apache 2.0 but we'll see. tilt-dev/rerun-process-wrapper#3
There was a problem hiding this comment.
https://github.com/windmilleng/rerun-process-wrapper
added apache 2.0
There was a problem hiding this comment.
Can you pull in their license file? Maybe also include the git commit that you copied from?
| // Find the owner reference | ||
| var machineRef *v1.OwnerReference | ||
| for _, ref := range config.OwnerReferences { | ||
| if ref.Kind == machineKind { | ||
| if fmt.Sprintf("%s, Kind=%s", ref.APIVersion, ref.Kind) == machineKind { |
There was a problem hiding this comment.
I'd almost rather see two separate comparisons than string munging to a single comparison.
|
lgtm, will leave tagging for @ncdc, though |
|
Commits squashed. Ready for take off! 🚀 |
|
Just 1 thing re third_party license file |
List of the changes: * Ignore the manager-image-patch backup file * Set up dockerfile to allow restarts in non-docker environments * Always use GOPROXY to build * Clean up hacked install controller-runtime-tools * Update cluster-api kustomize config * Clean up rbac * Update tests Signed-off-by: Chuck Ha <chuckh@vmware.com>
|
@ncdc ok, all set, commit added, full repo cloned with README as well. |
|
Thank you!!! |
What this PR does / why we need it:
This PR gets the bootstrap provider into a working state (but not yet generating the cloud init code). Some docker work allows for
Special notes for your reviewer:
The majority of this change set is pulling in cluster-api YAML.
/cc @amy
/assign @detiber @ncdc
Release note: