From debd42a07d49b97b44ae6a3cdb226626f8742038 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Mon, 15 Jun 2015 22:39:31 -0400 Subject: [PATCH] Cleaning up apiserver method signatures A lot of the changes in apiserver could have been represented more cleanly - this returns the signatures to their older behavior (and unbreaks OpenShift). --- pkg/apiserver/api_installer.go | 9 +++++---- pkg/apiserver/apiserver.go | 19 ++++++++++--------- pkg/apiserver/apiserver_test.go | 6 +++--- pkg/apiserver/resthandler.go | 2 +- pkg/apiserver/watch.go | 4 ++-- pkg/master/master.go | 33 ++++++++++++++++++++------------- 6 files changed, 41 insertions(+), 32 deletions(-) diff --git a/pkg/apiserver/api_installer.go b/pkg/apiserver/api_installer.go index 261b34da6da..010aa585d16 100644 --- a/pkg/apiserver/api_installer.go +++ b/pkg/apiserver/api_installer.go @@ -18,12 +18,12 @@ package apiserver import ( "fmt" - "net" "net/http" gpath "path" "reflect" "sort" "strings" + "time" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" @@ -40,7 +40,8 @@ type APIInstaller struct { group *APIGroupVersion info *APIRequestInfoResolver prefix string // Path prefix where API resources are to be registered. - minRequestTimeout int + minRequestTimeout time.Duration + proxyDialerFn ProxyDialerFunc } // Struct capturing information about an action ("GET", "POST", "WATCH", PROXY", etc). @@ -55,13 +56,13 @@ type action struct { var errEmptyName = errors.NewBadRequest("name must be provided") // Installs handlers for API resources. -func (a *APIInstaller) Install(proxyDialer func(network, addr string) (net.Conn, error)) (ws *restful.WebService, errors []error) { +func (a *APIInstaller) Install() (ws *restful.WebService, errors []error) { errors = make([]error, 0) // Create the WebService. ws = a.newWebService() - proxyHandler := (&ProxyHandler{a.prefix + "/proxy/", a.group.Storage, a.group.Codec, a.group.Context, a.info, proxyDialer}) + proxyHandler := (&ProxyHandler{a.prefix + "/proxy/", a.group.Storage, a.group.Codec, a.group.Context, a.info, a.proxyDialerFn}) // Register the paths in a deterministic (sorted) order to get a deterministic swagger spec. paths := make([]string, len(a.group.Storage)) diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index b92a0338d2c..6543eafdadd 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -109,6 +109,7 @@ type Mux interface { // It handles URLs of the form: // /${storage_key}[/${object_name}] // Where 'storage_key' points to a rest.Storage object stored in storage. +// This object should contain all parameterization necessary for running a particular API version type APIGroupVersion struct { Storage map[string]rest.Storage @@ -131,8 +132,13 @@ type APIGroupVersion struct { Admit admission.Interface Context api.RequestContextMapper + + ProxyDialerFn ProxyDialerFunc + MinRequestTimeout time.Duration } +type ProxyDialerFunc func(network, addr string) (net.Conn, error) + // TODO: Pipe these in through the apiserver cmd line const ( // Minimum duration before timing out read/write requests @@ -141,16 +147,10 @@ const ( MaxTimeoutSecs = 600 ) -// restContainer is a wrapper around a generic restful Container that also contains a MinRequestTimeout -type RestContainer struct { - *restful.Container - MinRequestTimeout int -} - // InstallREST registers the REST handlers (storage, watch, proxy and redirect) into a restful Container. // It is expected that the provided path root prefix will serve all operations. Root MUST NOT end // in a slash. A restful WebService is created for the group and version. -func (g *APIGroupVersion) InstallREST(container *RestContainer, proxyDialer func(network, addr string) (net.Conn, error)) error { +func (g *APIGroupVersion) InstallREST(container *restful.Container) error { info := &APIRequestInfoResolver{util.NewStringSet(strings.TrimPrefix(g.Root, "/")), g.Mapper} prefix := path.Join(g.Root, g.Version) @@ -158,9 +158,10 @@ func (g *APIGroupVersion) InstallREST(container *RestContainer, proxyDialer func group: g, info: info, prefix: prefix, - minRequestTimeout: container.MinRequestTimeout, + minRequestTimeout: g.MinRequestTimeout, + proxyDialerFn: g.ProxyDialerFn, } - ws, registrationErrors := installer.Install(proxyDialer) + ws, registrationErrors := installer.Install() container.Add(ws) return errors.NewAggregate(registrationErrors) } diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index 4de719e791a..7e2bc84172d 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -226,7 +226,7 @@ func handleInternal(legacy bool, storage map[string]rest.Storage, admissionContr container := restful.NewContainer() container.Router(restful.CurlyRouter{}) mux := container.ServeMux - if err := group.InstallREST(&RestContainer{container, 0}, nil); err != nil { + if err := group.InstallREST(container); err != nil { panic(fmt.Sprintf("unable to install container %s: %v", group.Version, err)) } ws := new(restful.WebService) @@ -1938,7 +1938,7 @@ func TestParentResourceIsRequired(t *testing.T) { Codec: newCodec, } container := restful.NewContainer() - if err := group.InstallREST(&RestContainer{container, 0}, nil); err == nil { + if err := group.InstallREST(container); err == nil { t.Fatal("expected error") } @@ -1966,7 +1966,7 @@ func TestParentResourceIsRequired(t *testing.T) { Codec: newCodec, } container = restful.NewContainer() - if err := group.InstallREST(&RestContainer{container, 0}, nil); err != nil { + if err := group.InstallREST(container); err != nil { t.Fatal(err) } diff --git a/pkg/apiserver/resthandler.go b/pkg/apiserver/resthandler.go index 6b8b1013185..567a3d837dd 100644 --- a/pkg/apiserver/resthandler.go +++ b/pkg/apiserver/resthandler.go @@ -185,7 +185,7 @@ func ConnectResource(connecter rest.Connecter, scope RequestScope, admit admissi } // ListResource returns a function that handles retrieving a list of resources from a rest.Storage object. -func ListResource(r rest.Lister, rw rest.Watcher, scope RequestScope, forceWatch bool, minRequestTimeout int) restful.RouteFunction { +func ListResource(r rest.Lister, rw rest.Watcher, scope RequestScope, forceWatch bool, minRequestTimeout time.Duration) restful.RouteFunction { return func(req *restful.Request, res *restful.Response) { w := res.ResponseWriter diff --git a/pkg/apiserver/watch.go b/pkg/apiserver/watch.go index 1be779e3611..9107f0c3abd 100644 --- a/pkg/apiserver/watch.go +++ b/pkg/apiserver/watch.go @@ -66,11 +66,11 @@ func (w *realTimeoutFactory) TimeoutCh() (<-chan time.Time, func() bool) { } // serveWatch handles serving requests to the server -func serveWatch(watcher watch.Interface, scope RequestScope, w http.ResponseWriter, req *restful.Request, minRequestTimeout int) { +func serveWatch(watcher watch.Interface, scope RequestScope, w http.ResponseWriter, req *restful.Request, minRequestTimeout time.Duration) { var timeout time.Duration if minRequestTimeout > 0 { // Each watch gets a random timeout between minRequestTimeout and 2*minRequestTimeout to avoid thundering herds. - timeout = time.Duration(minRequestTimeout+rand.Intn(minRequestTimeout)) * time.Second + timeout = time.Duration(float64(minRequestTimeout) * (rand.Float64() + 1.0)) } watchServer := &WatchServer{watcher, scope.Codec, func(obj runtime.Object) { if err := setSelfLink(obj, req, scope.Namer); err != nil { diff --git a/pkg/master/master.go b/pkg/master/master.go index adf0685d2b2..d495e3e1034 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -117,7 +117,7 @@ type Config struct { RestfulContainer *restful.Container // If specified, requests will be allocated a random timeout between this value, and twice this value. - // Note that it is up to the request handlers to ignore or honor this timeout. + // Note that it is up to the request handlers to ignore or honor this timeout. In seconds. MinRequestTimeout int // Number of masters running; all masters must be started with the @@ -163,10 +163,11 @@ type Master struct { serviceClusterIPRange *net.IPNet serviceNodePortRange util.PortRange cacheTimeout time.Duration + minRequestTimeout time.Duration mux apiserver.Mux muxHelper *apiserver.MuxHelper - handlerContainer *apiserver.RestContainer + handlerContainer *restful.Container rootWebService *restful.WebService enableCoreControllers bool enableLogsSupport bool @@ -210,6 +211,7 @@ type Master struct { InsecureHandler http.Handler // Used for secure proxy + dialer apiserver.ProxyDialerFunc tunnels *util.SSHTunnelList tunnelsLock sync.Mutex installSSHKey InstallSSHKey @@ -333,7 +335,8 @@ func New(c *Config) *Master { v1: !c.DisableV1, requestContextMapper: c.RequestContextMapper, - cacheTimeout: c.CacheTimeout, + cacheTimeout: c.CacheTimeout, + minRequestTimeout: time.Duration(c.MinRequestTimeout) * time.Second, masterCount: c.MasterCount, externalHost: c.ExternalHost, @@ -355,7 +358,7 @@ func New(c *Config) *Master { m.mux = mux handlerContainer = NewHandlerContainer(mux) } - m.handlerContainer = &apiserver.RestContainer{handlerContainer, c.MinRequestTimeout} + m.handlerContainer = handlerContainer // Use CurlyRouter to be able to use regular expressions in paths. Regular expressions are required in paths for example for proxy (where the path is proxy/{kind}/{name}/{*}) m.handlerContainer.Router(restful.CurlyRouter{}) m.muxHelper = &apiserver.MuxHelper{m.mux, []string{}} @@ -493,9 +496,10 @@ func (m *Master) init(c *Config) { "componentStatuses": componentstatus.NewStorage(func() map[string]apiserver.Server { return m.getServersToValidate(c) }), } - var proxyDialer func(net, addr string) (net.Conn, error) + // establish the node proxy dialer if len(c.SSHUser) > 0 { glog.Infof("Setting up proxy: %s %s", c.SSHUser, c.SSHKeyfile) + // public keyfile is written last, so check for that. publicKeyFile := c.SSHKeyfile + ".pub" exists, err := util.FileExists(publicKeyFile) @@ -509,14 +513,14 @@ func (m *Master) init(c *Config) { } } m.tunnels = &util.SSHTunnelList{} + m.dialer = m.Dial m.setupSecureProxy(c.SSHUser, c.SSHKeyfile, publicKeyFile) - proxyDialer = m.Dial // This is pretty ugly. A better solution would be to pull this all the way up into the // server.go file. httpKubeletClient, ok := c.KubeletClient.(*client.HTTPKubeletClient) if ok { - httpKubeletClient.Config.Dial = m.Dial + httpKubeletClient.Config.Dial = m.dialer transport, err := client.MakeTransport(httpKubeletClient.Config) if err != nil { glog.Errorf("Error setting up transport over SSH: %v", err) @@ -530,29 +534,29 @@ func (m *Master) init(c *Config) { apiVersions := []string{} if m.v1beta3 { - if err := m.api_v1beta3().InstallREST(m.handlerContainer, proxyDialer); err != nil { + if err := m.api_v1beta3().InstallREST(m.handlerContainer); err != nil { glog.Fatalf("Unable to setup API v1beta3: %v", err) } apiVersions = append(apiVersions, "v1beta3") } if m.v1 { - if err := m.api_v1().InstallREST(m.handlerContainer, proxyDialer); err != nil { + if err := m.api_v1().InstallREST(m.handlerContainer); err != nil { glog.Fatalf("Unable to setup API v1: %v", err) } apiVersions = append(apiVersions, "v1") } apiserver.InstallSupport(m.muxHelper, m.rootWebService) - apiserver.AddApiWebService(m.handlerContainer.Container, c.APIPrefix, apiVersions) + apiserver.AddApiWebService(m.handlerContainer, c.APIPrefix, apiVersions) defaultVersion := m.defaultAPIGroupVersion() requestInfoResolver := &apiserver.APIRequestInfoResolver{util.NewStringSet(strings.TrimPrefix(defaultVersion.Root, "/")), defaultVersion.Mapper} - apiserver.InstallServiceErrorHandler(m.handlerContainer.Container, requestInfoResolver, apiVersions) + apiserver.InstallServiceErrorHandler(m.handlerContainer, requestInfoResolver, apiVersions) // Register root handler. // We do not register this using restful Webservice since we do not want to surface this in api docs. // Allow master to be embedded in contexts which already have something registered at the root if c.EnableIndex { - m.mux.HandleFunc("/", apiserver.IndexHandler(m.handlerContainer.Container, m.muxHelper)) + m.mux.HandleFunc("/", apiserver.IndexHandler(m.handlerContainer, m.muxHelper)) } if c.EnableLogsSupport { @@ -677,7 +681,7 @@ func (m *Master) InstallSwaggerAPI() { SwaggerPath: "/swaggerui/", SwaggerFilePath: "/swagger-ui/", } - swagger.RegisterSwaggerService(swaggerConfig, m.handlerContainer.Container) + swagger.RegisterSwaggerService(swaggerConfig, m.handlerContainer) } func (m *Master) getServersToValidate(c *Config) map[string]apiserver.Server { @@ -723,6 +727,9 @@ func (m *Master) defaultAPIGroupVersion() *apiserver.APIGroupVersion { Admit: m.admissionControl, Context: m.requestContextMapper, + + ProxyDialerFn: m.dialer, + MinRequestTimeout: m.minRequestTimeout, } }