Skip to content

Commit

Permalink
K8s: Generic watch tests (grafana#90023)
Browse files Browse the repository at this point in the history
  • Loading branch information
@toddtreece
toddtreece authored Jul 5, 2024
1 parent e9fd191 commit 5f9ce12
Show file tree
Hide file tree
Showing 8 changed files with 4,727 additions and 100 deletions.
94 changes: 11 additions & 83 deletions pkg/apiserver/storage/file/file.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import (
"k8s.io/apiserver/pkg/storage/storagebackend/factory"
"k8s.io/client-go/tools/cache"
"k8s.io/klog/v2"

grafanaregistry "github.com/grafana/grafana/pkg/apiserver/registry/generic"
)

const MaxUpdateAttempts = 30
Expand All @@ -37,13 +39,6 @@ var _ storage.Interface = (*Storage)(nil)
// When we upgrade to 1.29
var errResourceVersionSetOnCreate = errors.New("resourceVersion should not be set on objects to be created")

type parsedKey struct {
group string
resource string
namespace string
name string
}

// Storage implements storage.Interface and storage resources as JSON files on disk.
type Storage struct {
root string
Expand Down Expand Up @@ -291,14 +286,14 @@ func (s *Storage) Watch(ctx context.Context, key string, opts storage.ListOption
return nil, apierrors.NewBadRequest(fmt.Sprintf("invalid resource version: %v", err))
}

parsedkey, err := s.convertToParsedKey(key, p)
parsedkey, err := grafanaregistry.ParseKey(key)
if err != nil {
return nil, err
}

var namespace *string
if parsedkey.namespace != "" {
namespace = &parsedkey.namespace
if parsedkey.Namespace != "" {
namespace = &parsedkey.Namespace
}

if (opts.SendInitialEvents == nil && requestedRV == 0) || (opts.SendInitialEvents != nil && *opts.SendInitialEvents) {
Expand Down Expand Up @@ -391,10 +386,15 @@ func (s *Storage) Get(ctx context.Context, key string, opts storage.GetOptions,
// No RV generation locking in single item get since its read from the disk
fpath := s.filePath(key)

rv, err := s.versioner.ParseResourceVersion(opts.ResourceVersion)
if err != nil {
return err
}

// Since it's a get, check if the dir exists and return early as needed
dirname := filepath.Dir(fpath)
if !exists(dirname) {
return apierrors.NewNotFound(s.gr, s.nameFromKey(key))
return storage.NewKeyNotFoundError(key, int64(rv))
}

obj, err := readFile(s.codec, fpath, func() runtime.Object {
Expand All @@ -404,10 +404,6 @@ func (s *Storage) Get(ctx context.Context, key string, opts storage.GetOptions,
if opts.IgnoreNotFound {
return runtime.SetZeroValue(objPtr)
}
rv, err := s.versioner.ParseResourceVersion(opts.ResourceVersion)
if err != nil {
return err
}
return storage.NewKeyNotFoundError(key, int64(rv))
}

Expand Down Expand Up @@ -603,10 +599,6 @@ func (s *Storage) GuaranteedUpdate(

s.rvMutex.Lock()
generatedRV := s.getNewResourceVersion()
if err != nil {
s.rvMutex.Unlock()
return err
}
s.rvMutex.Unlock()

if err := s.versioner.UpdateObject(updatedObj, generatedRV); err != nil {
Expand Down Expand Up @@ -683,70 +675,6 @@ func (s *Storage) nameFromKey(key string) string {
return strings.Replace(key, s.resourcePrefix+"/", "", 1)
}

// While this is an inefficient way to differentiate the ambiguous keys,
// we only need it for initial namespace calculation in watch
// This helps us with watcher tests that don't always set up requestcontext correctly
func (s *Storage) convertToParsedKey(key string, p storage.SelectionPredicate) (*parsedKey, error) {
// NOTE: the following supports the watcher tests that run against v1/pods
// Other than that, there are ambiguities in the key format that only field selector
// when set to use metadata.name can be used to bring clarity in the 3-segment case

// Cases handled below:
// namespace scoped:
// /<group>/<resource>/[<namespace>]/[<name>]
// /<group>/<resource>/[<namespace>]
//
// cluster scoped:
// /<group>/<resource>/[<name>]
// /<group>/<resource>
parts := strings.SplitN(key, "/", 5)
if len(parts) < 3 && s.gr.Group != "" {
return nil, fmt.Errorf("invalid key (expecting at least 2 parts): %s", key)
}

if len(parts) < 2 && s.gr.Group == "" {
return nil, fmt.Errorf("invalid key (expecting at least 1 part): %s", key)
}

// beware this empty "" as the first separated part for the rest of the parsing below
if parts[0] != "" {
return nil, fmt.Errorf("invalid key (expecting leading slash): %s", key)
}

k := &parsedKey{}

// for v1/pods that tests use, Group is empty
if len(parts) > 1 && s.gr.Group == "" {
k.resource = parts[1]
}

if len(parts) > 2 {
// for v1/pods that tests use, Group is empty
if parts[1] == s.gr.Resource {
k.resource = parts[1]
if _, found := p.Field.RequiresExactMatch("metadata.name"); !found {
k.namespace = parts[2]
}
} else {
k.group = parts[1]
k.resource = parts[2]
}
}

if len(parts) > 3 {
// for v1/pods that tests use, Group is empty
if parts[1] == s.gr.Resource {
k.name = parts[3]
} else {
if _, found := p.Field.RequiresExactMatch("metadata.name"); !found {
k.namespace = parts[3]
}
}
}

return k, nil
}

func copyModifiedObjectToDestination(updatedObj runtime.Object, destination runtime.Object) error {
u, err := conversion.EnforcePtr(updatedObj)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/apiserver/storage/file/restoptions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (r *RESTOptionsGetter) GetRESTOptions(resource schema.GroupResource) (gener
DeleteCollectionWorkers: 0,
EnableGarbageCollection: false,
// k8s expects forward slashes here, we'll convert them to os path separators in the storage
ResourcePrefix: "/" + resource.Group + "/" + resource.Resource,
ResourcePrefix: "/group/" + resource.Group + "/resource/" + resource.Resource,
CountMetricPollPeriod: 1 * time.Second,
StorageObjectCountTracker: storageConfig.Config.StorageObjectCountTracker,
}
Expand Down
15 changes: 10 additions & 5 deletions pkg/apiserver/storage/file/watcher_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

"github.com/stretchr/testify/assert"
"k8s.io/apimachinery/pkg/api/apitesting"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand All @@ -24,7 +25,8 @@ import (
"k8s.io/apiserver/pkg/storage"
"k8s.io/apiserver/pkg/storage/storagebackend"
"k8s.io/apiserver/pkg/storage/storagebackend/factory"
storagetesting "k8s.io/apiserver/pkg/storage/testing"

storagetesting "github.com/grafana/grafana/pkg/apiserver/storage/testing"
)

var scheme = runtime.NewScheme()
Expand Down Expand Up @@ -52,7 +54,7 @@ func withDefaults(options *setupOptions, t testing.TB) {
options.newFunc = newPod
options.newListFunc = newPodList
options.prefix = t.TempDir()
options.resourcePrefix = "/pods"
options.resourcePrefix = storagetesting.KeyFunc("", "")
options.groupResource = schema.GroupResource{Resource: "pods"}
}

Expand All @@ -70,7 +72,11 @@ func testSetup(t testing.TB, opts ...setupOption) (context.Context, storage.Inte
config.ForResource(setupOpts.groupResource),
setupOpts.resourcePrefix,
func(obj runtime.Object) (string, error) {
return storage.NamespaceKeyFunc(setupOpts.resourcePrefix, obj)
accessor, err := meta.Accessor(obj)
if err != nil {
return "", err
}
return storagetesting.KeyFunc(accessor.GetNamespace(), accessor.GetName()), nil
},
setupOpts.newFunc,
setupOpts.newListFunc,
Expand All @@ -80,7 +86,7 @@ func testSetup(t testing.TB, opts ...setupOption) (context.Context, storage.Inte
)

// Some tests may start reading before writing
if err := os.MkdirAll(path.Join(setupOpts.prefix, "pods", "test-ns"), fs.ModePerm); err != nil {
if err := os.MkdirAll(path.Join(setupOpts.prefix, storagetesting.KeyFunc("test-ns", "")), fs.ModePerm); err != nil {
return nil, nil, nil, err
}

Expand All @@ -95,7 +101,6 @@ func TestWatch(t *testing.T) {
ctx, store, destroyFunc, err := testSetup(t)
defer destroyFunc()
assert.NoError(t, err)

storagetesting.RunTestWatch(ctx, t, store)
}

Expand Down
Loading

0 comments on commit 5f9ce12

Please sign in to comment.