Skip to content
This repository has been archived by the owner on Dec 15, 2021. It is now read-only.

cmd,pkg: fix nil dereference when running kubeless install #41

Merged

Conversation

dongsupark
Copy link

kubeless install command can panic when no kubernetes cluster is available, like the following:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x45eb0d]

goroutine 1 [running]:
panic(0x1adc6e0, 0xc420010250)
        /usr/local/golang/src/runtime/panic.go:500 +0x1a1
github.com/skippbox/kubeless/cmd.newControllerConfig(0x0, 0x0, 0x1cce16e, 0x7, 0x0, 0x0, 0x0, 0x0, 0x0)
        /go/src/github.com/skippbox/kubeless/cmd/root.go:75 +0x1bd
github.com/skippbox/kubeless/cmd.glob..func5(0x2b87a00, 0x2bfa458, 0x0, 0x0)
        /go/src/github.com/skippbox/kubeless/cmd/install.go:41 +0x207
github.com/skippbox/kubeless/vendor/github.com/spf13/cobra.(*Command).execute(0x2b87a00, 0x2bfa458, 0x0, 0x0, 0x2b87a00, 0x2bfa458)
        /go/src/github.com/skippbox/kubeless/vendor/github.com/spf13/cobra/command.go:636 +0x443
github.com/skippbox/kubeless/vendor/github.com/spf13/cobra.(*Command).ExecuteC(0x2b88280, 0xc42007e058, 0x0, 0xc42065ff08)
        /go/src/github.com/skippbox/kubeless/vendor/github.com/spf13/cobra/command.go:722 +0x367
github.com/skippbox/kubeless/vendor/github.com/spf13/cobra.(*Command).Execute(0x2b88280, 0x0, 0x0)
        /go/src/github.com/skippbox/kubeless/vendor/github.com/spf13/cobra/command.go:681 +0x2b
github.com/skippbox/kubeless/cmd.Execute()
        /go/src/github.com/skippbox/kubeless/cmd/root.go:48 +0x31
main.main()
        /go/src/github.com/skippbox/kubeless/main.go:22 +0x14

Fix it by setting masterHost only when k8sConfig is not nil. Otherwise masterHost needs to be an empty string, which will cause later Controller.createTPR() to be skipped in Controller.initResource().

kubeless install command can panic when no kubernetes cluster is
available, like the following:

====
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x45eb0d]

goroutine 1 [running]:
panic(0x1adc6e0, 0xc420010250)
        /usr/local/golang/src/runtime/panic.go:500 +0x1a1
github.com/skippbox/kubeless/cmd.newControllerConfig(0x0, 0x0, 0x1cce16e, 0x7, 0x0, 0x0, 0x0, 0x0, 0x0)
        /go/src/github.com/skippbox/kubeless/cmd/root.go:75 +0x1bd
github.com/skippbox/kubeless/cmd.glob..func5(0x2b87a00, 0x2bfa458, 0x0, 0x0)
        /go/src/github.com/skippbox/kubeless/cmd/install.go:41 +0x207
github.com/skippbox/kubeless/vendor/github.com/spf13/cobra.(*Command).execute(0x2b87a00, 0x2bfa458, 0x0, 0x0, 0x2b87a00, 0x2bfa458)
        /go/src/github.com/skippbox/kubeless/vendor/github.com/spf13/cobra/command.go:636 +0x443
github.com/skippbox/kubeless/vendor/github.com/spf13/cobra.(*Command).ExecuteC(0x2b88280, 0xc42007e058, 0x0, 0xc42065ff08)
        /go/src/github.com/skippbox/kubeless/vendor/github.com/spf13/cobra/command.go:722 +0x367
github.com/skippbox/kubeless/vendor/github.com/spf13/cobra.(*Command).Execute(0x2b88280, 0x0, 0x0)
        /go/src/github.com/skippbox/kubeless/vendor/github.com/spf13/cobra/command.go:681 +0x2b
github.com/skippbox/kubeless/cmd.Execute()
        /go/src/github.com/skippbox/kubeless/cmd/root.go:48 +0x31
main.main()
        /go/src/github.com/skippbox/kubeless/main.go:22 +0x14
====

Fix it by setting masterHost only when k8sConfig is not nil.
Otherwise masterHost needs to be an empty string, which will cause
later Controller.createTPR() to be skipped in Controller.initResource().
@ngtuna
Copy link
Contributor

ngtuna commented Feb 1, 2017

Thanks @dongsupark for the patch. I tested it. It worked at the case of the kube config file is missing. But there is another case need to cover is that the file is there but the cluster is stopping. So it would be great to also warn user to check the availability of cluster in that case. If you wanna to keep working on that I will keep this PR open, otherwise we also be happy to merge it now and open another issue for that case.

@dongsupark
Copy link
Author

@ngtuna Sure, I'll push another fix to cover the case.

@dongsupark
Copy link
Author

@ngtuna My approach for checking if k8s cluster is in the stopping state, is using k8s.io/client-go/1.4/kubernetes etc. To do that, however, first of all we need to vendor k8s.io/client-go, which is currently not vendored in the repo. Without vendoring that, I encountered a strange redefinition issue, just like kubernetes/client-go#19. (simply speaking, vendor inside vendor)

So I need to think more about the approach. I'm going to create another issue about handling client-go.

Can you please merge this PR as-is? Thanks.

@ngtuna
Copy link
Contributor

ngtuna commented Feb 2, 2017

👍 @dongsupark . Adopting k8s.io/client-go would be great.

@ngtuna ngtuna merged commit 40f7e39 into vmware-archive:master Feb 2, 2017
@dongsupark dongsupark deleted the dongsu/fix-panic-install-cmd branch February 2, 2017 14:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants