Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Speedup generating selflinks for list and watch requests #46234

Merged
merged 2 commits into from
May 23, 2017
Merged

Speedup generating selflinks for list and watch requests #46234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
25f0fe1
Pass Context to GenerateLink
wojtek-t May 22, 2017
b4018f7
Pass RequestInfo to GenerateLink
wojtek-t May 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 2 additions & 7 deletions staging/src/k8s.io/apiserver/pkg/endpoints/handlers/namer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ type ScopeNamer interface {
SetSelfLink(obj runtime.Object, url string) error
// GenerateLink creates an encoded URI for a given runtime object that represents the canonical path
// and query.
GenerateLink(req *http.Request, obj runtime.Object) (uri string, err error)
GenerateLink(requestInfo *request.RequestInfo, obj runtime.Object) (uri string, err error)
// GenerateListLink creates an encoded URI for a list that represents the canonical path and query.
GenerateListLink(req *http.Request) (uri string, err error)
}
Expand Down Expand Up @@ -90,12 +90,7 @@ func (n ContextBasedNaming) Name(req *http.Request) (namespace, name string, err
return ns, requestInfo.Name, nil
}

func (n ContextBasedNaming) GenerateLink(req *http.Request, obj runtime.Object) (uri string, err error) {
requestInfo, ok := request.RequestInfoFrom(n.GetContext(req))
if !ok {
return "", fmt.Errorf("missing requestInfo")
}

func (n ContextBasedNaming) GenerateLink(requestInfo *request.RequestInfo, obj runtime.Object) (uri string, err error) {
namespace, name, err := n.ObjectName(obj)
if err == errEmptyName && len(requestInfo.Name) > 0 {
name = requestInfo.Name
Expand Down
51 changes: 40 additions & 11 deletions staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,12 @@ func getResourceHandler(scope RequestScope, getter getterFunc) http.HandlerFunc
scope.err(err, w, req)
return
}
if err := setSelfLink(result, req, scope.Namer); err != nil {
requestInfo, ok := request.RequestInfoFrom(ctx)
if !ok {
scope.err(fmt.Errorf("missing requestInfo"), w, req)
return
}
if err := setSelfLink(result, requestInfo, scope.Namer); err != nil {
scope.err(err, w, req)
return
}
Expand Down Expand Up @@ -321,7 +326,7 @@ func ListResource(r rest.Lister, rw rest.Watcher, scope RequestScope, forceWatch
return
}
trace.Step("Listing from storage done")
numberOfItems, err := setListSelfLink(result, req, scope.Namer)
numberOfItems, err := setListSelfLink(result, ctx, req, scope.Namer)
if err != nil {
scope.err(err, w, req)
return
Expand Down Expand Up @@ -419,7 +424,12 @@ func createHandler(r rest.NamedCreater, scope RequestScope, typer runtime.Object
}
trace.Step("Object stored in database")

if err := setSelfLink(result, req, scope.Namer); err != nil {
requestInfo, ok := request.RequestInfoFrom(ctx)
if !ok {
scope.err(fmt.Errorf("missing requestInfo"), w, req)
return
}
if err := setSelfLink(result, requestInfo, scope.Namer); err != nil {
scope.err(err, w, req)
return
}
Expand Down Expand Up @@ -512,7 +522,12 @@ func PatchResource(r rest.Patcher, scope RequestScope, admit admission.Interface
return
}

if err := setSelfLink(result, req, scope.Namer); err != nil {
requestInfo, ok := request.RequestInfoFrom(ctx)
if !ok {
scope.err(fmt.Errorf("missing requestInfo"), w, req)
return
}
if err := setSelfLink(result, requestInfo, scope.Namer); err != nil {
scope.err(err, w, req)
return
}
Expand Down Expand Up @@ -829,7 +844,12 @@ func UpdateResource(r rest.Updater, scope RequestScope, typer runtime.ObjectType
}
trace.Step("Object stored in database")

if err := setSelfLink(result, req, scope.Namer); err != nil {
requestInfo, ok := request.RequestInfoFrom(ctx)
if !ok {
scope.err(fmt.Errorf("missing requestInfo"), w, req)
return
}
if err := setSelfLink(result, requestInfo, scope.Namer); err != nil {
scope.err(err, w, req)
return
}
Expand Down Expand Up @@ -942,8 +962,13 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope RequestSco
}
} else {
// when a non-status response is returned, set the self link
requestInfo, ok := request.RequestInfoFrom(ctx)
if !ok {
scope.err(fmt.Errorf("missing requestInfo"), w, req)
return
}
if _, ok := result.(*metav1.Status); !ok {
if err := setSelfLink(result, req, scope.Namer); err != nil {
if err := setSelfLink(result, requestInfo, scope.Namer); err != nil {
scope.err(err, w, req)
return
}
Expand Down Expand Up @@ -1045,7 +1070,7 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope RequestSco
} else {
// when a non-status response is returned, set the self link
if _, ok := result.(*metav1.Status); !ok {
if _, err := setListSelfLink(result, req, scope.Namer); err != nil {
if _, err := setListSelfLink(result, ctx, req, scope.Namer); err != nil {
scope.err(err, w, req)
return
}
Expand Down Expand Up @@ -1113,9 +1138,9 @@ func transformDecodeError(typer runtime.ObjectTyper, baseErr error, into runtime

// setSelfLink sets the self link of an object (or the child items in a list) to the base URL of the request
// plus the path and query generated by the provided linkFunc
func setSelfLink(obj runtime.Object, req *http.Request, namer ScopeNamer) error {
func setSelfLink(obj runtime.Object, requestInfo *request.RequestInfo, namer ScopeNamer) error {
// TODO: SelfLink generation should return a full URL?
uri, err := namer.GenerateLink(req, obj)
uri, err := namer.GenerateLink(requestInfo, obj)
if err != nil {
return nil
}
Expand Down Expand Up @@ -1160,7 +1185,7 @@ func checkName(obj runtime.Object, name, namespace string, namer ScopeNamer) err

// setListSelfLink sets the self link of a list to the base URL, then sets the self links
// on all child objects returned. Returns the number of items in the list.
func setListSelfLink(obj runtime.Object, req *http.Request, namer ScopeNamer) (int, error) {
func setListSelfLink(obj runtime.Object, ctx request.Context, req *http.Request, namer ScopeNamer) (int, error) {
if !meta.IsListType(obj) {
return 0, nil
}
Expand All @@ -1172,11 +1197,15 @@ func setListSelfLink(obj runtime.Object, req *http.Request, namer ScopeNamer) (i
if err := namer.SetSelfLink(obj, uri); err != nil {
glog.V(4).Infof("Unable to set self link on object: %v", err)
}
requestInfo, ok := request.RequestInfoFrom(ctx)
if !ok {
return 0, fmt.Errorf("missing requestInfo")
}

count := 0
err = meta.EachListItem(obj, func(obj runtime.Object) error {
count++
return setSelfLink(obj, req, namer)
return setSelfLink(obj, requestInfo, namer)
})
return count, err
}
Expand Down
4 changes: 2 additions & 2 deletions staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,11 @@ func (p *testNamer) SetSelfLink(obj runtime.Object, url string) error {
}

// GenerateLink creates a path and query for a given runtime object that represents the canonical path.
func (p *testNamer) GenerateLink(req *http.Request, obj runtime.Object) (uri string, err error) {
func (p *testNamer) GenerateLink(requestInfo *request.RequestInfo, obj runtime.Object) (uri string, err error) {
return "", errors.New("not implemented")
}

// GenerateLink creates a path and query for a list that represents the canonical path.
// GenerateListLink creates a path and query for a list that represents the canonical path.
func (p *testNamer) GenerateListLink(req *http.Request) (uri string, err error) {
return "", errors.New("not implemented")
}
Expand Down
10 changes: 9 additions & 1 deletion staging/src/k8s.io/apiserver/pkg/endpoints/handlers/watch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/watch"
"k8s.io/apiserver/pkg/endpoints/handlers/negotiation"
"k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/server/httplog"
"k8s.io/apiserver/pkg/util/wsstream"

Expand Down Expand Up @@ -88,6 +89,13 @@ func serveWatch(watcher watch.Interface, scope RequestScope, req *http.Request,
mediaType += ";stream=watch"
}

ctx := scope.ContextFunc(req)
requestInfo, ok := request.RequestInfoFrom(ctx)
if !ok {
scope.err(fmt.Errorf("missing requestInfo"), w, req)
return
}

server := &WatchServer{
Watching: watcher,
Scope: scope,
Expand All @@ -98,7 +106,7 @@ func serveWatch(watcher watch.Interface, scope RequestScope, req *http.Request,
Encoder: encoder,
EmbeddedEncoder: embeddedEncoder,
Fixup: func(obj runtime.Object) {
if err := setSelfLink(obj, req, scope.Namer); err != nil {
if err := setSelfLink(obj, requestInfo, scope.Namer); err != nil {
utilruntime.HandleError(fmt.Errorf("failed to set link for object %v: %v", reflect.TypeOf(obj), err))
}
},
Expand Down