Skip to content

Commit

Permalink
Merge pull request #6065 from JimDevil/feat-search
Browse files Browse the repository at this point in the history 
fix(search/proxy/controller): resourceregistry merge ns
  • Loading branch information
@karmada-bot
karmada-bot authored Jan 22, 2025
2 parents e3628af + 93c09c1 commit 07747ed
Show file tree
Hide file tree
Showing 5 changed files with 219 additions and 45 deletions.
41 changes: 24 additions & 17 deletions pkg/search/proxy/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,29 +228,36 @@ func (ctl *Controller) reconcile(util.QueueKey) error {
klog.Warningf("cluster %s is notReady", cluster.Name)
continue
}

if _, exist := resourcesByClusters[cluster.Name]; !exist {
resourcesByClusters[cluster.Name] = make(map[schema.GroupVersionResource]*store.MultiNamespace)
}

for resource, multiNS := range matchedResources {
gvk, err := ctl.restMapper.KindFor(resource)
if err != nil {
klog.Errorf("Failed to get gvk: %v", err)
continue
}
if !helper.IsAPIEnabled(cluster.Status.APIEnablements, gvk.GroupVersion().String(), gvk.Kind) {
klog.Warningf("Resource %s is not enabled for cluster %s", resource.String(), cluster)
continue
}
resourcesByClusters[cluster.Name][resource] = multiNS
}
ctl.mergeResourcesByClusters(resourcesByClusters, cluster, matchedResources)
}
}

return ctl.store.UpdateCache(resourcesByClusters, registeredResources)
}

func (ctl *Controller) mergeResourcesByClusters(resourcesByClusters map[string]map[schema.GroupVersionResource]*store.MultiNamespace, cluster *clusterv1alpha1.Cluster, matchedResources map[schema.GroupVersionResource]*store.MultiNamespace) {
if _, exist := resourcesByClusters[cluster.Name]; !exist {
resourcesByClusters[cluster.Name] = make(map[schema.GroupVersionResource]*store.MultiNamespace)
}

for resource, multiNS := range matchedResources {
gvk, err := ctl.restMapper.KindFor(resource)
if err != nil {
klog.Errorf("Failed to get gvk: %v", err)
continue
}
if !helper.IsAPIEnabled(cluster.Status.APIEnablements, gvk.GroupVersion().String(), gvk.Kind) {
klog.Warningf("Resource %s is not enabled for cluster %s", resource.String(), cluster)
continue
}
if ns, exist := resourcesByClusters[cluster.Name][resource]; !exist {
resourcesByClusters[cluster.Name][resource] = multiNS
} else {
resourcesByClusters[cluster.Name][resource] = ns.Merge(multiNS)
}
}
}

