correct fix for GC of objects whose types could not be discovered (#148)

* correct fix for GC of objects whose types could not be discovered

* add integration test to ensure lazy resource creation

* avoid races in sio

Author: gotwarlost

.golangci.yml

@@ -11,7 +11,6 @@ linters-settings:
 linters:
     disable-all: true
     enable:
-    - bodyclose
     - deadcode
     - depguard
     - dogsled
@@ -29,5 +28,4 @@ linters:
     - stylecheck
     - typecheck
     - unconvert
-    - unused
     - varcheck

internal/commands/apply.go

@@ -144,7 +144,7 @@ func doApply(args []string, config applyCommandConfig) error {
 			Application:    config.App().Name(),
 			Tag:            config.App().Tag(),
 			Environment:    env,
-			KindFilter:     fp.kindFilter,
+			KindFilter:     fp.GVKFilter,
 			ListQueryScope: scope,
 		})
 	}

internal/commands/delete.go

@@ -75,7 +75,7 @@ func doDelete(args []string, config deleteCommandConfig) error {
 			Application:    config.App().Name(),
 			Tag:            config.App().Tag(),
 			Environment:    env,
-			KindFilter:     fp.kindFilter,
+			KindFilter:     fp.GVKFilter,
 			ListQueryScope: scope,
 		})
 		deletions, err = lister.deletions(nil, fp.Includes)

internal/commands/diff.go

@@ -318,7 +318,7 @@ func doDiff(args []string, config diffCommandConfig) error {
 			Application:    config.App().Name(),
 			Tag:            config.App().Tag(),
 			Environment:    env,
-			KindFilter:     fp.kindFilter,
+			KindFilter:     fp.GVKFilter,
 			ListQueryScope: scope,
 		})
 	}

internal/commands/filter.go

