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

Change svcat --kube-context to --context to align with kubectl #1821

Closed
carolynvs opened this issue Mar 8, 2018 · 9 comments
Closed

Change svcat --kube-context to --context to align with kubectl #1821

carolynvs opened this issue Mar 8, 2018 · 9 comments
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. svcat
Milestone

Comments

@carolynvs
Copy link
Contributor

The svcat flag --kube-context should be renamed to --context to align kubectl's flags.

@carolynvs carolynvs added help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. svcat labels Mar 8, 2018
@Bubblemelon
Copy link
Contributor

Hi @carolynvs ! Thanks for posting this on Slack 😄 I have not worked on the kubectl code before, any pointers on how I can get started to help?

@carolynvs
Copy link
Contributor Author

@Bubblemelon Feel free to ping me on slack if you have questions on how to get started. I know it's harder to get traction when every question needs to wait for me to read my email. 😀

Here is the code that needs to be changed: https://github.com/kubernetes-incubator/service-catalog/blob/70afb56e632856e8381d93a21137c6b729be3f71/cmd/svcat/main.go#L95

If you haven't installed minikube yet, I recommend starting there and getting comfortable with the basic kubectl commands to list pods, and try out the configuration flags for kubectl, like --context.

https://kubernetes.io/docs/tutorials/stateless-application/hello-minikube/

This issue is changing the svcat code (in this repo) to use a different flag name and description to handle saying that the user wants to use a named context. Right now the flag is called --kube-context in svcat and we want to update the flag to context to match kubectl. The reason why we want to match kubectl, is that svcat is meant to mimic kubectl as much as possible.

Here's more info on contexts which should help you setup a second context and test your changes:

https://kubernetes.io/docs/tasks/access-application-cluster/configure-access-multiple-clusters/

This fix does not require adding new tests, but may require updating any tests that break (I don't anticipate that). To run the tests run make test-unit, or run just the svcat tests directly with go test ./cmd/svcat/...

@carolynvs
Copy link
Contributor Author

@Bubblemelon Is this still something that you'd like to work on? Let me know if you are stuck or have questions! 💖

@carolynvs carolynvs added this to the 1.0.0 milestone Apr 23, 2018
@carolynvs carolynvs added the good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. label Apr 24, 2018
@Bubblemelon
Copy link
Contributor

Hi @carolynvs thanks for the kind reminder! 😅

@Bubblemelon
Copy link
Contributor

When I was doing a walkthrough the dev guide, I encountered this error after following the Build instructions:

[bin/healthcheck] Error 2

This was returned after doing ctrl+c

I located the section in the makefile that was causing it to hang:

# Mount .pkg as pkg so that we save our cached "go build" output files
DOCKER_CMD = docker run --security-opt label:disable --rm -v $(PWD):/go/src/$(SC_PKG) \
  -v $(PWD)/.pkg:/go/pkg --env AZURE_STORAGE_CONNECTION_STRING scbuildimage
scBuildImageTarget = .scBuildImage

Do you know what might be causing this ?

@carolynvs
Copy link
Contributor Author

carolynvs commented Apr 26, 2018

I am not sure what is causing that... 🤔 For now you can run this command to build and test your changes:

go build -o bin/svcat/svcat ./cmd/svcat
go test ./cmd/svcat/...
# ./bin/svcat/svcat to run the binary

@Bubblemelon
Copy link
Contributor

Thanks! Those commands worked 😃

However, make test-unit didn't. The same error [bin/healthcheck] Error 2 was returned and the same code section hung.

Bubblemelon pushed a commit to Bubblemelon/service-catalog that referenced this issue Apr 26, 2018
* resolves the following issue (kubernetes-retired#1821)
  Change svcat --kube-context to --context to align with kubectl
Bubblemelon pushed a commit to Bubblemelon/service-catalog that referenced this issue Apr 26, 2018
Bubblemelon added a commit to Bubblemelon/service-catalog that referenced this issue Apr 26, 2018
@Bubblemelon
Copy link
Contributor

Here are my changes: #1997

@Bubblemelon
Copy link
Contributor

There are 3 commits with the same content because I was trying to change my commit message format to match with previous commits on the master.

Bubblemelon added a commit to Bubblemelon/service-catalog that referenced this issue Apr 27, 2018
MHBauer pushed a commit that referenced this issue Apr 27, 2018
* Change svcat --kube-context to --context to align with kubectl (#1821)

* simply renamed the string kube-context flag as context

* did gofmt -w cmd/svcat/plugin/manifest.go to resolve Travis CI test error

* Renamed svcat --kube-context flag to --context  (#1821)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. svcat
Projects
None yet
Development

No branches or pull requests

2 participants