Merge pull request #187 from snasovich/fix-rancher-35656

Fixed distinct namespaces resolution in Apply

Author: kinarashah

pkg/apply/desiredset_process.go

@@ -251,7 +251,7 @@ func (o *desiredSet) process(debugID string, set labels.Selector, gvk schema.Gro
 
 	reconciler := o.reconcilers[gvk]
 
-	existing, err := o.list(nsed, controller, client, set)
+	existing, err := o.list(nsed, controller, client, set, objs)
 	if err != nil {
 		o.err(errors.Wrapf(err, "failed to list %s for %s", gvk, debugID))
 		return
@@ -338,19 +338,22 @@ func (o *desiredSet) process(debugID string, set labels.Selector, gvk schema.Gro
 	}
 }
 
-func (o *desiredSet) list(namespaced bool, informer cache.SharedIndexInformer, client dynamic.NamespaceableResourceInterface, selector labels.Selector) (map[objectset.ObjectKey]runtime.Object, error) {
+func (o *desiredSet) list(namespaced bool, informer cache.SharedIndexInformer, client dynamic.NamespaceableResourceInterface,
+	selector labels.Selector, desiredObjects map[objectset.ObjectKey]runtime.Object) (map[objectset.ObjectKey]runtime.Object, error) {
 	var (
 		errs []error
 		objs = map[objectset.ObjectKey]runtime.Object{}
 	)
 
 	if informer == nil {
-		// if a lister namespace is set assume all objects will belong to the listerNamespace
+		// if a lister namespace is set assume all objects belong to the listerNamespace
+		// otherwise use distinct namespaces from the desired objects
+		// (even if not namespaced as we'll get a single empty namespace in this case which is working OK)
 		var namespaces []string
 		if o.listerNamespace != "" {
 			namespaces = append(namespaces, o.listerNamespace)
 		} else {
-			namespaces = o.objs.Namespaces()
+			namespaces = getDistinctNamespaces(desiredObjects)
 		}
 
 		err := multiNamespaceList(o.ctx, namespaces, client, selector, func(obj unstructured.Unstructured) {
@@ -462,3 +465,23 @@ func multiNamespaceList(ctx context.Context, namespaces []string, baseClient dyn
 
 	return wg.Wait()
 }
+
+func getDistinctNamespaces(objs map[objectset.ObjectKey]runtime.Object) (namespaces []string) {
+	for objKey, _ := range objs {
+		var duplicate bool
+		for i := range namespaces {
+			if namespaces[i] == objKey.Namespace {
+				duplicate = true
+				break
+			}
+		}
+
+		if duplicate {
+			continue
+		}
+
+		namespaces = append(namespaces, objKey.Namespace)
+	}
+
+	return
+}

pkg/apply/desiredset_process_test.go

@@ -3,15 +3,17 @@ package apply
 import (
 	"context"
 	"errors"
+	"strings"
+	"testing"
+
+	"github.com/rancher/wrangler/pkg/objectset"
 	"github.com/stretchr/testify/assert"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/dynamic/fake"
 	k8stesting "k8s.io/client-go/testing"
-	"strings"
-	"testing"
 )
 
 func Test_multiNamespaceList(t *testing.T) {
@@ -106,3 +108,56 @@ func Test_multiNamespaceList(t *testing.T) {
 		})
 	}
 }
+
+func TestObjectSet_getDistinctNamespaces(t *testing.T) {
+	tests := []struct {
+		name           string
+		objects        map[objectset.ObjectKey]runtime.Object
+		wantNamespaces []string
+	}{
+		{
+			name:           "empty",
+			objects:        map[objectset.ObjectKey]runtime.Object{},
+			wantNamespaces: nil,
+		},
+		{
+			name: "1 namespace",
+			objects: map[objectset.ObjectKey]runtime.Object{
+				objectset.ObjectKey{Namespace: "ns1", Name: "a"}: nil,
+				objectset.ObjectKey{Namespace: "ns1", Name: "b"}: nil,
+			},
+			wantNamespaces: []string{"ns1"},
+		},
+		{
+			name: "many namespaces",
+			objects: map[objectset.ObjectKey]runtime.Object{
+				objectset.ObjectKey{Namespace: "ns1", Name: "a"}: nil,
+				objectset.ObjectKey{Namespace: "ns2", Name: "b"}: nil,
+			},
+			wantNamespaces: []string{"ns1", "ns2"},
+		},
+		{
+			name: "many namespaces with duplicates",
+			objects: map[objectset.ObjectKey]runtime.Object{
+				objectset.ObjectKey{Namespace: "ns1", Name: "a"}: nil,
+				objectset.ObjectKey{Namespace: "ns2", Name: "b"}: nil,
+				objectset.ObjectKey{Namespace: "ns1", Name: "c"}: nil,
+			},
+			wantNamespaces: []string{"ns1", "ns2"},
+		},
+		{
+			name: "missing namespace",
+			objects: map[objectset.ObjectKey]runtime.Object{
+				objectset.ObjectKey{Namespace: "ns1", Name: "a"}: nil,
+				objectset.ObjectKey{Name: "b"}:                   nil,
+			},
+			wantNamespaces: []string{"", "ns1"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotNamespaces := getDistinctNamespaces(tt.objects)
+			assert.ElementsMatchf(t, tt.wantNamespaces, gotNamespaces, "getDistinctNamespaces() = %v, want %v", gotNamespaces, tt.wantNamespaces)
+		})
+	}
+}