tao12345666333 commented on a change in pull request #772:
URL:
https://github.com/apache/apisix-ingress-controller/pull/772#discussion_r759988121
##########
File path: pkg/apisix/cache/cache.go
##########
@@ -82,4 +88,6 @@ type Cache interface {
DeleteConsumer(*v1.Consumer) error
// DeleteSchema deletes the specified schema in cache.
DeleteSchema(*v1.Schema) error
+ // DeletePluginConfig deletes the specified plugin_config in cache.
+ DeletePluginConfig(config *v1.PluginConfig) error
Review comment:
```suggestion
DeletePluginConfig(*v1.PluginConfig) error
```
##########
File path: pkg/apisix/cache/memdb_test.go
##########
@@ -389,3 +389,58 @@ func TestMemDBCacheSchema(t *testing.T) {
}
assert.Error(t, ErrNotFound, c.DeleteSchema(s4))
}
+
+func TestMemDBCachePluginConfig(t *testing.T) {
+ c, err := NewMemDBCache()
+ assert.Nil(t, err, "NewMemDBCache")
+
+ pc1 := &v1.PluginConfig{
+ Metadata: v1.Metadata{
+ ID: "1",
+ Name: "name1",
+ },
+ }
+ assert.Nil(t, c.InsertPluginConfig(pc1), "inserting plugin_config s1")
+
+ pc11, err := c.GetPluginConfig("1")
+ assert.Nil(t, err)
+ assert.Equal(t, pc1, pc11)
+
+ pc2 := &v1.PluginConfig{
+ Metadata: v1.Metadata{
+ ID: "2",
+ Name: "name2",
+ },
+ }
+ s3 := &v1.PluginConfig{
+ Metadata: v1.Metadata{
+ ID: "3",
+ Name: "name3",
+ },
+ }
+ assert.Nil(t, c.InsertPluginConfig(pc2), "inserting plugin_config s2")
Review comment:
And no s2
##########
File path: pkg/apisix/cache/memdb_test.go
##########
@@ -389,3 +389,58 @@ func TestMemDBCacheSchema(t *testing.T) {
}
assert.Error(t, ErrNotFound, c.DeleteSchema(s4))
}
+
+func TestMemDBCachePluginConfig(t *testing.T) {
+ c, err := NewMemDBCache()
+ assert.Nil(t, err, "NewMemDBCache")
+
+ pc1 := &v1.PluginConfig{
+ Metadata: v1.Metadata{
+ ID: "1",
+ Name: "name1",
+ },
+ }
+ assert.Nil(t, c.InsertPluginConfig(pc1), "inserting plugin_config s1")
Review comment:
There is no s1.
##########
File path: pkg/apisix/cache/memdb_test.go
##########
@@ -354,7 +354,7 @@ func TestMemDBCacheSchema(t *testing.T) {
Name: "plugins/p1",
Content: "plugin schema",
}
- assert.Nil(t, c.InsertSchema(s1), "inserting schema s1")
+ assert.Nil(t, c.InsertSchema(s1), "inserting schema pc1")
Review comment:
Why amend it here?
##########
File path: pkg/apisix/pluginconfig.go
##########
@@ -0,0 +1,235 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package apisix
+
+import (
+ "bytes"
+ "context"
+ "encoding/json"
+
+ "go.uber.org/zap"
+
+ "github.com/apache/apisix-ingress-controller/pkg/apisix/cache"
+ "github.com/apache/apisix-ingress-controller/pkg/id"
+ "github.com/apache/apisix-ingress-controller/pkg/log"
+ v1 "github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1"
+)
+
+type pluginConfigClient struct {
+ url string
+ cluster *cluster
+}
+
+func newPluginConfigClient(c *cluster) PluginConfig {
+ return &pluginConfigClient{
+ url: c.baseURL + "/plugin_configs",
+ cluster: c,
+ }
+}
+
+// Get returns the v1.PluginConfig.
+// FIXME, currently if caller pass a non-existent resource, the Get always
passes
+// through cache.
+func (pc *pluginConfigClient) Get(ctx context.Context, name string)
(*v1.PluginConfig, error) {
+ log.Debugw("try to look up pluginConfig",
+ zap.String("name", name),
+ zap.String("url", pc.url),
+ zap.String("cluster", "default"),
+ )
+ rid := id.GenID(name)
+ pluginConfig, err := pc.cluster.cache.GetPluginConfig(rid)
+ if err == nil {
+ return pluginConfig, nil
+ }
+ if err != cache.ErrNotFound {
+ log.Errorw("failed to find pluginConfig in cache, will try to
lookup from APISIX",
+ zap.String("name", name),
+ zap.Error(err),
+ )
+ } else {
+ log.Debugw("pluginConfig not found in cache, will try to lookup
from APISIX",
+ zap.String("name", name),
+ zap.Error(err),
+ )
+ }
+
+ // TODO Add mutex here to avoid dog-pile effect.
+ url := pc.url + "/" + rid
+ resp, err := pc.cluster.getResource(ctx, url, "pluginConfig")
+ pc.cluster.metricsCollector.IncrAPISIXRequest("pluginConfig")
+ if err != nil {
+ if err == cache.ErrNotFound {
+ log.Warnw("pluginConfig not found",
+ zap.String("name", name),
+ zap.String("url", url),
+ zap.String("cluster", "default"),
+ )
+ } else {
+ log.Errorw("failed to get pluginConfig from APISIX",
+ zap.String("name", name),
+ zap.String("url", url),
+ zap.String("cluster", "default"),
+ zap.Error(err),
+ )
+ }
+ return nil, err
+ }
+
+ pluginConfig, err = resp.Item.pluginConfig()
+ if err != nil {
+ log.Errorw("failed to convert pluginConfig item",
+ zap.String("url", pc.url),
+ zap.String("pluginConfig_key", resp.Item.Key),
+ zap.String("pluginConfig_value",
string(resp.Item.Value)),
+ zap.Error(err),
+ )
+ return nil, err
+ }
+
+ if err := pc.cluster.cache.InsertPluginConfig(pluginConfig); err != nil
{
+ log.Errorf("failed to reflect pluginConfig create to cache:
%s", err)
+ return nil, err
+ }
+ return pluginConfig, nil
+}
+
+// List is only used in cache warming up. So here just pass through
+// to APISIX.
+func (pc *pluginConfigClient) List(ctx context.Context) ([]*v1.PluginConfig,
error) {
+ log.Debugw("try to list pluginConfig in APISIX",
+ zap.String("cluster", "default"),
+ zap.String("url", pc.url),
+ )
+ pluginConfigItems, err := pc.cluster.listResource(ctx, pc.url,
"pluginConfig")
+ pc.cluster.metricsCollector.IncrAPISIXRequest("pluginConfig")
+ if err != nil {
+ log.Errorf("failed to list pluginConfig: %s", err)
+ return nil, err
+ }
+
+ var items []*v1.PluginConfig
+ for i, item := range pluginConfigItems.Node.Items {
+ pluginConfig, err := item.pluginConfig()
+ if err != nil {
+ log.Errorw("failed to convert pluginConfig item",
+ zap.String("url", pc.url),
+ zap.String("pluginConfig_key", item.Key),
+ zap.String("pluginConfig_value",
string(item.Value)),
+ zap.Error(err),
+ )
+ return nil, err
+ }
+
+ items = append(items, pluginConfig)
+ log.Debugf("list pluginConfig #%d, body: %s", i,
string(item.Value))
+ }
+
+ return items, nil
+}
+
+func (pc *pluginConfigClient) Create(ctx context.Context, obj
*v1.PluginConfig) (*v1.PluginConfig, error) {
+ log.Debugw("try to create pluginConfig",
+ zap.String("name", obj.Name),
+ zap.Any("plugins", obj.Plugins),
+ zap.String("cluster", "default"),
+ zap.String("url", pc.url),
+ )
+
+ if err := pc.cluster.HasSynced(ctx); err != nil {
+ return nil, err
+ }
+ data, err := json.Marshal(obj)
+ if err != nil {
+ return nil, err
+ }
+
+ url := pc.url + "/" + obj.ID
+ log.Debugw("creating pluginConfig", zap.ByteString("body", data),
zap.String("url", url))
+ resp, err := pc.cluster.createResource(ctx, url, "pluginConfig",
bytes.NewReader(data))
+ pc.cluster.metricsCollector.IncrAPISIXRequest("pluginConfig")
+ if err != nil {
+ log.Errorf("failed to create pluginConfig: %s", err)
+ return nil, err
+ }
+
+ pluginConfig, err := resp.Item.pluginConfig()
+ if err != nil {
+ return nil, err
+ }
+ if err := pc.cluster.cache.InsertPluginConfig(pluginConfig); err != nil
{
+ log.Errorf("failed to reflect pluginConfig create to cache:
%s", err)
+ return nil, err
+ }
+ return pluginConfig, nil
+}
+
+func (pc *pluginConfigClient) Delete(ctx context.Context, obj
*v1.PluginConfig) error {
+ log.Debugw("try to delete pluginConfig",
+ zap.String("id", obj.ID),
+ zap.String("name", obj.Name),
+ zap.String("cluster", "default"),
+ zap.String("url", pc.url),
+ )
+ if err := pc.cluster.HasSynced(ctx); err != nil {
+ return err
+ }
+ url := pc.url + "/" + obj.ID
+ if err := pc.cluster.deleteResource(ctx, url, "pluginConfig"); err !=
nil {
+ pc.cluster.metricsCollector.IncrAPISIXRequest("pluginConfig")
+ return err
+ }
+ pc.cluster.metricsCollector.IncrAPISIXRequest("pluginConfig")
+ if err := pc.cluster.cache.DeletePluginConfig(obj); err != nil {
+ log.Errorf("failed to reflect pluginConfig delete to cache:
%s", err)
+ if err != cache.ErrNotFound {
+ return err
+ }
+ }
+ return nil
+}
+
+func (pc *pluginConfigClient) Update(ctx context.Context, obj
*v1.PluginConfig) (*v1.PluginConfig, error) {
+ log.Debugw("try to update pluginConfig",
+ zap.String("id", obj.ID),
+ zap.String("name", obj.Name),
+ zap.Any("plugins", obj.Plugins),
+ zap.String("cluster", "default"),
+ zap.String("url", pc.url),
+ )
+ if err := pc.cluster.HasSynced(ctx); err != nil {
+ return nil, err
+ }
+ body, err := json.Marshal(obj)
+ if err != nil {
+ return nil, err
+ }
+ url := pc.url + "/" + obj.ID
+ log.Debugw("updating username", zap.ByteString("body", body),
zap.String("url", url))
Review comment:
typo. no "username"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
