Skip to content

Commit

Permalink
fix comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@ahutsunshine
ahutsunshine committed Nov 29, 2023
1 parent 7254cb9 commit af5bc9b
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 35 deletions.
6 changes: 3 additions & 3 deletions pkg/features/kube_features.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -977,10 +977,10 @@ const (
UserNamespacesPodSecurityStandards featuregate.Feature = "UserNamespacesPodSecurityStandards"

// owner: @ahutsunshine
// alpha: v1.29
// beta: v1.29
//
// Allows namespace indexer for namespace scope resources in apiserver cache to accelerate list operations.
NamespaceIndex featuregate.Feature = "NamespaceIndex"
StorageNamespaceIndex featuregate.Feature = "StorageNamespaceIndex"
)

func init() {
Expand Down Expand Up @@ -1288,5 +1288,5 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
// ...
HPAScaleToZero: {Default: false, PreRelease: featuregate.Alpha},

NamespaceIndex: {Default: false, PreRelease: featuregate.Alpha},
StorageNamespaceIndex: {Default: true, PreRelease: featuregate.Beta},
}
4 changes: 2 additions & 2 deletions pkg/registry/core/pod/strategy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ func GetAttrs(obj runtime.Object) (labels.Set, fields.Set, error) {
// MatchPod returns a generic matcher for a given label and field selector.
func MatchPod(label labels.Selector, field fields.Selector) storage.SelectionPredicate {
var indexFields = []string{"spec.nodeName"}
if utilfeature.DefaultFeatureGate.Enabled(features.NamespaceIndex) {
if utilfeature.DefaultFeatureGate.Enabled(features.StorageNamespaceIndex) {
indexFields = append(indexFields, "metadata.namespace")
}
return storage.SelectionPredicate{
Expand Down Expand Up @@ -329,7 +329,7 @@ func Indexers() *cache.Indexers {
var indexers = cache.Indexers{
storage.FieldIndex("spec.nodeName"): NodeNameIndexFunc,
}
if utilfeature.DefaultFeatureGate.Enabled(features.NamespaceIndex) {
if utilfeature.DefaultFeatureGate.Enabled(features.StorageNamespaceIndex) {
indexers[storage.FieldIndex("metadata.namespace")] = NamespaceIndexFunc
}
return &indexers
Expand Down
29 changes: 1 addition & 28 deletions staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"fmt"
"net/http"
"reflect"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -747,33 +746,7 @@ func (c *Cacher) listItems(ctx context.Context, listRV uint64, key string, pred
}
return nil, readResourceVersion, "", nil
}
return c.watchCache.WaitUntilFreshAndList(ctx, listRV, matcherIndex(ctx, pred))
}

func matcherIndex(ctx context.Context, pred storage.SelectionPredicate) []storage.MatchValue {
re, _ := request.RequestInfoFrom(ctx)
if re == nil || !re.IsResourceRequest || re.Subresource != "" || !whitelistResource(re.Resource) {
return pred.MatcherIndex()
}
// there multiple pattern for listing pods
// 1. list with namespace: /api/v1/namespace/ns/pods
// 2. list pods without namespace and with --field-selector=metadata.namespace=ns

// #1 case
if re.Namespace != "" {
if _, found := pred.Field.RequiresExactMatch("metadata.namespace"); !found {
pred.Field = fields.OneTermEqualSelector("metadata.namespace", re.Namespace)
}
}
// #2 case
// 2.1 pred.Field will not have "metadata.namespace" field if the request is like /api/v1/pods
// 2.2 pred.Field will have "metadata.namespace" field if the request is like /api/v1/pods?fieldSelector=metadata.namespace=ns
return pred.MatcherIndex()
}

// whitelistResource only allows pod to pass that represents only listing pods will enable namespace indexer
func whitelistResource(resource string) bool {
return strings.ToLower(resource) == "pods"
return c.watchCache.WaitUntilFreshAndList(ctx, listRV, pred.MatcherIndex(ctx))
}

// GetList implements storage.Interface
Expand Down
41 changes: 40 additions & 1 deletion staging/src/k8s.io/apiserver/pkg/storage/selection_predicate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,14 @@ limitations under the License.
package storage

import (
"strings"

"golang.org/x/net/context"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apiserver/pkg/endpoints/request"
)

// AttrFunc returns label and field sets and the uninitialized flag for List or Watch to match.
Expand Down Expand Up @@ -145,9 +149,44 @@ func (s *SelectionPredicate) Empty() bool {
// For any index defined by IndexFields, if a matcher can match only (a subset)
// of objects that return <value> for a given index, a pair (<index name>, <value>)
// wil be returned.
func (s *SelectionPredicate) MatcherIndex() []MatchValue {
func (s *SelectionPredicate) MatcherIndex(ctx context.Context) []MatchValue {
re, _ := request.RequestInfoFrom(ctx)
if re == nil || !re.IsResourceRequest || re.Subresource != "" || !whitelistResource(re.Resource) {
return s.matcherIndex(nil)
}

var metaNamespaceSelector fields.Selector
// case1: list with namespace in url. i.e. /api/v1/namespaces/default/pods or /api/v1/namespaces/default/pods?fieldSelector=metadata.namespace=kube-system
if re.Namespace != "" {
if value, found := s.Field.RequiresExactMatch("metadata.namespace"); !found || value != re.Namespace {
metaNamespaceSelector = fields.OneTermEqualSelector("metadata.namespace", re.Namespace)
}
}
// case2: list all pods with namespace in query params. i.e. /api/v1/pods?fieldSelector=metadata.namespace=default
return s.matcherIndex(metaNamespaceSelector)
}

// whitelistResource only allows pod to pass that represents only listing pods will enable namespace indexer
func whitelistResource(resource string) bool {
return strings.EqualFold(resource, "pods")
}

func (s *SelectionPredicate) matcherIndex(metaNamespaceSelector fields.Selector) []MatchValue {
var result []MatchValue
for _, field := range s.IndexFields {
var (
val string
found bool
)
if metaNamespaceSelector != nil {
val, found = metaNamespaceSelector.RequiresExactMatch(field)
// skip if found metadata.namespace
if found {
result = append(result, MatchValue{IndexName: FieldIndex(field), Value: val})
continue
}
}

if value, ok := s.Field.RequiresExactMatch(field); ok {
result = append(result, MatchValue{IndexName: FieldIndex(field), Value: value})
}
Expand Down
80 changes: 79 additions & 1 deletion staging/src/k8s.io/apiserver/pkg/storage/selection_predicate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package storage

import (
"context"
"errors"
"reflect"
"testing"
Expand All @@ -25,6 +26,7 @@ import (
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apiserver/pkg/endpoints/request"
)

type Ignored struct {
Expand Down Expand Up @@ -127,54 +129,130 @@ func TestSelectionPredicateMatcherIndex(t *testing.T) {
indexLabels []string
indexFields []string
expected []MatchValue
ctx context.Context
}{
"Match nil": {
labelSelector: "name=foo",
fieldSelector: "uid=12345",
indexLabels: []string{"bar"},
indexFields: []string{},
ctx: context.Background(),
expected: nil,
},
"Match field": {
labelSelector: "name=foo",
fieldSelector: "uid=12345",
indexLabels: []string{},
indexFields: []string{"uid"},
ctx: context.Background(),
expected: []MatchValue{{IndexName: FieldIndex("uid"), Value: "12345"}},
},
"Match field for listing namespace pods without metadata.namespace field selector": {
labelSelector: "",
fieldSelector: "",
indexLabels: []string{},
indexFields: []string{"metadata.namespace"},
ctx: request.WithRequestInfo(context.Background(), &request.RequestInfo{
IsResourceRequest: true,
Path: "/api/v1/namespaces/default/pods",
Verb: "list",
APIPrefix: "api",
APIGroup: "",
APIVersion: "v1",
Namespace: "default",
Resource: "pods",
}),
expected: []MatchValue{{IndexName: FieldIndex("metadata.namespace"), Value: "default"}},
},
"Match field for listing namespace pods with metadata.namespace field selector": {
labelSelector: "",
fieldSelector: "metadata.namespace=kube-system",
indexLabels: []string{},
indexFields: []string{"metadata.namespace"},
ctx: request.WithRequestInfo(context.Background(), &request.RequestInfo{
IsResourceRequest: true,
Path: "/api/v1/namespaces/default/pods",
Verb: "list",
APIPrefix: "api",
APIGroup: "",
APIVersion: "v1",
Namespace: "default",
Resource: "pods",
}),
expected: []MatchValue{{IndexName: FieldIndex("metadata.namespace"), Value: "default"}},
},
"Match field for listing all pods without metadata.namespace field selector": {
labelSelector: "",
fieldSelector: "",
indexLabels: []string{},
indexFields: []string{"metadata.namespace"},
ctx: request.WithRequestInfo(context.Background(), &request.RequestInfo{
IsResourceRequest: true,
Path: "/api/v1/pods",
Verb: "list",
APIPrefix: "api",
APIGroup: "",
APIVersion: "v1",
Namespace: "",
Resource: "pods",
}),
expected: nil,
},
"Match field for listing all pods with metadata.namespace field selector": {
labelSelector: "",
fieldSelector: "metadata.namespace=default",
indexLabels: []string{},
indexFields: []string{"metadata.namespace"},
ctx: request.WithRequestInfo(context.Background(), &request.RequestInfo{
IsResourceRequest: true,
Path: "/api/v1/pods",
Verb: "list",
APIPrefix: "api",
APIGroup: "",
APIVersion: "v1",
Namespace: "default",
Resource: "pods",
}),
expected: []MatchValue{{IndexName: FieldIndex("metadata.namespace"), Value: "default"}},
},
"Match label": {
labelSelector: "name=foo",
fieldSelector: "uid=12345",
indexLabels: []string{"name"},
indexFields: []string{},
ctx: context.Background(),
expected: []MatchValue{{IndexName: LabelIndex("name"), Value: "foo"}},
},
"Match field and label": {
labelSelector: "name=foo",
fieldSelector: "uid=12345",
indexLabels: []string{"name"},
indexFields: []string{"uid"},
ctx: context.Background(),
expected: []MatchValue{{IndexName: FieldIndex("uid"), Value: "12345"}, {IndexName: LabelIndex("name"), Value: "foo"}},
},
"Negative match field and label": {
labelSelector: "name!=foo",
fieldSelector: "uid!=12345",
indexLabels: []string{"name"},
indexFields: []string{"uid"},
ctx: context.Background(),
expected: nil,
},
"Negative match field and match label": {
labelSelector: "name=foo",
fieldSelector: "uid!=12345",
indexLabels: []string{"name"},
indexFields: []string{"uid"},
ctx: context.Background(),
expected: []MatchValue{{IndexName: LabelIndex("name"), Value: "foo"}},
},
"Negative match label and match field": {
labelSelector: "name!=foo",
fieldSelector: "uid=12345",
indexLabels: []string{"name"},
indexFields: []string{"uid"},
ctx: context.Background(),
expected: []MatchValue{{IndexName: FieldIndex("uid"), Value: "12345"}},
},
}
Expand All @@ -194,7 +272,7 @@ func TestSelectionPredicateMatcherIndex(t *testing.T) {
IndexLabels: testCase.indexLabels,
IndexFields: testCase.indexFields,
}
actual := sp.MatcherIndex()
actual := sp.MatcherIndex(testCase.ctx)
if !reflect.DeepEqual(testCase.expected, actual) {
t.Errorf("%v: expected %v, got %v", name, testCase.expected, actual)
}
Expand Down

0 comments on commit af5bc9b

Please sign in to comment.