@@ -23,6 +23,7 @@ import (
 	"github.com/splunk/qbec/internal/eval"
 	"github.com/splunk/qbec/internal/model"
 	"github.com/splunk/qbec/internal/sio"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
 type filterParams struct {
@@ -32,6 +33,10 @@ type filterParams struct {
 	componentFilter model.Filter
 }
 
+func (f filterParams) GVKFilter(gvk schema.GroupVersionKind) bool {
+	return f.kindFilter == nil || f.kindFilter.ShouldInclude(gvk.Kind)
+}
+
 func (f filterParams) Includes(o model.K8sQbecMeta) bool {
 	if !(f.kindFilter == nil || f.kindFilter.ShouldInclude(o.GetKind())) {
 		return false

internal/commands/integration_scaffold_test.go

@@ -25,7 +25,7 @@ var (
 
 func initialize(t *testing.T) {
 	contextName = os.Getenv("QBEC_CONTEXT")
-	if contextName != "" {
+	if contextName == "" {
 		contextName = "kind-kind"
 	}
 	c, err := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(
@@ -77,3 +77,10 @@ func (s *integrationScaffold) executeCommand(testArgs ...string) error {
 	)
 	return s.baseScaffold.executeCommand(args...)
 }
+
+func (s *integrationScaffold) sub() *integrationScaffold {
+	return &integrationScaffold{
+		baseScaffold: s.baseScaffold.sub(),
+		ns:           s.ns,
+	}
+}

internal/commands/integration_test.go

@@ -3,8 +3,11 @@
 package commands
 
 import (
+	"fmt"
 	"regexp"
+	"sync"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -99,3 +102,32 @@ func TestIntegrationBasic(t *testing.T) {
 		a.EqualValues(2, len(stats["updated"].([]interface{})))
 	})
 }
+
+func TestIntegrationLazyCustomResources(t *testing.T) {
+	dir := "testdata/projects/lazy-resources"
+	ns, reset := newNamespace(t)
+	defer reset()
+
+	extra := []string{"--vm:ext-str=suffix=" + fmt.Sprint(time.Now().Unix())}
+	var err1, err2 error
+	done := make(chan struct{}, 1)
+	var wg sync.WaitGroup
+	wg.Add(2)
+
+	s1 := newIntegrationScaffold(t, ns, dir)
+	defer s1.reset()
+	defer s1.executeCommand(append(extra, "delete", "local")...)
+
+	s2 := s1.sub()
+	defer s2.reset()
+
+	go func() {
+		defer func() { close(done) }()
+		err1 = s2.executeCommand(append(extra, "apply", "-C", "crds", "local")...)
+		require.NoError(t, err1)
+	}()
+	time.Sleep(2 * time.Second)
+	err2 = s1.executeCommand(append(extra, "apply", "-k", "customresourcedefinitions", "local")...)
+	require.NoError(t, err2)
+	<-done
+}

internal/commands/remote-list.go

@@ -44,9 +44,10 @@ func (s *stubLister) deletions(ignore []model.K8sLocalObject, filter func(obj mo
 }
 
 type remoteLister struct {
-	client listClient
-	ch     chan listResult
-	cfg    remote.ListQueryConfig
+	client       listClient
+	ch           chan listResult
+	cfg          remote.ListQueryConfig
+	unknownTypes map[schema.GroupVersionKind]bool
 }
 
 type listResult struct {
@@ -90,8 +91,9 @@ func newRemoteLister(client listClient, allObjects []model.K8sLocalObject, defau
 	sort.Strings(nsList)
 
 	return &remoteLister{
-			client: client,
-			ch:     make(chan listResult, 1),
+			client:       client,
+			ch:           make(chan listResult, 1),
+			unknownTypes: unknown,
 		},
 		remote.ListQueryScope{
 			Namespaces:     nsList,
@@ -102,6 +104,16 @@ func newRemoteLister(client listClient, allObjects []model.K8sLocalObject, defau
 
 func (r *remoteLister) start(config remote.ListQueryConfig) {
 	r.cfg = config
+	if config.KindFilter == nil {
+		config.KindFilter = func(_ schema.GroupVersionKind) bool { return true }
+	}
+	orig := config.KindFilter
+	config.KindFilter = func(gvk schema.GroupVersionKind) bool {
+		if r.unknownTypes[gvk] {
+			return false
+		}
+		return orig(gvk)
+	}
 	go func() {
 		start := time.Now()
 		list, err := r.client.ListObjects(config)
@@ -120,14 +132,13 @@ func (r *remoteLister) deletions(all []model.K8sLocalObject, filter func(obj mod
 	sio.Debugf("server objects load took %v\n", lr.duration)
 
 	cfg := r.cfg
-	if cfg.KindFilter == nil {
-		f, _ := model.NewKindFilter(nil, nil)
-		cfg.KindFilter = f
-	}
 
 	var removals []model.K8sQbecMeta
 	for _, c := range all {
-		if !cfg.KindFilter.ShouldInclude(c.GetKind()) {
+		if !cfg.KindFilter(c.GroupVersionKind()) {
+			continue
+		}
+		if r.unknownTypes[c.GroupVersionKind()] {
 			continue
 		}
 		removals = append(removals, c)

internal/commands/setup.go

@@ -227,7 +227,7 @@ func doSetup(root *cobra.Command, cf configFactory, overrideCP clientProvider) {
 		if !cmd.Flags().Changed("colors") {
 			cf.colors = isatty.IsTerminal(os.Stdout.Fd())
 		}
-		sio.EnableColors = cf.colors
+		sio.EnableColors(cf.colors)
 
 		// if env file has been specified on the command line, ensure it is resolved w.r.t to the current working
 		// directory before we change it

internal/commands/testdata/projects/lazy-resources/components/crds/crd.yaml

@@ -0,0 +1,42 @@
+---
+apiVersion: apiextensions.k8s.io/v1beta1
+kind: CustomResourceDefinition
+metadata:
+  name: foo.test.qbec.io
+spec:
+  group: test.qbec.io
+  names:
+    kind: Foo
+    listKind: FooList
+    plural: foos
+    singular: foo
+  scope: Namespaced
+  validation:
+    openAPIV3Schema:
+      description: Foo is a foo.
+      properties:
+        apiVersion:
+          type: string
+        kind:
+          type: string
+        metadata:
+          type: object
+        spec:
+          properties:
+            bar:
+              description: the bar for the foo
+              type: string
+          required:
+            - bar
+          type: object
+        status:
+          type: object
+      required:
+        - metadata
+        - spec
+      type: object
+  version: v1alpha1
+  versions:
+    - name: v1alpha1
+      served: true
+      storage: true