From c7beb9078c80f268e4a8a5f8118ecd25614a4ca7 Mon Sep 17 00:00:00 2001 From: nikhiljindal Date: Wed, 3 Feb 2016 14:26:11 -0800 Subject: [PATCH] Updating methods to return error rather than using glog.Fatalf --- cmd/integration/integration.go | 5 ++- cmd/kube-apiserver/app/server.go | 5 ++- examples/apiserver/apiserver.go | 17 ++++--- examples/apiserver/apiserver_test.go | 6 ++- examples/apiserver/server/main.go | 6 ++- pkg/genericapiserver/genericapiserver.go | 6 +-- pkg/genericapiserver/genericapiserver_test.go | 11 ++++- pkg/master/master.go | 11 +++-- pkg/master/master_test.go | 6 ++- test/component/scheduler/perf/util.go | 5 ++- test/integration/auth_test.go | 45 +++++++++++++++---- test/integration/extender_test.go | 5 ++- test/integration/framework/master_utils.go | 12 ++++- test/integration/scheduler_test.go | 16 +++++-- test/integration/secret_test.go | 5 ++- test/integration/service_account_test.go | 5 ++- 16 files changed, 128 insertions(+), 38 deletions(-) diff --git a/cmd/integration/integration.go b/cmd/integration/integration.go index 69318702e10..e021ea2c05e 100644 --- a/cmd/integration/integration.go +++ b/cmd/integration/integration.go @@ -170,7 +170,10 @@ func startComponents(firstManifestURL, secondManifestURL string) (string, string masterConfig.CacheTimeout = 2 * time.Second // Create a master and install handlers into mux. - m := master.New(masterConfig) + m, err := master.New(masterConfig) + if err != nil { + glog.Fatalf("Error in bringing up the master: %v", err) + } handler.delegate = m.Handler // Scheduler diff --git a/cmd/kube-apiserver/app/server.go b/cmd/kube-apiserver/app/server.go index 71cdf66cca4..6e56c9e7211 100644 --- a/cmd/kube-apiserver/app/server.go +++ b/cmd/kube-apiserver/app/server.go @@ -389,7 +389,10 @@ func Run(s *options.APIServer) error { Tunneler: tunneler, } - m := master.New(config) + m, err := master.New(config) + if err != nil { + return err + } m.Run(s.ServerRunOptions) return nil } diff --git a/examples/apiserver/apiserver.go b/examples/apiserver/apiserver.go index 9a61528f75f..e515f349b55 100644 --- a/examples/apiserver/apiserver.go +++ b/examples/apiserver/apiserver.go @@ -17,7 +17,8 @@ limitations under the License. package apiserver import ( - "github.com/golang/glog" + "fmt" + "k8s.io/kubernetes/cmd/libs/go2idl/client-gen/testdata/apis/testgroup/v1" testgroupetcd "k8s.io/kubernetes/examples/apiserver/rest" "k8s.io/kubernetes/pkg/api" @@ -45,24 +46,27 @@ func newStorageDestinations(groupName string, groupMeta *apimachinery.GroupMeta) return &storageDestinations, nil } -func Run() { +func Run() error { config := genericapiserver.Config{ EnableIndex: true, APIPrefix: "/api", APIGroupPrefix: "/apis", Serializer: api.Codecs, } - s := genericapiserver.New(&config) + s, err := genericapiserver.New(&config) + if err != nil { + return fmt.Errorf("Error in bringing up the server: %v", err) + } groupVersion := v1.SchemeGroupVersion groupName := groupVersion.Group groupMeta, err := registered.Group(groupName) if err != nil { - glog.Fatalf("%v", err) + return fmt.Errorf("%v", err) } storageDestinations, err := newStorageDestinations(groupName, groupMeta) if err != nil { - glog.Fatalf("Unable to init etcd: %v", err) + return fmt.Errorf("Unable to init etcd: %v", err) } restStorageMap := map[string]rest.Storage{ "testtypes": testgroupetcd.NewREST(storageDestinations.Get(groupName, "testtype"), s.StorageDecorator()), @@ -76,7 +80,8 @@ func Run() { NegotiatedSerializer: api.Codecs, } if err := s.InstallAPIGroups([]genericapiserver.APIGroupInfo{apiGroupInfo}); err != nil { - glog.Fatalf("Error in installing API: %v", err) + return fmt.Errorf("Error in installing API: %v", err) } s.Run(genericapiserver.NewServerRunOptions()) + return nil } diff --git a/examples/apiserver/apiserver_test.go b/examples/apiserver/apiserver_test.go index 134e356987e..6f74dedd397 100644 --- a/examples/apiserver/apiserver_test.go +++ b/examples/apiserver/apiserver_test.go @@ -35,7 +35,11 @@ var serverIP = "http://localhost:8080" var groupVersion = v1.SchemeGroupVersion func TestRun(t *testing.T) { - go Run() + go func() { + if err := Run(); err != nil { + t.Fatalf("Error in bringing up the server: %v", err) + } + }() if err := waitForApiserverUp(); err != nil { t.Fatalf("%v", err) } diff --git a/examples/apiserver/server/main.go b/examples/apiserver/server/main.go index 19961442fc5..aec0ef0ec90 100644 --- a/examples/apiserver/server/main.go +++ b/examples/apiserver/server/main.go @@ -18,8 +18,12 @@ package main import ( "k8s.io/kubernetes/examples/apiserver" + + "github.com/golang/glog" ) func main() { - apiserver.Run() + if err := apiserver.Run(); err != nil { + glog.Fatalf("Error in bringing up the server: %v", err) + } } diff --git a/pkg/genericapiserver/genericapiserver.go b/pkg/genericapiserver/genericapiserver.go index d80893b49c7..b024fdb618f 100644 --- a/pkg/genericapiserver/genericapiserver.go +++ b/pkg/genericapiserver/genericapiserver.go @@ -378,9 +378,9 @@ func setDefaults(c *Config) { // If the caller wants to add additional endpoints not using the GenericAPIServer's // auth, then the caller should create a handler for those endpoints, which delegates the // any unhandled paths to "Handler". -func New(c *Config) *GenericAPIServer { +func New(c *Config) (*GenericAPIServer, error) { if c.Serializer == nil { - glog.Fatalf("Genericapiserver.New() called with config.Serializer == nil") + return nil, fmt.Errorf("Genericapiserver.New() called with config.Serializer == nil") } setDefaults(c) @@ -435,7 +435,7 @@ func New(c *Config) *GenericAPIServer { s.init(c) - return s + return s, nil } func (s *GenericAPIServer) NewRequestInfoResolver() *apiserver.RequestInfoResolver { diff --git a/pkg/genericapiserver/genericapiserver_test.go b/pkg/genericapiserver/genericapiserver_test.go index e21ddcc95a4..6b7209a8218 100644 --- a/pkg/genericapiserver/genericapiserver_test.go +++ b/pkg/genericapiserver/genericapiserver_test.go @@ -56,7 +56,10 @@ func TestNew(t *testing.T) { config.ProxyTLSClientConfig = &tls.Config{} config.Serializer = api.Codecs - s := New(&config) + s, err := New(&config) + if err != nil { + t.Fatalf("Error in bringing up the server: %v", err) + } // Verify many of the variables match their config counterparts assert.Equal(s.enableLogsSupport, config.EnableLogsSupport) @@ -97,7 +100,11 @@ func TestInstallAPIGroups(t *testing.T) { config.APIGroupPrefix = "/apiGroupPrefix" config.Serializer = api.Codecs - s := New(&config) + s, err := New(&config) + if err != nil { + t.Fatalf("Error in bringing up the server: %v", err) + } + apiGroupMeta := registered.GroupOrDie(api.GroupName) extensionsGroupMeta := registered.GroupOrDie(extensions.GroupName) apiGroupsInfo := []APIGroupInfo{ diff --git a/pkg/master/master.go b/pkg/master/master.go index df7db628f53..ed6b115078f 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -131,12 +131,15 @@ type thirdPartyEntry struct { // Certain config fields will be set to a default value if unset. // Certain config fields must be specified, including: // KubeletClient -func New(c *Config) *Master { +func New(c *Config) (*Master, error) { if c.KubeletClient == nil { - glog.Fatalf("Master.New() called with config.KubeletClient == nil") + return nil, fmt.Errorf("Master.New() called with config.KubeletClient == nil") } - s := genericapiserver.New(c.Config) + s, err := genericapiserver.New(c.Config) + if err != nil { + return nil, err + } m := &Master{ GenericAPIServer: s, @@ -155,7 +158,7 @@ func New(c *Config) *Master { m.NewBootstrapController().Start() } - return m + return m, nil } func (m *Master) InstallAPIs(c *Config) { diff --git a/pkg/master/master_test.go b/pkg/master/master_test.go index b1996bd9604..a6e76d45747 100644 --- a/pkg/master/master_test.go +++ b/pkg/master/master_test.go @@ -89,7 +89,11 @@ func newMaster(t *testing.T) (*Master, *etcdtesting.EtcdTestServer, Config, *ass config.ProxyDialer = func(network, addr string) (net.Conn, error) { return nil, nil } config.ProxyTLSClientConfig = &tls.Config{} - master := New(&config) + master, err := New(&config) + if err != nil { + t.Fatalf("Error in bringing up the master: %v", err) + } + return master, etcdserver, config, assert } diff --git a/test/component/scheduler/perf/util.go b/test/component/scheduler/perf/util.go index 7c5a1d5c190..49b6acaf656 100644 --- a/test/component/scheduler/perf/util.go +++ b/test/component/scheduler/perf/util.go @@ -45,7 +45,10 @@ func mustSetupScheduler() (schedulerConfigFactory *factory.ConfigFactory, destro var m *master.Master masterConfig := framework.NewIntegrationTestMasterConfig() - m = master.New(masterConfig) + m, err := master.New(masterConfig) + if err != nil { + panic("error in brining up the master: " + err.Error()) + } s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { m.Handler.ServeHTTP(w, req) })) diff --git a/test/integration/auth_test.go b/test/integration/auth_test.go index 3c5335650e5..ba7e1525838 100644 --- a/test/integration/auth_test.go +++ b/test/integration/auth_test.go @@ -394,7 +394,10 @@ func TestAuthModeAlwaysAllow(t *testing.T) { // defer s.Close() masterConfig := framework.NewIntegrationTestMasterConfig() - m = master.New(masterConfig) + m, err := master.New(masterConfig) + if err != nil { + t.Fatalf("error in bringing up the master: %v", err) + } transport := http.DefaultTransport previousResourceVersion := make(map[string]float64) @@ -498,7 +501,10 @@ func TestAuthModeAlwaysDeny(t *testing.T) { masterConfig := framework.NewIntegrationTestMasterConfig() masterConfig.Authorizer = apiserver.NewAlwaysDenyAuthorizer() - m = master.New(masterConfig) + m, err := master.New(masterConfig) + if err != nil { + t.Fatalf("error in bringing up the master: %v", err) + } transport := http.DefaultTransport @@ -555,7 +561,10 @@ func TestAliceNotForbiddenOrUnauthorized(t *testing.T) { masterConfig.Authenticator = getTestTokenAuth() masterConfig.Authorizer = allowAliceAuthorizer{} masterConfig.AdmissionControl = admit.NewAlwaysAdmit() - m = master.New(masterConfig) + m, err := master.New(masterConfig) + if err != nil { + t.Fatalf("error in bringing up the master: %v", err) + } previousResourceVersion := make(map[string]float64) transport := http.DefaultTransport @@ -630,7 +639,10 @@ func TestBobIsForbidden(t *testing.T) { masterConfig := framework.NewIntegrationTestMasterConfig() masterConfig.Authenticator = getTestTokenAuth() masterConfig.Authorizer = allowAliceAuthorizer{} - m = master.New(masterConfig) + m, err := master.New(masterConfig) + if err != nil { + t.Fatalf("error in bringing up the master: %v", err) + } transport := http.DefaultTransport @@ -679,7 +691,10 @@ func TestUnknownUserIsUnauthorized(t *testing.T) { masterConfig := framework.NewIntegrationTestMasterConfig() masterConfig.Authenticator = getTestTokenAuth() masterConfig.Authorizer = allowAliceAuthorizer{} - m = master.New(masterConfig) + m, err := master.New(masterConfig) + if err != nil { + t.Fatalf("error in bringing up the master: %v", err) + } transport := http.DefaultTransport @@ -753,7 +768,10 @@ func TestAuthorizationAttributeDetermination(t *testing.T) { masterConfig := framework.NewIntegrationTestMasterConfig() masterConfig.Authenticator = getTestTokenAuth() masterConfig.Authorizer = trackingAuthorizer - m = master.New(masterConfig) + m, err := master.New(masterConfig) + if err != nil { + t.Fatalf("error in bringing up the master: %v", err) + } transport := http.DefaultTransport @@ -822,7 +840,10 @@ func TestNamespaceAuthorization(t *testing.T) { masterConfig := framework.NewIntegrationTestMasterConfig() masterConfig.Authenticator = getTestTokenAuth() masterConfig.Authorizer = a - m = master.New(masterConfig) + m, err := master.New(masterConfig) + if err != nil { + t.Fatalf("error in bringing up the master: %v", err) + } previousResourceVersion := make(map[string]float64) transport := http.DefaultTransport @@ -925,7 +946,10 @@ func TestKindAuthorization(t *testing.T) { masterConfig := framework.NewIntegrationTestMasterConfig() masterConfig.Authenticator = getTestTokenAuth() masterConfig.Authorizer = a - m = master.New(masterConfig) + m, err := master.New(masterConfig) + if err != nil { + t.Fatalf("error in bringing up the master: %v", err) + } previousResourceVersion := make(map[string]float64) transport := http.DefaultTransport @@ -1016,7 +1040,10 @@ func TestReadOnlyAuthorization(t *testing.T) { masterConfig.Authenticator = getTestTokenAuth() masterConfig.Authorizer = a - m = master.New(masterConfig) + m, err := master.New(masterConfig) + if err != nil { + t.Fatalf("error in bringing up the master: %v", err) + } transport := http.DefaultTransport diff --git a/test/integration/extender_test.go b/test/integration/extender_test.go index 4d46221a089..5a1b7a6aee2 100644 --- a/test/integration/extender_test.go +++ b/test/integration/extender_test.go @@ -195,7 +195,10 @@ func TestSchedulerExtender(t *testing.T) { // defer s.Close() masterConfig := framework.NewIntegrationTestMasterConfig() - m = master.New(masterConfig) + m, err := master.New(masterConfig) + if err != nil { + t.Fatalf("error in bringing up the master: %v", err) + } restClient := client.NewOrDie(&client.Config{Host: s.URL, ContentConfig: client.ContentConfig{GroupVersion: testapi.Default.GroupVersion()}}) diff --git a/test/integration/framework/master_utils.go b/test/integration/framework/master_utils.go index 9d91be20d1a..c119daa5a30 100644 --- a/test/integration/framework/master_utils.go +++ b/test/integration/framework/master_utils.go @@ -135,7 +135,11 @@ func startMasterOrDie(masterConfig *master.Config) (*master.Master, *httptest.Se masterConfig.EnableProfiling = true masterConfig.EnableSwaggerSupport = true } - m = master.New(masterConfig) + m, err := master.New(masterConfig) + if err != nil { + glog.Fatalf("error in bringing up the master: %v", err) + } + return m, s } @@ -289,7 +293,11 @@ func StartPods(numPods int, host string, restClient *client.Client) error { func RunAMaster(t *testing.T) (*master.Master, *httptest.Server) { masterConfig := NewMasterConfig() masterConfig.EnableProfiling = true - m := master.New(masterConfig) + m, err := master.New(masterConfig) + if err != nil { + // TODO: Return error. + glog.Fatalf("error in bringing up the master: %v", err) + } s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { m.Handler.ServeHTTP(w, req) diff --git a/test/integration/scheduler_test.go b/test/integration/scheduler_test.go index 21cf958f775..e7c73840d26 100644 --- a/test/integration/scheduler_test.go +++ b/test/integration/scheduler_test.go @@ -62,7 +62,10 @@ func TestUnschedulableNodes(t *testing.T) { // defer s.Close() masterConfig := framework.NewIntegrationTestMasterConfig() - m = master.New(masterConfig) + m, err := master.New(masterConfig) + if err != nil { + t.Fatalf("Error in bringing up the master: %v", err) + } restClient := client.NewOrDie(&client.Config{Host: s.URL, ContentConfig: client.ContentConfig{GroupVersion: testapi.Default.GroupVersion()}}) @@ -286,7 +289,11 @@ func TestMultiScheduler(t *testing.T) { // defer s.Close() masterConfig := framework.NewIntegrationTestMasterConfig() - m = master.New(masterConfig) + m, err := master.New(masterConfig) + if err != nil { + t.Fatalf("Error in bringing up the master: %v", err) + } + /* This integration tests the multi-scheduler feature in the following way: 1. create a default scheduler @@ -461,7 +468,10 @@ func TestAllocatable(t *testing.T) { defer s.Close() masterConfig := framework.NewIntegrationTestMasterConfig() - m = master.New(masterConfig) + m, err := master.New(masterConfig) + if err != nil { + t.Fatalf("Error in bringing up the master: %v", err) + } // 1. create and start default-scheduler restClient := client.NewOrDie(&client.Config{Host: s.URL, ContentConfig: client.ContentConfig{GroupVersion: testapi.Default.GroupVersion()}}) diff --git a/test/integration/secret_test.go b/test/integration/secret_test.go index a1e5e7b2c0f..8c89d868465 100644 --- a/test/integration/secret_test.go +++ b/test/integration/secret_test.go @@ -53,7 +53,10 @@ func TestSecrets(t *testing.T) { // defer s.Close() masterConfig := framework.NewIntegrationTestMasterConfig() - m = master.New(masterConfig) + m, err := master.New(masterConfig) + if err != nil { + t.Fatalf("Error in bringing up the master: %v", err) + } framework.DeleteAllEtcdKeys() client := client.NewOrDie(&client.Config{Host: s.URL, ContentConfig: client.ContentConfig{GroupVersion: testapi.Default.GroupVersion()}}) diff --git a/test/integration/service_account_test.go b/test/integration/service_account_test.go index 278b9be300e..3f8b3850c0b 100644 --- a/test/integration/service_account_test.go +++ b/test/integration/service_account_test.go @@ -408,7 +408,10 @@ func startServiceAccountTestServer(t *testing.T) (*client.Client, client.Config, masterConfig.AdmissionControl = serviceAccountAdmission // Create a master and install handlers into mux. - m = master.New(masterConfig) + m, err := master.New(masterConfig) + if err != nil { + t.Fatalf("Error in bringing up the master: %v", err) + } // Start the service account and service account token controllers tokenController := serviceaccountcontroller.NewTokensController(rootClient, serviceaccountcontroller.TokensControllerOptions{TokenGenerator: serviceaccount.JWTTokenGenerator(serviceAccountKey)})