-
Notifications
You must be signed in to change notification settings - Fork 136
Enable building kubectl hermetically as part of toolchain setup #229
Conversation
@alex1545 fyi |
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.
Looks awesome. We need tests and docs
strip_prefix = k8s_prefix, | ||
urls = [("https://github.com/kubernetes/kubernetes/archive/%s.tar.gz" % k8s_commit)], | ||
) | ||
http_archive( |
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.
Do we need this?
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.
yes, this is what Jeff Grafton was referring to as problematic with the kubernetes repo and which would be solved by the abandoned cl (kubernetes/kubernetes#71290 (comment))
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.
Just a nit pick. Love this!!
Default Args: | ||
build_srcs: Optional. Set to true to build kubectl from sources. Default: False | ||
k8s_commit: Otional. Commit / release tag at which to build kubectl | ||
from. Default "v1.13.0-beta.1" |
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.
we need an issue to update this when 1.13 is out.
Also we need docs that only = or gt 1.13 is supported
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.
will do docs in upcoming PR to avoid re-triggering CI.
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.
Let’s get docs and tests in please
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chrislovecnm, nlopezgi 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 |
Ready for review now.
This PR enables building kubectl. This is now the default behavior which can be overriden by using a custom kubectl_configure(name="k8s_config" ...) target.
This PR resolves: #227