type errorHTTPHandler struct {
requestInfo *request.RequestInfo
err error
Expand Down
112 changes: 85 additions & 27 deletions pkg/search/proxy/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ import (
"net/http"
"net/http/httptest"
"reflect"
"sort"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -104,19 +102,26 @@ func TestController(t *testing.T) {
}

func TestController_reconcile(t *testing.T) {
echoStrings := func(ss ...string) string {
sort.Strings(ss)
return strings.Join(ss, ",")
newMultiNs := func(namespaces ...string) *store.MultiNamespace {
multiNs := store.NewMultiNamespace()
if len(namespaces) == 0 {
multiNs.Add(metav1.NamespaceAll)
return multiNs
}
for _, ns := range namespaces {
multiNs.Add(ns)
}
return multiNs
}
tests := []struct {
name string
input []runtime.Object
want map[string]string
want map[string]map[string]*store.MultiNamespace
}{
{
name: "all empty",
input: []runtime.Object{},
want: map[string]string{},
want: map[string]map[string]*store.MultiNamespace{},
},
{
name: "resource registered, while cluster not registered",
Expand All @@ -133,7 +138,7 @@ func TestController_reconcile(t *testing.T) {
},
},
},
want: map[string]string{},
want: map[string]map[string]*store.MultiNamespace{},
},
{
name: "pod and node are registered",
Expand All @@ -153,9 +158,15 @@ func TestController_reconcile(t *testing.T) {
},
},
},
want: map[string]string{
"cluster1": echoStrings("pods", "nodes"),
"cluster2": echoStrings("pods", "nodes"),
want: map[string]map[string]*store.MultiNamespace{
"cluster1": {
"pods": newMultiNs(),
"nodes": newMultiNs(),
},
"cluster2": {
"pods": newMultiNs(),
"nodes": newMultiNs(),
},
},
},
{
Expand All @@ -178,9 +189,13 @@ func TestController_reconcile(t *testing.T) {
},
},
},
want: map[string]string{
"cluster1": echoStrings("pods"),
"cluster2": echoStrings("nodes"),
want: map[string]map[string]*store.MultiNamespace{
"cluster1": {
"pods": newMultiNs(),
},
"cluster2": {
"nodes": newMultiNs(),
},
},
},
{
Expand All @@ -203,9 +218,14 @@ func TestController_reconcile(t *testing.T) {
},
},
},
want: map[string]string{
"cluster1": echoStrings("pods", "nodes"),
"cluster2": echoStrings("nodes"),
want: map[string]map[string]*store.MultiNamespace{
"cluster1": {
"pods": newMultiNs(),
"nodes": newMultiNs(),
},
"cluster2": {
"nodes": newMultiNs(),
},
},
},
{
Expand All @@ -226,8 +246,10 @@ func TestController_reconcile(t *testing.T) {
},
},
},
want: map[string]string{
"cluster1": echoStrings("pods"),
want: map[string]map[string]*store.MultiNamespace{
"cluster1": {
"pods": newMultiNs(),
},
},
},
{
Expand Down Expand Up @@ -258,8 +280,10 @@ func TestController_reconcile(t *testing.T) {
},
},
},
want: map[string]string{
"cluster1": echoStrings("pods"),
want: map[string]map[string]*store.MultiNamespace{
"cluster1": {
"pods": newMultiNs(),
},
},
},
{
Expand All @@ -278,13 +302,47 @@ func TestController_reconcile(t *testing.T) {
},
},
},
want: map[string]string{},
want: map[string]map[string]*store.MultiNamespace{},
},
{
name: "register pod twice in two ResourceRegistries with different namespace",
input: []runtime.Object{
newCluster("cluster1"),
newCluster("cluster2"),
&searchv1alpha1.ResourceRegistry{
ObjectMeta: metav1.ObjectMeta{Name: "rr1"},
Spec: searchv1alpha1.ResourceRegistrySpec{
TargetCluster: policyv1alpha1.ClusterAffinity{
ClusterNames: []string{"cluster1"},
},
ResourceSelectors: []searchv1alpha1.ResourceSelector{
proxytest.PodSelectorWithNS1,
},
},
},
&searchv1alpha1.ResourceRegistry{
ObjectMeta: metav1.ObjectMeta{Name: "rr2"},
Spec: searchv1alpha1.ResourceRegistrySpec{
TargetCluster: policyv1alpha1.ClusterAffinity{
ClusterNames: []string{"cluster1"},
},
ResourceSelectors: []searchv1alpha1.ResourceSelector{
proxytest.PodSelectorWithNS2,
},
},
},
},
want: map[string]map[string]*store.MultiNamespace{
"cluster1": {
"pods": newMultiNs("ns1", "ns2"),
},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
actual := map[string]string{}
actual := map[string]map[string]*store.MultiNamespace{}
karmadaClientset := karmadafake.NewSimpleClientset(tt.input...)
karmadaFactory := karmadainformers.NewSharedInformerFactory(karmadaClientset, 0)

Expand All @@ -295,11 +353,11 @@ func TestController_reconcile(t *testing.T) {
store: &proxytest.MockStore{
UpdateCacheFunc: func(m map[string]map[schema.GroupVersionResource]*store.MultiNamespace, _ map[schema.GroupVersionResource]struct{}) error {
for clusterName, resources := range m {
resourceNames := make([]string, 0, len(resources))
for resource := range resources {
resourceNames = append(resourceNames, resource.Resource)
resourceCaches := map[string]*store.MultiNamespace{}
for resource, multiNs := range resources {
resourceCaches[resource.Resource] = multiNs
}
actual[clusterName] = echoStrings(resourceNames...)
actual[clusterName] = resourceCaches
}
if len(actual) != len(m) {
return fmt.Errorf("cluster duplicate: %#v", m)
Expand Down
14 changes: 14 additions & 0 deletions pkg/search/proxy/store/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,20 @@ type MultiNamespace struct {
namespaces sets.Set[string]
}

// Merge merges multiNS into n.
func (n *MultiNamespace) Merge(multiNS *MultiNamespace) *MultiNamespace {
if n.allNamespaces || multiNS.allNamespaces {
n.allNamespaces = true
n.namespaces = nil
return n
}
if n.Equal(multiNS) {
return n
}
n.namespaces.Insert(multiNS.namespaces.Clone().UnsortedList()...)
return n
}

// NewMultiNamespace return a new empty MultiNamespace.
func NewMultiNamespace() *MultiNamespace {
return &MultiNamespace{
Expand Down
90 changes: 90 additions & 0 deletions pkg/search/proxy/store/util_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1082,3 +1082,93 @@ func TestMultiNamespace_Equal(t *testing.T) {
})
}
}

func TestMultiNamespace_Merge(t *testing.T) {
type fields struct {
allNamespaces bool
namespaces sets.Set[string]
}
type args struct {
multiNS *MultiNamespace
}
tests := []struct {
name string
fields fields
args args
want *MultiNamespace
}{
{
name: "all ns merge all ns",
fields: fields{
allNamespaces: true,
},
args: args{
multiNS: &MultiNamespace{
allNamespaces: true,
},
},
want: &MultiNamespace{
allNamespaces: true,
},
},
{
name: "all ns merge not all",
fields: fields{
allNamespaces: true,
},
args: args{
multiNS: &MultiNamespace{
allNamespaces: false,
namespaces: sets.New[string]("foo"),
},
},
want: &MultiNamespace{
allNamespaces: true,
},
},
{
name: "not all ns merge all",
fields: fields{
allNamespaces: false,
namespaces: sets.New[string]("foo"),
},
args: args{
multiNS: &MultiNamespace{
allNamespaces: true,
},
},
want: &MultiNamespace{
allNamespaces: true,
},
},
{
name: "not all ns merge not all ns",
fields: fields{
allNamespaces: false,
namespaces: sets.New[string]("foo", "zoo"),
},
args: args{
multiNS: &MultiNamespace{
allNamespaces: false,
namespaces: sets.New[string]("bar"),
},
},
want: &MultiNamespace{
allNamespaces: false,
namespaces: sets.New[string]("foo", "bar", "zoo"),
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
n := &MultiNamespace{
allNamespaces: tt.fields.allNamespaces,
namespaces: tt.fields.namespaces,
}
got := n.Merge(tt.args.multiNS)
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("MultiNamespace.Merge() = %v, want %v", got, tt.want)
}
})
}
}
7 changes: 6 additions & 1 deletion pkg/search/proxy/testing/constant.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@ var (
SecretGVR = corev1.SchemeGroupVersion.WithResource("secret")
ClusterGVR = clusterv1alpha1.SchemeGroupVersion.WithResource("cluster")

PodSelector = searchv1alpha1.ResourceSelector{APIVersion: PodGVK.GroupVersion().String(), Kind: PodGVK.Kind}
PodSelector = searchv1alpha1.ResourceSelector{APIVersion: PodGVK.GroupVersion().String(), Kind: PodGVK.Kind}

PodSelectorWithNS1 = searchv1alpha1.ResourceSelector{APIVersion: PodGVK.GroupVersion().String(), Kind: PodGVK.Kind, Namespace: "ns1"}

PodSelectorWithNS2 = searchv1alpha1.ResourceSelector{APIVersion: PodGVK.GroupVersion().String(), Kind: PodGVK.Kind, Namespace: "ns2"}

NodeSelector = searchv1alpha1.ResourceSelector{APIVersion: NodeGVK.GroupVersion().String(), Kind: NodeGVK.Kind}

RestMapper *meta.DefaultRESTMapper
Expand Down

0 comments on commit 07747ed

Please sign in to comment.