[BOP-39] Add arg checking #2
Conversation
| ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) | ||
|
|
||
| mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ | ||
| config, err := ctrl.GetConfig() |
There was a problem hiding this comment.
This still prints a large stack trace when it fails
|
@nwneisen To fulfill the original ticket, wouldn't these changes be more appropriate in the boundless-cli repository? There is no way to pass these args using |
| // checkArgs checks that the arguments are valid | ||
| func checkArgs(args *args) { | ||
| // Check that the metrics port is available | ||
| ln, err := net.Listen("tcp", ":"+args.metricsAddr) |
There was a problem hiding this comment.
I don't think we need to check if the port is available here. I think these ports are internal container ports. What do you think?
There was a problem hiding this comment.
I added the check just because I was getting a stack trace on my system due to it already being used and figured we could provide something nicer
2023-10-31T12:53:45-06:00 INFO controller-runtime.metrics Metrics server is starting to listen {"addr": ":8080"}
2023-10-31T12:53:45-06:00 ERROR controller-runtime.metrics metrics server failed to listen. You may want to disable the metrics server or use another port if it is due to conflicts {"error": "error listening on :8080: listen tcp :8080: bind: address already in use"}
sigs.k8s.io/controller-runtime/pkg/metrics.NewListener
/home/nneisen/.gvm/pkgsets/go1.20.5/global/pkg/mod/sigs.k8s.io/[email protected]/pkg/metrics/listener.go:48
sigs.k8s.io/controller-runtime/pkg/manager.New
/home/nneisen/.gvm/pkgsets/go1.20.5/global/pkg/mod/sigs.k8s.io/[email protected]/pkg/manager/manager.go:407
main.main
/home/nneisen/code/boundless-operator/main.go:57
runtime.main
/home/nneisen/.gvm/gos/go1.20.5/src/runtime/proc.go:250
2023-10-31T12:53:45-06:00 ERROR setup unable to start manager {"error": "error listening on :8080: listen tcp :8080: bind: address already in use"}
main.main
/home/nneisen/code/boundless-operator/main.go:77
runtime.main
/home/nneisen/.gvm/gos/go1.20.5/src/runtime/proc.go:250
There was a problem hiding this comment.
Using setupLog.Error() still gives me a stack trace. I originally tested with fmt.Printf and allows for a one line suggestion on how to fix it
|
@ranyodh Agreed. I'm still getting familiar with where everything is. I'll move these changes over to the cli repo |
https://mirantis.jira.com/browse/BOP-39
What this PR does
The original ticket is to make running
bocliwithout args print the usage message. There are default values that are used and the actual errors are when these default values do not work. Instead, I'm adding checking around the args so that a clearer message can be given.Checks the args that we specifically add to make sure they are valid. This is really just checking that the ports are available and giving a cleaner message if they aren't. I'd like to also do some checks on the kubeconfig but I'm still looking at doing this through the ctrl package.
Testing
I tested these by running
bocliand finding whenever there is an error and handling it with a cleaner message.