Add alternative names for the server binaries to hyperkube #41017
Conversation
|
This change is |
k8s-ci-robot
added
the
cncf-cla: yes
k8s-github-robot
added
size/M
| @@ -76,8 +78,11 @@ RUN ln -s /hyperkube /apiserver \ | |||
| && ln -s /hyperkube /kubectl \ | |||
| && ln -s /hyperkube /kubelet \ | |||
There was a problem hiding this comment.
while at it, may be add symlinks in /usr/local/bin for these two also? It's kind of annoying these aren't in the path...
| @@ -27,8 +27,10 @@ func NewKubeAPIServer() *Server { | |||
| s := options.NewServerRunOptions() | |||
|
|
|||
| hks := Server{ | |||
| SimpleUsage: "apiserver", | |||
| Long: "The main API entrypoint and interface to the storage system. The API server is also the focal point for all authorization decisions.", | |||
| name: "apiserver", | |||
There was a problem hiding this comment.
is it necessary to add name for all of these? doesn't Name() DTRT already?
There was a problem hiding this comment.
It's not 100% required, but I think it's a little bit cleaner than just relying on SimpleUsage for this
There was a problem hiding this comment.
not critical, but if we're going to add names for everything, maybe also add them for the federation- servers and then just simplify Name()?
| @@ -55,7 +55,7 @@ func (hk *HyperKube) AddServer(s *Server) { | |||
| // FindServer will find a specific server named name. | |||
| func (hk *HyperKube) FindServer(name string) (*Server, error) { | |||
| for _, s := range hk.servers { | |||
| if s.Name() == name { | |||
| if s.Name() == name || s.AlternativeName == name { | |||
There was a problem hiding this comment.
it's not 100% correct, but I wonder if just additionally checking for a kube- prefix would be an easier way of accomplishing the same thing.
There was a problem hiding this comment.
Yes, I thought about that as well, but I think the code changes are so small in any case it's worth to do it "the real way"
|
@ixdy Does this look good to you if I fix the nit @errordeveloper mentioned? |
luxas
force-pushed
the
symlink_hyperkube
branch
from
b47fb57 to
6f6ddc0
Compare
| SimpleUsage string // One line description of the server. | ||
| Long string // Longer free form description of the server | ||
| Run serverRunFunc // Run the server. This is not expected to return. | ||
| AlternativeName string |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/approve please squash |
|
[APPROVALNOTIFIER] This PR is APPROVED The following people have approved this PR: luxas, mikedanese Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
k8s-github-robot
added
the
approved
mikedanese
added
the
do-not-merge
|
Also @ixdy has a comment. |
|
The commits are logical, one for modifying May I please add the comment in a future PR if that's important? I'd like to get this in during the weekend in time for v1.6.0-alpha.2 (so kubeadm can be simplified) and I'm going to sleep now :) |
|
I'm fine with addressing my two comments in a separate PR. |
mikedanese
removed
the
do-not-merge
|
Automatic merge from submit-queue (batch tested with PRs 38252, 41122, 36101, 41017, 41264) |
What this PR does / why we need it:
Right now one can't swap a server image to the hyperkube image without touching the
commandfield in the yaml spec, and that's daunting and leading to extra and unnecessary logic for example in kubeadm.This makes the hyperkube image directly swappable, so now
/usr/local/bin/kube-*is a portable first argument (or simplykube-*if there's a shell).Special notes for your reviewer:
Release note:
@jessfraz @thockin @ixdy