diff --git a/internal/controllers/reconciliation/controller_test.go b/internal/controllers/reconciliation/controller_test.go index 9d33835b..3694826b 100644 --- a/internal/controllers/reconciliation/controller_test.go +++ b/internal/controllers/reconciliation/controller_test.go @@ -24,6 +24,8 @@ import ( // TODO: Cover no-op update, assert on exact k8s api requests +// TODO: Why are we sending strategic patches for CRs? Why does it work? + type crudTestCase struct { Name string Empty, Initial, Updated client.Object diff --git a/internal/controllers/reconciliation/discoverycache.go b/internal/controllers/reconciliation/discoverycache.go index bd00a6ac..b0331817 100644 --- a/internal/controllers/reconciliation/discoverycache.go +++ b/internal/controllers/reconciliation/discoverycache.go @@ -42,31 +42,25 @@ func (d *discoveryCache) Get(ctx context.Context, gvk schema.GroupVersionKind) ( d.mut.Lock() defer d.mut.Unlock() - if d.current == nil { - if err := d.fillUnlocked(ctx); err != nil { - return nil, err + for i := 0; i < 2; i++ { + if d.current == nil { + logger.V(1).Info("filling discovery cache") + if err := d.fillUnlocked(ctx); err != nil { + return nil, err + } } - } - model := d.current.LookupResource(gvk) - if model != nil { + model := d.current.LookupResource(gvk) + if model == nil && d.fillWhenNotFound { + d.current = nil + continue + } return d.checkSupportUnlocked(ctx, gvk, model) } - - if !d.fillWhenNotFound { - logger.V(1).Info("type not found in openapi schema") - return nil, nil - } - - if err := d.fillUnlocked(ctx); err != nil { - return nil, err - } - return d.checkSupportUnlocked(ctx, gvk, d.current.LookupResource(gvk)) + return nil, nil } func (d *discoveryCache) fillUnlocked(ctx context.Context) error { - logr.FromContextOrDiscard(ctx).V(1).Info("filling discovery cache") - doc, err := d.client.OpenAPISchema() if err != nil { return err @@ -82,11 +76,7 @@ func (d *discoveryCache) fillUnlocked(ctx context.Context) error { func (d *discoveryCache) checkSupportUnlocked(ctx context.Context, gvk schema.GroupVersionKind, model proto.Schema) (proto.Schema, error) { logger := logr.FromContextOrDiscard(ctx) if model == nil { - if d.fillWhenNotFound { - logger.V(0).Info("type not found in openapi schema after refresh - this is unexpected and suspicious") - } else { - logger.V(1).Info("type not found in openapi schema") - } + logger.V(1).Info("type not found in openapi schema") return nil, nil } diff --git a/internal/controllers/reconciliation/discoverycache_test.go b/internal/controllers/reconciliation/discoverycache_test.go index a2380b91..831bc7bf 100644 --- a/internal/controllers/reconciliation/discoverycache_test.go +++ b/internal/controllers/reconciliation/discoverycache_test.go @@ -11,10 +11,9 @@ import ( "k8s.io/client-go/discovery/fake" ) -func TestDiscoveryCacheTypeMissingInitially(t *testing.T) { - t.Skip("TODO") +func TestDiscoveryCacheRefill(t *testing.T) { client := &fakeDiscovery{} - d := &discoveryCache{client: client} + d := &discoveryCache{client: client, fillWhenNotFound: true} gvk := schema.GroupVersionKind{ Group: "test-group", @@ -22,33 +21,45 @@ func TestDiscoveryCacheTypeMissingInitially(t *testing.T) { Kind: "TestKind1", } - t.Run("missing from spec", func(t *testing.T) { - s, err := d.Get(context.Background(), gvk) - require.EqualError(t, err, "resource was not found in openapi spec") - assert.Nil(t, s) - }) - - t.Run("added to spec after initial cache fill", func(t *testing.T) { - client.Schema = &openapi_v2.Document{} // TODO - - s, err := d.Get(context.Background(), gvk) - require.NoError(t, err) - assert.NotNil(t, s) - }) - - t.Run("cache hit", func(t *testing.T) { - s, err := d.Get(context.Background(), gvk) - require.NoError(t, err) - assert.NotNil(t, s) - }) + s, err := d.Get(context.Background(), gvk) + require.NoError(t, err) + assert.Nil(t, s) + + s, err = d.Get(context.Background(), gvk) + require.NoError(t, err) + assert.Nil(t, s) + + assert.Equal(t, 4, client.Calls) +} + +func TestDiscoveryCacheRefillDisabled(t *testing.T) { + client := &fakeDiscovery{} + d := &discoveryCache{client: client, fillWhenNotFound: false} + + gvk := schema.GroupVersionKind{ + Group: "test-group", + Version: "test-version", + Kind: "TestKind1", + } + + s, err := d.Get(context.Background(), gvk) + require.NoError(t, err) + assert.Nil(t, s) + + s, err = d.Get(context.Background(), gvk) + require.NoError(t, err) + assert.Nil(t, s) + + assert.Equal(t, 1, client.Calls) } // the fake.FakeDiscovery doesn't allow fake OpenAPISchema return values. type fakeDiscovery struct { fake.FakeDiscovery - Schema *openapi_v2.Document + Calls int } func (f *fakeDiscovery) OpenAPISchema() (*openapi_v2.Document, error) { - return f.Schema, nil + f.Calls++ + return &openapi_v2.Document{}, nil }