From 2097a67e68457fcd6cc91e4a5d777d6f76b8964c Mon Sep 17 00:00:00 2001 From: kv Date: Mon, 8 Nov 2021 12:42:42 +0800 Subject: [PATCH 1/4] fix: ingress do not watching any namespace when namespaceSelector is empty --- pkg/api/validation/utils.go | 11 +++++++++++ pkg/api/validation/utils_test.go | 9 +++++++++ pkg/ingress/compare.go | 3 ++- pkg/ingress/controller.go | 3 ++- 4 files changed, 24 insertions(+), 2 deletions(-) diff --git a/pkg/api/validation/utils.go b/pkg/api/validation/utils.go index 5d2145cf9a..84180be955 100644 --- a/pkg/api/validation/utils.go +++ b/pkg/api/validation/utils.go @@ -99,3 +99,14 @@ func validateSchema(schemaLoader *gojsonschema.JSONLoader, obj interface{}) (boo return false, resultErr } + +func HasValueInSyncMap(m *sync.Map) bool { + hasValue := false + if m != nil { + m.Range(func(k, v interface{}) bool { + hasValue = true + return false + }) + } + return hasValue +} diff --git a/pkg/api/validation/utils_test.go b/pkg/api/validation/utils_test.go index 17c3961bac..59f538a97c 100644 --- a/pkg/api/validation/utils_test.go +++ b/pkg/api/validation/utils_test.go @@ -16,8 +16,10 @@ package validation import ( + "sync" "testing" + "github.com/stretchr/testify/assert" "github.com/xeipuuv/gojsonschema" v1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v1" @@ -46,3 +48,10 @@ func Test_validateSchema(t *testing.T) { }) } } + +func TestHasValueInSyncMap(t *testing.T) { + m := new(sync.Map) + assert.False(t, HasValueInSyncMap(m), "sync.Map should be empty") + m.Store("hello", "test") + assert.True(t, HasValueInSyncMap(m), "sync.Map should not be empty") +} diff --git a/pkg/ingress/compare.go b/pkg/ingress/compare.go index b5829a9f41..977549ab24 100644 --- a/pkg/ingress/compare.go +++ b/pkg/ingress/compare.go @@ -20,6 +20,7 @@ import ( v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/apache/apisix-ingress-controller/pkg/api/validation" "github.com/apache/apisix-ingress-controller/pkg/log" ) @@ -42,7 +43,7 @@ func (c *Controller) CompareResources(ctx context.Context) error { consumerMapA6 = make(map[string]string) ) // watchingNamespace == nil means to monitor all namespaces - if c.watchingNamespace == nil { + if !validation.HasValueInSyncMap(c.watchingNamespace) { opts := v1.ListOptions{} // list all namespaces nsList, err := c.kubeClient.Client.CoreV1().Namespaces().List(ctx, opts) diff --git a/pkg/ingress/controller.go b/pkg/ingress/controller.go index 14b024bcc4..7e544ac30c 100644 --- a/pkg/ingress/controller.go +++ b/pkg/ingress/controller.go @@ -37,6 +37,7 @@ import ( "k8s.io/client-go/tools/record" "github.com/apache/apisix-ingress-controller/pkg/api" + "github.com/apache/apisix-ingress-controller/pkg/api/validation" "github.com/apache/apisix-ingress-controller/pkg/apisix" apisixcache "github.com/apache/apisix-ingress-controller/pkg/apisix/cache" "github.com/apache/apisix-ingress-controller/pkg/config" @@ -517,7 +518,7 @@ func (c *Controller) run(ctx context.Context) { // namespaceWatching accepts a resource key, getting the namespace part // and checking whether the namespace is being watched. func (c *Controller) namespaceWatching(key string) (ok bool) { - if c.watchingNamespace == nil { + if !validation.HasValueInSyncMap(c.watchingNamespace) { ok = true return } From 07e170c54e05e94619ed104e3b220cc939101040 Mon Sep 17 00:00:00 2001 From: kv Date: Mon, 8 Nov 2021 16:25:40 +0800 Subject: [PATCH 2/4] fix: label-selector Empty labels matches everything. --- pkg/types/labels.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/types/labels.go b/pkg/types/labels.go index 2ff9c12538..f99a943b31 100644 --- a/pkg/types/labels.go +++ b/pkg/types/labels.go @@ -21,8 +21,8 @@ type Labels map[string]string // the passed Labels. func (s Labels) IsSubsetOf(f Labels) bool { if len(s) == 0 { - // Empty labels matches nothing not everything. - return false + // Empty labels matches everything. + return true } for k, v := range s { if vv, ok := f[k]; !ok || vv != v { From 48950e79a96437f7a1198619f1e384a8e985203c Mon Sep 17 00:00:00 2001 From: kv Date: Mon, 8 Nov 2021 16:27:11 +0800 Subject: [PATCH 3/4] fix: label ut --- pkg/types/labels_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/types/labels_test.go b/pkg/types/labels_test.go index 970811c2fe..7ba1e4c9f9 100644 --- a/pkg/types/labels_test.go +++ b/pkg/types/labels_test.go @@ -26,7 +26,7 @@ func TestLabelsIsSubsetOf(t *testing.T) { "version": "v1", "env": "prod", } - assert.Equal(t, l.IsSubsetOf(f), false) + assert.Equal(t, l.IsSubsetOf(f), true) l["env"] = "prod" assert.Equal(t, l.IsSubsetOf(f), true) l["env"] = "qa" From 39dee5228c5413a776f8758117eaf43e55c77532 Mon Sep 17 00:00:00 2001 From: kv Date: Fri, 26 Nov 2021 11:13:21 +0800 Subject: [PATCH 4/4] add comment for CompareResources method --- pkg/ingress/compare.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/ingress/compare.go b/pkg/ingress/compare.go index 977549ab24..5d5747b360 100644 --- a/pkg/ingress/compare.go +++ b/pkg/ingress/compare.go @@ -27,6 +27,8 @@ import ( // CompareResources used to compare the object IDs in resources and APISIX // Find out the rest of objects in APISIX // AND warn them in log. +// This func is NOT concurrency safe. +// cc https://github.com/apache/apisix-ingress-controller/pull/742#discussion_r757197791 func (c *Controller) CompareResources(ctx context.Context) error { var ( wg sync.WaitGroup