Skip to content
This repository has been archived by the owner on May 6, 2022. It is now read-only.

fix hyperkube to run from goland. #1984

Merged
merged 2 commits into from
Apr 25, 2018

Conversation

n3wscott
Copy link
Contributor

Until #1983 is fixed, I wanted a way to be able to run the api server in debug mode from goland. This was impossible with the current setup so I fixed it to work.

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 25, 2018
Copy link
Contributor

@carolynvs carolynvs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -164,6 +165,9 @@ func (hk *HyperKube) Run(args []string, stopCh <-chan struct{}) error {

s, err := hk.FindServer(serverName)
if err != nil {
if len(args) > 0 {
goto RunAgain // let's keep trying.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: May want to clarify that what we are really doing is chopping off the left-most os arg and trying with the remaining args to deal with varying entry points.

@@ -164,6 +165,9 @@ func (hk *HyperKube) Run(args []string, stopCh <-chan struct{}) error {

s, err := hk.FindServer(serverName)
if err != nil {
if len(args) > 0 {
goto RunAgain // the first args was popped off at start of Run, try again with new args
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How/Why does this work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you read the helpful comment?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we have to pop off args at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was how this code worked. I, clearly, did not write that part.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the existing code popped off the first item in the array because it represents the entrypoint (e.g. main.go or the name of the binary) and isn't really one of the arguments for the command.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I get it, we're looking for apiserver or controller-manager specifically here. Seems like there's something weird still.

thanks @carolynvs

Copy link
Contributor

@MHBauer MHBauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you do a demo of this goland?

@n3wscott
Copy link
Contributor Author

can you do a demo of this goland?

prob not, but maybe in a few weeks

@jboyd01 jboyd01 added the LGTM2 label Apr 25, 2018
@jboyd01 jboyd01 merged commit 06ffe53 into kubernetes-retired:master Apr 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. LGTM1 LGTM2 size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants