From 14538bbf37314765ab2cd7a859f4998ed0ba2a3c Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Tue, 2 Oct 2018 15:25:20 -0700 Subject: [PATCH 01/17] Enable controller to process Custom Metrics --- main.go | 5 +- pkg/controller/controller.go | 47 +++++---- pkg/controller/controller_test.go | 156 +++++++++++++++++++++++++----- 3 files changed, 167 insertions(+), 41 deletions(-) diff --git a/main.go b/main.go index 672178a1..b1b5b77e 100755 --- a/main.go +++ b/main.go @@ -81,9 +81,10 @@ func newController(cmd *basecmd.AdapterBase, metricsCache *metriccache.MetricCac } adapterInformerFactory := informers.NewSharedInformerFactory(adapterClientSet, time.Second*30) - handler := controller.NewHandler(adapterInformerFactory.Azure().V1alpha1().ExternalMetrics().Lister(), metricsCache) - controller := controller.NewController(adapterInformerFactory.Azure().V1alpha1().ExternalMetrics(), &handler) + + controller := controller.NewController(adapterInformerFactory.Azure().V1alpha1().ExternalMetrics(), + adapterInformerFactory.Azure().V1alpha1().CustomMetrics(), &handler) return controller, adapterInformerFactory } diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index e7203514..5de51201 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -16,22 +16,26 @@ import ( // Controller will do the work of syncing the external metrics the metric adapter knows about. type Controller struct { - externalMetricqueue workqueue.RateLimitingInterface + metricQueue workqueue.RateLimitingInterface externalMetricSynced cache.InformerSynced - handler ContollerHandler + customMetricSynced cache.InformerSynced enqueuer func(obj interface{}) + metricHandler ContollerHandler } // NewController returns a new controller for handling external and custom metric types -func NewController(externalMetricInformer informers.ExternalMetricInformer, handler ContollerHandler) *Controller { +func NewController(externalMetricInformer informers.ExternalMetricInformer, customMetricInformer informers.CustomMetricInformer, metricHandler ContollerHandler) *Controller { controller := &Controller{ externalMetricSynced: externalMetricInformer.Informer().HasSynced, - externalMetricqueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "externalmetrics"), - handler: handler, + metricQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "metrics"), + metricHandler: metricHandler, + customMetricSynced: customMetricInformer.Informer().HasSynced, } - glog.Info("Setting up external metric event handlers") + // wire up enque step. This provides a hook for testing enqueue step controller.enqueuer = controller.enqueueExternalMetric + + glog.Info("Setting up external metric event handlers") externalMetricInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: controller.enqueuer, UpdateFunc: func(old, new interface{}) { @@ -43,18 +47,27 @@ func NewController(externalMetricInformer informers.ExternalMetricInformer, hand DeleteFunc: controller.enqueuer, }) + glog.Info("Setting up custom metric event handlers") + customMetricInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: controller.enqueuer, + UpdateFunc: func(old, new interface{}) { + controller.enqueuer(new) + }, + DeleteFunc: controller.enqueuer, + }) + return controller } // Run is the main path of execution for the controller loop func (c *Controller) Run(numberOfWorkers int, interval time.Duration, stopCh <-chan struct{}) { defer utilruntime.HandleCrash() - defer c.externalMetricqueue.ShutDown() + defer c.metricQueue.ShutDown() glog.V(2).Info("initializing controller") // do the initial synchronization (one time) to populate resources - if !cache.WaitForCacheSync(stopCh, c.externalMetricSynced) { + if !cache.WaitForCacheSync(stopCh, c.externalMetricSynced, c.customMetricSynced) { runtime.HandleError(fmt.Errorf("Error syncing controller cache")) return } @@ -82,42 +95,42 @@ func (c *Controller) runWorker() { func (c *Controller) processNextItem() bool { glog.V(2).Info("processing item") - key, quit := c.externalMetricqueue.Get() + key, quit := c.metricQueue.Get() if quit { glog.V(2).Info("recieved quit signal") return false } - defer c.externalMetricqueue.Done(key) + defer c.metricQueue.Done(key) var namespaceNameKey string var ok bool if namespaceNameKey, ok = key.(string); !ok { // not valid key do not put back on queue - c.externalMetricqueue.Forget(key) + c.metricQueue.Forget(key) runtime.HandleError(fmt.Errorf("expected string in workqueue but got %#v", key)) return true } - err := c.handler.Process(namespaceNameKey) + err := c.metricHandler.Process(namespaceNameKey) if err != nil { - retrys := c.externalMetricqueue.NumRequeues(key) + retrys := c.metricQueue.NumRequeues(key) if retrys < 5 { glog.Errorf("Transient error with %d retrys for key %s: %s", retrys, key, err) - c.externalMetricqueue.AddRateLimited(key) + c.metricQueue.AddRateLimited(key) return true } // something was wrong with the item on queue glog.Errorf("Max retries hit for key %s: %s", key, err) - c.externalMetricqueue.Forget(key) + c.metricQueue.Forget(key) utilruntime.HandleError(err) return true } //if here success for get item glog.V(2).Infof("succesfully proccessed item '%s'", namespaceNameKey) - c.externalMetricqueue.Forget(key) + c.metricQueue.Forget(key) return true } @@ -130,5 +143,5 @@ func (c *Controller) enqueueExternalMetric(obj interface{}) { } glog.V(2).Infof("adding item to queue for '%s'", key) - c.externalMetricqueue.AddRateLimited(key) + c.metricQueue.AddRateLimited(key) } diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index beb47828..7c59213b 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -20,6 +20,7 @@ type controllerConfig struct { // process to the store store []runtime.Object externalMetricsListerCache []*api.ExternalMetric + customMetricsListerCache []*api.CustomMetric syncedFunction cache.InformerSynced enqueuer func(c *Controller) func(obj interface{}) handler ContollerHandler @@ -40,15 +41,7 @@ type testConfig struct { want wanted } -func testStore() []runtime.Object { - var storeObjects []runtime.Object - - externalMetric := newExternalMetric() - storeObjects = append(storeObjects, externalMetric) - return storeObjects -} - -func testListerCache() []*api.ExternalMetric { +func testExternalListerCache() []*api.ExternalMetric { var externalMetricsListerCache []*api.ExternalMetric externalMetric := newExternalMetric() @@ -56,11 +49,14 @@ func testListerCache() []*api.ExternalMetric { return externalMetricsListerCache } -func TestProcessRunsToCompletion(t *testing.T) { +func TestProcessRunsToCompletionWithExternalMetric(t *testing.T) { + var storeObjects []runtime.Object + externalMetric := newExternalMetric() + storeObjects = append(storeObjects, externalMetric) testConfig := testConfig{ controllerConfig: controllerConfig{ - store: testStore(), + store: storeObjects, syncedFunction: alwaysSynced, handler: succesFakeHandler{}, runtimes: 1, @@ -74,11 +70,57 @@ func TestProcessRunsToCompletion(t *testing.T) { runControllerTests(testConfig, t) } -func TestFailedProcessorReEnqueues(t *testing.T) { +func TestProcessRunsToCompletionWithCustomMetrics(t *testing.T) { + var storeObjects []runtime.Object + customMetric := newCustomMetric() + storeObjects = append(storeObjects, customMetric) testConfig := testConfig{ controllerConfig: controllerConfig{ - store: testStore(), + store: storeObjects, + syncedFunction: alwaysSynced, + handler: succesFakeHandler{}, + runtimes: 1, + }, + want: wanted{ + itemsRemaing: 0, + keepRunning: true, + }, + } + + runControllerTests(testConfig, t) +} + +func TestProcessRunsToCompletionWithCustomAndExternalMetrics(t *testing.T) { + var storeObjects []runtime.Object + externalMetric := newExternalMetric() + customMetric := newCustomMetric() + storeObjects = append(storeObjects, customMetric, externalMetric) + + testConfig := testConfig{ + controllerConfig: controllerConfig{ + store: storeObjects, + syncedFunction: alwaysSynced, + handler: succesFakeHandler{}, + runtimes: 2, + }, + want: wanted{ + itemsRemaing: 0, + keepRunning: true, + }, + } + + runControllerTests(testConfig, t) +} + +func TestFailedProcessorReEnqueuesWithExternalMetrics(t *testing.T) { + var storeObjects []runtime.Object + externalMetric := newExternalMetric() + storeObjects = append(storeObjects, externalMetric) + + testConfig := testConfig{ + controllerConfig: controllerConfig{ + store: storeObjects, syncedFunction: alwaysSynced, handler: failedFakeHandler{}, runtimes: 1, @@ -93,11 +135,58 @@ func TestFailedProcessorReEnqueues(t *testing.T) { runControllerTests(testConfig, t) } -func TestRetryThenRemoveAfter5Attempts(t *testing.T) { +func TestFailedProcessorReEnqueuesWithCustomMetric(t *testing.T) { + var storeObjects []runtime.Object + customMetric := newCustomMetric() + storeObjects = append(storeObjects, customMetric) testConfig := testConfig{ controllerConfig: controllerConfig{ - store: testStore(), + store: storeObjects, + syncedFunction: alwaysSynced, + handler: failedFakeHandler{}, + runtimes: 1, + }, + want: wanted{ + itemsRemaing: 1, + keepRunning: true, + enqueCount: 2, // should be two because it got added two second time on failure + }, + } + + runControllerTests(testConfig, t) +} + +func TestRetryThenRemoveAfter5AttemptsWithExternalMetric(t *testing.T) { + var storeObjects []runtime.Object + externalMetric := newExternalMetric() + storeObjects = append(storeObjects, externalMetric) + + testConfig := testConfig{ + controllerConfig: controllerConfig{ + store: storeObjects, + syncedFunction: alwaysSynced, + handler: failedFakeHandler{}, + runtimes: 5, + }, + want: wanted{ + itemsRemaing: 0, + keepRunning: true, + enqueCount: 0, // will be zero after it gets removed + }, + } + + runControllerTests(testConfig, t) +} + +func TestRetryThenRemoveAfter5AttemptsWithCustomMetric(t *testing.T) { + var storeObjects []runtime.Object + customMetric := newCustomMetric() + storeObjects = append(storeObjects, customMetric) + + testConfig := testConfig{ + controllerConfig: controllerConfig{ + store: storeObjects, syncedFunction: alwaysSynced, handler: failedFakeHandler{}, runtimes: 5, @@ -120,20 +209,24 @@ func TestInvalidItemOnQueue(t *testing.T) { // this pushes the object on instead of the key which // will cause an error - c.externalMetricqueue.AddRateLimited(obj) + c.metricQueue.AddRateLimited(obj) } return enquer } + var storeObjects []runtime.Object + externalMetric := newExternalMetric() + storeObjects = append(storeObjects, externalMetric) + testConfig := testConfig{ controllerConfig: controllerConfig{ - store: testStore(), + store: storeObjects, syncedFunction: alwaysSynced, enqueuer: badenquer, handler: succesFakeHandler{}, runtimes: 1, - externalMetricsListerCache: testListerCache(), + externalMetricsListerCache: testExternalListerCache(), }, want: wanted{ itemsRemaing: 0, @@ -166,13 +259,13 @@ func runControllerTests(testConfig testConfig, t *testing.T) { t.Errorf("should continue processing = %v, want %v", keepRunning, testConfig.want.keepRunning) } - items := c.externalMetricqueue.Len() + items := c.metricQueue.Len() if items != testConfig.want.itemsRemaing { t.Errorf("Items still on queue = %v, want %v", items, testConfig.want.itemsRemaing) } - retrys := c.externalMetricqueue.NumRequeues("default/test") + retrys := c.metricQueue.NumRequeues("default/test") if retrys != testConfig.want.enqueCount { t.Errorf("Items enqueued times = %v, want %v", retrys, testConfig.want.enqueCount) } @@ -182,10 +275,11 @@ func newController(config controllerConfig) (*Controller, informers.SharedInform fakeClient := fake.NewSimpleClientset(config.store...) i := informers.NewSharedInformerFactory(fakeClient, 0) - c := NewController(i.Azure().V1alpha1().ExternalMetrics(), config.handler) + c := NewController(i.Azure().V1alpha1().ExternalMetrics(), i.Azure().V1alpha1().CustomMetrics(), config.handler) // override for testing c.externalMetricSynced = config.syncedFunction + c.customMetricSynced = config.syncedFunction if config.enqueuer != nil { // override for testings @@ -193,13 +287,18 @@ func newController(config controllerConfig) (*Controller, informers.SharedInform } // override so the item gets added right away for testing with no delay - c.externalMetricqueue = workqueue.NewNamedRateLimitingQueue(NoDelyRateLimiter(), "nodelay") + c.metricQueue = workqueue.NewNamedRateLimitingQueue(NoDelyRateLimiter(), "nodelay") for _, em := range config.externalMetricsListerCache { // this will force the enqueuer to reload i.Azure().V1alpha1().ExternalMetrics().Informer().GetIndexer().Add(em) } + for _, cm := range config.customMetricsListerCache { + // this will force the enqueuer to reload + i.Azure().V1alpha1().CustomMetrics().Informer().GetIndexer().Add(cm) + } + return c, i } @@ -217,6 +316,19 @@ func newExternalMetric() *api.ExternalMetric { } } +func newCustomMetric() *api.CustomMetric { + return &api.CustomMetric{ + TypeMeta: metav1.TypeMeta{APIVersion: api.SchemeGroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: metav1.NamespaceDefault, + }, + Spec: api.CustomMetricSpec{ + MetricConfig: api.CustomMetricConfig{}, + }, + } +} + type succesFakeHandler struct{} func (h succesFakeHandler) Process(key string) error { From 08a31c3d2c24fb2fc3d37622176d69b4bdd4a53f Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Tue, 2 Oct 2018 15:29:23 -0700 Subject: [PATCH 02/17] Fix Typo in Controller Inferface --- pkg/controller/controller.go | 4 ++-- pkg/controller/controller_test.go | 2 +- pkg/controller/handler.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 5de51201..43eb6c33 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -20,11 +20,11 @@ type Controller struct { externalMetricSynced cache.InformerSynced customMetricSynced cache.InformerSynced enqueuer func(obj interface{}) - metricHandler ContollerHandler + metricHandler ControllerHandler } // NewController returns a new controller for handling external and custom metric types -func NewController(externalMetricInformer informers.ExternalMetricInformer, customMetricInformer informers.CustomMetricInformer, metricHandler ContollerHandler) *Controller { +func NewController(externalMetricInformer informers.ExternalMetricInformer, customMetricInformer informers.CustomMetricInformer, metricHandler ControllerHandler) *Controller { controller := &Controller{ externalMetricSynced: externalMetricInformer.Informer().HasSynced, metricQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "metrics"), diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 7c59213b..6a3a4c22 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -23,7 +23,7 @@ type controllerConfig struct { customMetricsListerCache []*api.CustomMetric syncedFunction cache.InformerSynced enqueuer func(c *Controller) func(obj interface{}) - handler ContollerHandler + handler ControllerHandler runtimes int } diff --git a/pkg/controller/handler.go b/pkg/controller/handler.go index 2b6c3091..1c76040b 100644 --- a/pkg/controller/handler.go +++ b/pkg/controller/handler.go @@ -26,7 +26,7 @@ func NewHandler(externalmetricLister listers.ExternalMetricLister, metricCache * } } -type ContollerHandler interface { +type ControllerHandler interface { Process(namespaceNameKey string) error } From 92496b099a59312626ee844d31f37e891930f335 Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Tue, 2 Oct 2018 20:13:25 -0700 Subject: [PATCH 03/17] Implement cache storage for custom metric --- main.go | 4 +- pkg/apis/metrics/v1alpha1/custommetric.go | 6 +- pkg/controller/controller.go | 54 ++++-- pkg/controller/controller_test.go | 27 ++- pkg/controller/handler.go | 54 +++++- pkg/controller/handler_test.go | 218 +++++++++++++++++++--- pkg/metriccache/metric_cache.go | 36 +++- pkg/provider/provider_external.go | 4 +- pkg/provider/provider_external_test.go | 4 +- 9 files changed, 338 insertions(+), 69 deletions(-) diff --git a/main.go b/main.go index b1b5b77e..4bd11c6d 100755 --- a/main.go +++ b/main.go @@ -81,7 +81,9 @@ func newController(cmd *basecmd.AdapterBase, metricsCache *metriccache.MetricCac } adapterInformerFactory := informers.NewSharedInformerFactory(adapterClientSet, time.Second*30) - handler := controller.NewHandler(adapterInformerFactory.Azure().V1alpha1().ExternalMetrics().Lister(), metricsCache) + handler := controller.NewHandler(adapterInformerFactory.Azure().V1alpha1().ExternalMetrics().Lister(), + adapterInformerFactory.Azure().V1alpha1().CustomMetrics().Lister(), + metricsCache) controller := controller.NewController(adapterInformerFactory.Azure().V1alpha1().ExternalMetrics(), adapterInformerFactory.Azure().V1alpha1().CustomMetrics(), &handler) diff --git a/pkg/apis/metrics/v1alpha1/custommetric.go b/pkg/apis/metrics/v1alpha1/custommetric.go index f4593665..6bf16d69 100755 --- a/pkg/apis/metrics/v1alpha1/custommetric.go +++ b/pkg/apis/metrics/v1alpha1/custommetric.go @@ -28,9 +28,9 @@ type CustomMetricSpec struct { // CustomMetricConfig holds app insights configuration type CustomMetricConfig struct { - metricName string `json:"metricName"` - applicationID string `json:"applicationID"` - query string `json:"query"` + MetricName string `json:"metricName"` + ApplicationID string `json:"applicationID"` + Query string `json:"query"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 43eb6c33..2194c710 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -4,6 +4,8 @@ import ( "fmt" "time" + "k8s.io/apimachinery/pkg/api/meta" + "github.com/golang/glog" "k8s.io/apimachinery/pkg/util/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -95,42 +97,42 @@ func (c *Controller) runWorker() { func (c *Controller) processNextItem() bool { glog.V(2).Info("processing item") - key, quit := c.metricQueue.Get() + rawItem, quit := c.metricQueue.Get() if quit { glog.V(2).Info("recieved quit signal") return false } - defer c.metricQueue.Done(key) + defer c.metricQueue.Done(rawItem) - var namespaceNameKey string + var queueItem namespacedQueueItem var ok bool - if namespaceNameKey, ok = key.(string); !ok { + if queueItem, ok = rawItem.(namespacedQueueItem); !ok { // not valid key do not put back on queue - c.metricQueue.Forget(key) - runtime.HandleError(fmt.Errorf("expected string in workqueue but got %#v", key)) + c.metricQueue.Forget(rawItem) + runtime.HandleError(fmt.Errorf("expected string in workqueue but got %#v", rawItem)) return true } - err := c.metricHandler.Process(namespaceNameKey) + err := c.metricHandler.Process(queueItem) if err != nil { - retrys := c.metricQueue.NumRequeues(key) + retrys := c.metricQueue.NumRequeues(rawItem) if retrys < 5 { - glog.Errorf("Transient error with %d retrys for key %s: %s", retrys, key, err) - c.metricQueue.AddRateLimited(key) + glog.Errorf("Transient error with %d retrys for key %s: %s", retrys, rawItem, err) + c.metricQueue.AddRateLimited(rawItem) return true } // something was wrong with the item on queue - glog.Errorf("Max retries hit for key %s: %s", key, err) - c.metricQueue.Forget(key) + glog.Errorf("Max retries hit for key %s: %s", rawItem, err) + c.metricQueue.Forget(rawItem) utilruntime.HandleError(err) return true } //if here success for get item - glog.V(2).Infof("succesfully proccessed item '%s'", namespaceNameKey) - c.metricQueue.Forget(key) + glog.V(2).Infof("succesfully proccessed item '%s'", queueItem) + c.metricQueue.Forget(rawItem) return true } @@ -142,6 +144,26 @@ func (c *Controller) enqueueExternalMetric(obj interface{}) { return } - glog.V(2).Infof("adding item to queue for '%s'", key) - c.metricQueue.AddRateLimited(key) + t, err := meta.TypeAccessor(obj) + if err != nil { + runtime.HandleError(err) + return + } + + kind := t.GetKind() + + glog.V(2).Infof("adding item to queue for '%s' with kind '%s'", key, kind) + c.metricQueue.AddRateLimited(namespacedQueueItem{ + namespaceKey: key, + kind: kind, + }) +} + +type namespacedQueueItem struct { + namespaceKey string + kind string +} + +func (q namespacedQueueItem) Key() string { + return fmt.Sprintf("%s/%s", q.namespaceKey, q.kind) } diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 6a3a4c22..53864ba8 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -34,6 +34,7 @@ type wanted struct { // number of times added two queue // will be zero if the item was forgeten enqueCount int + enquedItem namespacedQueueItem } type testConfig struct { @@ -129,6 +130,10 @@ func TestFailedProcessorReEnqueuesWithExternalMetrics(t *testing.T) { itemsRemaing: 1, keepRunning: true, enqueCount: 2, // should be two because it got added two second time on failure + enquedItem: namespacedQueueItem{ + namespaceKey: "default/test", + kind: "ExternalMetric", + }, }, } @@ -151,6 +156,10 @@ func TestFailedProcessorReEnqueuesWithCustomMetric(t *testing.T) { itemsRemaing: 1, keepRunning: true, enqueCount: 2, // should be two because it got added two second time on failure + enquedItem: namespacedQueueItem{ + namespaceKey: "default/test", + kind: "CustomMetric", + }, }, } @@ -173,6 +182,10 @@ func TestRetryThenRemoveAfter5AttemptsWithExternalMetric(t *testing.T) { itemsRemaing: 0, keepRunning: true, enqueCount: 0, // will be zero after it gets removed + enquedItem: namespacedQueueItem{ + namespaceKey: "default/test", + kind: "ExternalMetric", + }, }, } @@ -195,6 +208,10 @@ func TestRetryThenRemoveAfter5AttemptsWithCustomMetric(t *testing.T) { itemsRemaing: 0, keepRunning: true, enqueCount: 0, // will be zero after it gets removed + enquedItem: namespacedQueueItem{ + namespaceKey: "default/test", + kind: "CustomMetric", + }, }, } @@ -265,7 +282,7 @@ func runControllerTests(testConfig testConfig, t *testing.T) { t.Errorf("Items still on queue = %v, want %v", items, testConfig.want.itemsRemaing) } - retrys := c.metricQueue.NumRequeues("default/test") + retrys := c.metricQueue.NumRequeues(testConfig.want.enquedItem) if retrys != testConfig.want.enqueCount { t.Errorf("Items enqueued times = %v, want %v", retrys, testConfig.want.enqueCount) } @@ -304,7 +321,7 @@ func newController(config controllerConfig) (*Controller, informers.SharedInform func newExternalMetric() *api.ExternalMetric { return &api.ExternalMetric{ - TypeMeta: metav1.TypeMeta{APIVersion: api.SchemeGroupVersion.String()}, + TypeMeta: metav1.TypeMeta{APIVersion: api.SchemeGroupVersion.String(), Kind: "ExternalMetric"}, ObjectMeta: metav1.ObjectMeta{ Name: "test", Namespace: metav1.NamespaceDefault, @@ -318,7 +335,7 @@ func newExternalMetric() *api.ExternalMetric { func newCustomMetric() *api.CustomMetric { return &api.CustomMetric{ - TypeMeta: metav1.TypeMeta{APIVersion: api.SchemeGroupVersion.String()}, + TypeMeta: metav1.TypeMeta{APIVersion: api.SchemeGroupVersion.String(), Kind: "CustomMetric"}, ObjectMeta: metav1.ObjectMeta{ Name: "test", Namespace: metav1.NamespaceDefault, @@ -331,13 +348,13 @@ func newCustomMetric() *api.CustomMetric { type succesFakeHandler struct{} -func (h succesFakeHandler) Process(key string) error { +func (h succesFakeHandler) Process(key namespacedQueueItem) error { return nil } type failedFakeHandler struct{} -func (h failedFakeHandler) Process(key string) error { +func (h failedFakeHandler) Process(key namespacedQueueItem) error { return errors.New("this fake always fails") } diff --git a/pkg/controller/handler.go b/pkg/controller/handler.go index 1c76040b..fa54b7f5 100644 --- a/pkg/controller/handler.go +++ b/pkg/controller/handler.go @@ -3,6 +3,8 @@ package controller import ( "fmt" + "github.com/Azure/azure-k8s-metrics-adapter/pkg/azure/appinsights" + "github.com/Azure/azure-k8s-metrics-adapter/pkg/azure/monitor" listers "github.com/Azure/azure-k8s-metrics-adapter/pkg/client/listers/metrics/v1alpha1" "github.com/Azure/azure-k8s-metrics-adapter/pkg/metriccache" @@ -16,29 +18,67 @@ import ( type Handler struct { externalmetricLister listers.ExternalMetricLister metriccache *metriccache.MetricCache + customMetricLister listers.CustomMetricLister } // NewHandler created a new handler -func NewHandler(externalmetricLister listers.ExternalMetricLister, metricCache *metriccache.MetricCache) Handler { +func NewHandler(externalmetricLister listers.ExternalMetricLister, customMetricLister listers.CustomMetricLister, metricCache *metriccache.MetricCache) Handler { return Handler{ externalmetricLister: externalmetricLister, + customMetricLister: customMetricLister, metriccache: metricCache, } } type ControllerHandler interface { - Process(namespaceNameKey string) error + Process(queueItem namespacedQueueItem) error } // Process validates the item exists then stores updates the metric cached used to make requests to azure -func (h *Handler) Process(namespaceNameKey string) error { - ns, name, err := cache.SplitMetaNamespaceKey(namespaceNameKey) +func (h *Handler) Process(queueItem namespacedQueueItem) error { + ns, name, err := cache.SplitMetaNamespaceKey(queueItem.namespaceKey) if err != nil { // not a valid key do not put back on queue - runtime.HandleError(fmt.Errorf("expected namespace/name key in workqueue but got %s", namespaceNameKey)) + runtime.HandleError(fmt.Errorf("expected namespace/name key in workqueue but got %s", queueItem.namespaceKey)) return err } + switch queueItem.kind { + case "CustomMetric": + return h.handleCustomMetric(ns, name, queueItem) + case "ExternalMetric": + return h.handleExternalMetric(ns, name, queueItem) + } + + return nil +} + +func (h *Handler) handleCustomMetric(ns, name string, namespaceNameKey namespacedQueueItem) error { + // check if item exists + glog.V(2).Infof("processing item '%s' in namespace '%s'", name, ns) + customMetricInfo, err := h.customMetricLister.CustomMetrics(ns).Get(name) + if err != nil { + if errors.IsNotFound(err) { + // Then this we should remove + glog.V(2).Infof("removing item from cache '%s' in namespace '%s'", name, ns) + h.metriccache.Remove(namespaceNameKey.Key()) + return nil + } + + return err + } + + metric := appinsights.MetricRequest{ + MetricName: customMetricInfo.Spec.MetricConfig.MetricName, + } + + glog.V(2).Infof("adding to cache item '%s' in namespace '%s'", name, ns) + h.metriccache.Update(namespaceNameKey.Key(), metric) + + return nil +} + +func (h *Handler) handleExternalMetric(ns, name string, namespaceNameKey namespacedQueueItem) error { // check if item exists glog.V(2).Infof("processing item '%s' in namespace '%s'", name, ns) externalMetricInfo, err := h.externalmetricLister.ExternalMetrics(ns).Get(name) @@ -46,7 +86,7 @@ func (h *Handler) Process(namespaceNameKey string) error { if errors.IsNotFound(err) { // Then this we should remove glog.V(2).Infof("removing item from cache '%s' in namespace '%s'", name, ns) - h.metriccache.Remove(namespaceNameKey) + h.metriccache.Remove(namespaceNameKey.Key()) return nil } @@ -65,7 +105,7 @@ func (h *Handler) Process(namespaceNameKey string) error { } glog.V(2).Infof("adding to cache item '%s' in namespace '%s'", name, ns) - h.metriccache.Update(namespaceNameKey, azureMetricRequest) + h.metriccache.Update(namespaceNameKey.Key(), azureMetricRequest) return nil } diff --git a/pkg/controller/handler_test.go b/pkg/controller/handler_test.go index a514d920..e0602784 100644 --- a/pkg/controller/handler_test.go +++ b/pkg/controller/handler_test.go @@ -4,6 +4,8 @@ import ( "fmt" "testing" + "github.com/Azure/azure-k8s-metrics-adapter/pkg/azure/appinsights" + api "github.com/Azure/azure-k8s-metrics-adapter/pkg/apis/metrics/v1alpha1" "github.com/Azure/azure-k8s-metrics-adapter/pkg/azure/monitor" "github.com/Azure/azure-k8s-metrics-adapter/pkg/metriccache" @@ -14,103 +16,249 @@ import ( informers "github.com/Azure/azure-k8s-metrics-adapter/pkg/client/informers/externalversions" ) -func getKey(externalMetric *api.ExternalMetric) string { - return fmt.Sprintf("%s/%s", externalMetric.Namespace, externalMetric.Name) +func getExternalKey(externalMetric *api.ExternalMetric) namespacedQueueItem { + return namespacedQueueItem{ + namespaceKey: fmt.Sprintf("%s/%s", externalMetric.Namespace, externalMetric.Name), + kind: externalMetric.TypeMeta.Kind, + } } -func TestMetricValueIsStored(t *testing.T) { +func getCustomKey(customMetric *api.CustomMetric) namespacedQueueItem { + return namespacedQueueItem{ + namespaceKey: fmt.Sprintf("%s/%s", customMetric.Namespace, customMetric.Name), + kind: customMetric.TypeMeta.Kind, + } +} + +func TestExternalMetricValueIsStored(t *testing.T) { var storeObjects []runtime.Object var externalMetricsListerCache []*api.ExternalMetric + var customMetricsListerCache []*api.CustomMetric externalMetric := newFullExternalMetric("test") storeObjects = append(storeObjects, externalMetric) externalMetricsListerCache = append(externalMetricsListerCache, externalMetric) - handler, metriccache := newHandler(storeObjects, externalMetricsListerCache) + handler, metriccache := newHandler(storeObjects, externalMetricsListerCache, customMetricsListerCache) + + queueItem := getExternalKey(externalMetric) + err := handler.Process(queueItem) + + if err != nil { + t.Errorf("error after processing = %v, want %v", err, nil) + } + + metricRequest, exists := metriccache.GetAzureMonitorRequest(queueItem.Key()) + + if exists == false { + t.Errorf("exist = %v, want %v", exists, true) + } + + validateExternalMetricResult(metricRequest, externalMetric, t) +} + +func TestShouldBeAbleToStoreCustomAndExternalWithSameNameAndNamespace(t *testing.T) { + var storeObjects []runtime.Object + var externalMetricsListerCache []*api.ExternalMetric + var customMetricsListerCache []*api.CustomMetric + + externalMetric := newFullExternalMetric("test") + customMetric := newFullCustomMetric("test") + storeObjects = append(storeObjects, externalMetric, customMetric) + externalMetricsListerCache = append(externalMetricsListerCache, externalMetric) + customMetricsListerCache = append(customMetricsListerCache, customMetric) + + handler, metriccache := newHandler(storeObjects, externalMetricsListerCache, customMetricsListerCache) + + externalItem := getExternalKey(externalMetric) + err := handler.Process(externalItem) + + if err != nil { + t.Errorf("error after processing = %v, want %v", err, nil) + } + + customItem := getCustomKey(customMetric) + err = handler.Process(customItem) + + if err != nil { + t.Errorf("error after processing = %v, want %v", err, nil) + } + + externalRequest, exists := metriccache.GetAzureMonitorRequest(externalItem.Key()) - key := getKey(externalMetric) - err := handler.Process(key) + if exists == false { + t.Errorf("exist = %v, want %v", exists, true) + } + + validateExternalMetricResult(externalRequest, externalMetric, t) + + metricRequest, exists := metriccache.GetAppInsightsRequest(customItem.Key()) + + if exists == false { + t.Errorf("exist = %v, want %v", exists, true) + } + + validateCustomMetricResult(metricRequest, customMetric, t) +} + +func TestCustomMetricValueIsStored(t *testing.T) { + var storeObjects []runtime.Object + var externalMetricsListerCache []*api.ExternalMetric + var customMetricsListerCache []*api.CustomMetric + + customMetric := newFullCustomMetric("test") + storeObjects = append(storeObjects, customMetric) + customMetricsListerCache = append(customMetricsListerCache, customMetric) + + handler, metriccache := newHandler(storeObjects, externalMetricsListerCache, customMetricsListerCache) + + queueItem := getCustomKey(customMetric) + err := handler.Process(queueItem) if err != nil { t.Errorf("error after processing = %v, want %v", err, nil) } - metricRequest, exists := metriccache.Get(key) + metricRequest, exists := metriccache.GetAppInsightsRequest(queueItem.Key()) if exists == false { t.Errorf("exist = %v, want %v", exists, true) } - validateMetricResult(metricRequest, externalMetric, t) + validateCustomMetricResult(metricRequest, customMetric, t) } func TestShouldFailOnInvalidCacheKey(t *testing.T) { var storeObjects []runtime.Object var externalMetricsListerCache []*api.ExternalMetric + var customMetricsListerCache []*api.CustomMetric externalMetric := newFullExternalMetric("test") storeObjects = append(storeObjects, externalMetric) externalMetricsListerCache = append(externalMetricsListerCache, externalMetric) - handler, metriccache := newHandler(storeObjects, externalMetricsListerCache) + handler, metriccache := newHandler(storeObjects, externalMetricsListerCache, customMetricsListerCache) - key := "invalidkey/with/extrainfo" - err := handler.Process(key) + queueItem := namespacedQueueItem{ + namespaceKey: "invalidkey/with/extrainfo", + kind: "somethingwrong", + } + err := handler.Process(queueItem) if err == nil { t.Errorf("error after processing nil, want non nil") } - _, exists := metriccache.Get(key) + _, exists := metriccache.GetAzureMonitorRequest(queueItem.Key()) if exists == true { t.Errorf("exist = %v, want %v", exists, false) } } -func TestWhenItemHasBeenDeleted(t *testing.T) { +func TestWhenExternalItemHasBeenDeleted(t *testing.T) { var storeObjects []runtime.Object var externalMetricsListerCache []*api.ExternalMetric + var customMetricsListerCache []*api.CustomMetric externalMetric := newFullExternalMetric("test") // don't put anything in the stores - handler, metriccache := newHandler(storeObjects, externalMetricsListerCache) + handler, metriccache := newHandler(storeObjects, externalMetricsListerCache, customMetricsListerCache) // add the item to the cache then test if it gets deleted - key := getKey(externalMetric) - metriccache.Update(key, monitor.AzureMetricRequest{}) + queueItem := getExternalKey(externalMetric) + metriccache.Update(queueItem.Key(), monitor.AzureMetricRequest{}) - err := handler.Process(key) + err := handler.Process(queueItem) if err != nil { t.Errorf("error == %v, want nil", err) } - _, exists := metriccache.Get(key) + _, exists := metriccache.GetAzureMonitorRequest(queueItem.Key()) if exists == true { t.Errorf("exist = %v, want %v", exists, false) } } -func newHandler(storeObjects []runtime.Object, externalMetricsListerCache []*api.ExternalMetric) (Handler, *metriccache.MetricCache) { +func TestWhenCustomItemHasBeenDeleted(t *testing.T) { + var storeObjects []runtime.Object + var externalMetricsListerCache []*api.ExternalMetric + var customMetricsListerCache []*api.CustomMetric + + customMetric := newFullCustomMetric("test") + + // don't put anything in the stores + handler, metriccache := newHandler(storeObjects, externalMetricsListerCache, customMetricsListerCache) + + // add the item to the cache then test if it gets deleted + queueItem := getCustomKey(customMetric) + metriccache.Update(queueItem.Key(), appinsights.MetricRequest{}) + + err := handler.Process(queueItem) + + if err != nil { + t.Errorf("error == %v, want nil", err) + } + + _, exists := metriccache.GetAppInsightsRequest(queueItem.Key()) + + if exists == true { + t.Errorf("exist = %v, want %v", exists, false) + } +} + +func TestWhenItemKindIsUnknown(t *testing.T) { + var storeObjects []runtime.Object + var externalMetricsListerCache []*api.ExternalMetric + var customMetricsListerCache []*api.CustomMetric + + // don't put anything in the stores, as we are not looking anything up + handler, metriccache := newHandler(storeObjects, externalMetricsListerCache, customMetricsListerCache) + + // add the item to the cache then test if it gets deleted + queueItem := namespacedQueueItem{ + namespaceKey: "Default/unknown", + kind: "Unknown", + } + + err := handler.Process(queueItem) + + if err != nil { + t.Errorf("error == %v, want nil", err) + } + + _, exists := metriccache.GetAppInsightsRequest(queueItem.Key()) + + if exists == true { + t.Errorf("exist = %v, want %v", exists, false) + } +} + +func newHandler(storeObjects []runtime.Object, externalMetricsListerCache []*api.ExternalMetric, customMetricsListerCache []*api.CustomMetric) (Handler, *metriccache.MetricCache) { fakeClient := fake.NewSimpleClientset(storeObjects...) i := informers.NewSharedInformerFactory(fakeClient, 0) - lister := i.Azure().V1alpha1().ExternalMetrics().Lister() + externalMetricLister := i.Azure().V1alpha1().ExternalMetrics().Lister() + customMetricLister := i.Azure().V1alpha1().CustomMetrics().Lister() for _, em := range externalMetricsListerCache { i.Azure().V1alpha1().ExternalMetrics().Informer().GetIndexer().Add(em) } + for _, cm := range customMetricsListerCache { + i.Azure().V1alpha1().CustomMetrics().Informer().GetIndexer().Add(cm) + } + metriccache := metriccache.NewMetricCache() - handler := NewHandler(lister, metriccache) + handler := NewHandler(externalMetricLister, customMetricLister, metriccache) return handler, metriccache } -func validateMetricResult(metricRequest monitor.AzureMetricRequest, externalMetricInfo *api.ExternalMetric, t *testing.T) { +func validateExternalMetricResult(metricRequest monitor.AzureMetricRequest, externalMetricInfo *api.ExternalMetric, t *testing.T) { // Metric Config if metricRequest.MetricName != externalMetricInfo.Spec.MetricConfig.MetricName { @@ -148,10 +296,18 @@ func validateMetricResult(metricRequest monitor.AzureMetricRequest, externalMetr } +func validateCustomMetricResult(metricRequest appinsights.MetricRequest, customMetricInfo *api.CustomMetric, t *testing.T) { + // Metric Config + if metricRequest.MetricName != customMetricInfo.Spec.MetricConfig.MetricName { + t.Errorf("metricRequest MetricName = %v, want %v", metricRequest.MetricName, customMetricInfo.Spec.MetricConfig.MetricName) + } + +} + func newFullExternalMetric(name string) *api.ExternalMetric { // must preserve upper casing for azure api return &api.ExternalMetric{ - TypeMeta: metav1.TypeMeta{APIVersion: api.SchemeGroupVersion.String()}, + TypeMeta: metav1.TypeMeta{APIVersion: api.SchemeGroupVersion.String(), Kind: "ExternalMetric"}, ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: metav1.NamespaceDefault, @@ -171,3 +327,19 @@ func newFullExternalMetric(name string) *api.ExternalMetric { }, } } + +func newFullCustomMetric(name string) *api.CustomMetric { + // must preserve upper casing for azure api + return &api.CustomMetric{ + TypeMeta: metav1.TypeMeta{APIVersion: api.SchemeGroupVersion.String(), Kind: "CustomMetric"}, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: metav1.NamespaceDefault, + }, + Spec: api.CustomMetricSpec{ + MetricConfig: api.CustomMetricConfig{ + MetricName: "performance/requestpersecond", + }, + }, + } +} diff --git a/pkg/metriccache/metric_cache.go b/pkg/metriccache/metric_cache.go index 2d3e9943..feebc101 100644 --- a/pkg/metriccache/metric_cache.go +++ b/pkg/metriccache/metric_cache.go @@ -3,43 +3,59 @@ package metriccache import ( "sync" + "github.com/Azure/azure-k8s-metrics-adapter/pkg/azure/appinsights" + "github.com/Azure/azure-k8s-metrics-adapter/pkg/azure/monitor" "github.com/golang/glog" ) // MetricCache holds the loaded metric request info in the system type MetricCache struct { - metricMutext sync.RWMutex - metrics map[string]monitor.AzureMetricRequest + metricMutext sync.RWMutex + metricRequests map[string]interface{} } // NewMetricCache creates the cache func NewMetricCache() *MetricCache { return &MetricCache{ - metrics: make(map[string]monitor.AzureMetricRequest), + metricRequests: make(map[string]interface{}), } } // Update sets a metric request in the cache -func (mc *MetricCache) Update(key string, metricRequest monitor.AzureMetricRequest) { +func (mc *MetricCache) Update(key string, metricRequest interface{}) { mc.metricMutext.Lock() defer mc.metricMutext.Unlock() - mc.metrics[key] = metricRequest + mc.metricRequests[key] = metricRequest } -// Get retrieves a metric request from the cache -func (mc *MetricCache) Get(key string) (monitor.AzureMetricRequest, bool) { +// GetAzureMonitorRequest retrieves a metric request from the cache +func (mc *MetricCache) GetAzureMonitorRequest(key string) (monitor.AzureMetricRequest, bool) { mc.metricMutext.RLock() defer mc.metricMutext.RUnlock() - metricRequest, exists := mc.metrics[key] + metricRequest, exists := mc.metricRequests[key] if !exists { glog.V(2).Infof("metric not found %s", key) return monitor.AzureMetricRequest{}, false } - return metricRequest, true + return metricRequest.(monitor.AzureMetricRequest), true +} + +// GetAppInsightsRequest retrieves a metric request from the cache +func (mc *MetricCache) GetAppInsightsRequest(key string) (appinsights.MetricRequest, bool) { + mc.metricMutext.RLock() + defer mc.metricMutext.RUnlock() + + metricRequest, exists := mc.metricRequests[key] + if !exists { + glog.V(2).Infof("metric not found %s", key) + return appinsights.MetricRequest{}, false + } + + return metricRequest.(appinsights.MetricRequest), true } // Remove retrieves a metric request from the cache @@ -47,5 +63,5 @@ func (mc *MetricCache) Remove(key string) { mc.metricMutext.Lock() defer mc.metricMutext.Unlock() - delete(mc.metrics, key) + delete(mc.metricRequests, key) } diff --git a/pkg/provider/provider_external.go b/pkg/provider/provider_external.go index d0219790..7731108b 100644 --- a/pkg/provider/provider_external.go +++ b/pkg/provider/provider_external.go @@ -72,7 +72,7 @@ func (p *AzureProvider) ListAllExternalMetrics() []provider.ExternalMetricInfo { func (p *AzureProvider) getMetricRequest(namespace string, metricName string, metricSelector labels.Selector) (monitor.AzureMetricRequest, error) { key := metricKey(namespace, metricName) - azMetricRequest, found := p.metricCache.Get(key) + azMetricRequest, found := p.metricCache.GetAzureMonitorRequest(key) if found { azMetricRequest.Timespan = monitor.TimeSpan() if azMetricRequest.SubscriptionID == "" { @@ -90,5 +90,5 @@ func (p *AzureProvider) getMetricRequest(namespace string, metricName string, me } func metricKey(namespace string, name string) string { - return fmt.Sprintf("%s/%s", namespace, name) + return fmt.Sprintf("%s/%s/ExternalMetric", namespace, name) } diff --git a/pkg/provider/provider_external_test.go b/pkg/provider/provider_external_test.go index d095082f..4333c65a 100644 --- a/pkg/provider/provider_external_test.go +++ b/pkg/provider/provider_external_test.go @@ -36,7 +36,7 @@ func TestFindMetricInCache(t *testing.T) { request := monitor.AzureMetricRequest{ MetricName: "MessageCount", } - metricCache.Update("default/metricname", request) + metricCache.Update("default/metricname/ExternalMetric", request) provider := AzureProvider{ metricCache: metricCache, @@ -70,7 +70,7 @@ func TestFindMetricInCacheUsesOverrideSubscriptionId(t *testing.T) { MetricName: "MessageCount", SubscriptionID: "9876", } - metricCache.Update("default/metricname", request) + metricCache.Update("default/metricname/ExternalMetric", request) provider := AzureProvider{ metricCache: metricCache, From 2ab5cc74b4f01df0d6182ab443347ab5015235c8 Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Wed, 3 Oct 2018 15:27:52 -0700 Subject: [PATCH 04/17] Key generation is in cache --- pkg/controller/handler.go | 12 ++++++------ pkg/controller/handler_test.go | 18 +++++++++--------- pkg/metriccache/metric_cache.go | 15 +++++++++++++-- pkg/provider/provider_external.go | 9 +-------- 4 files changed, 29 insertions(+), 25 deletions(-) diff --git a/pkg/controller/handler.go b/pkg/controller/handler.go index fa54b7f5..aa3ec22a 100644 --- a/pkg/controller/handler.go +++ b/pkg/controller/handler.go @@ -53,7 +53,7 @@ func (h *Handler) Process(queueItem namespacedQueueItem) error { return nil } -func (h *Handler) handleCustomMetric(ns, name string, namespaceNameKey namespacedQueueItem) error { +func (h *Handler) handleCustomMetric(ns, name string, queueItem namespacedQueueItem) error { // check if item exists glog.V(2).Infof("processing item '%s' in namespace '%s'", name, ns) customMetricInfo, err := h.customMetricLister.CustomMetrics(ns).Get(name) @@ -61,7 +61,7 @@ func (h *Handler) handleCustomMetric(ns, name string, namespaceNameKey namespace if errors.IsNotFound(err) { // Then this we should remove glog.V(2).Infof("removing item from cache '%s' in namespace '%s'", name, ns) - h.metriccache.Remove(namespaceNameKey.Key()) + h.metriccache.Remove(queueItem.Key()) return nil } @@ -73,12 +73,12 @@ func (h *Handler) handleCustomMetric(ns, name string, namespaceNameKey namespace } glog.V(2).Infof("adding to cache item '%s' in namespace '%s'", name, ns) - h.metriccache.Update(namespaceNameKey.Key(), metric) + h.metriccache.Update(queueItem.Key(), metric) return nil } -func (h *Handler) handleExternalMetric(ns, name string, namespaceNameKey namespacedQueueItem) error { +func (h *Handler) handleExternalMetric(ns, name string, queueItem namespacedQueueItem) error { // check if item exists glog.V(2).Infof("processing item '%s' in namespace '%s'", name, ns) externalMetricInfo, err := h.externalmetricLister.ExternalMetrics(ns).Get(name) @@ -86,7 +86,7 @@ func (h *Handler) handleExternalMetric(ns, name string, namespaceNameKey namespa if errors.IsNotFound(err) { // Then this we should remove glog.V(2).Infof("removing item from cache '%s' in namespace '%s'", name, ns) - h.metriccache.Remove(namespaceNameKey.Key()) + h.metriccache.Remove(queueItem.Key()) return nil } @@ -105,7 +105,7 @@ func (h *Handler) handleExternalMetric(ns, name string, namespaceNameKey namespa } glog.V(2).Infof("adding to cache item '%s' in namespace '%s'", name, ns) - h.metriccache.Update(namespaceNameKey.Key(), azureMetricRequest) + h.metriccache.Update(queueItem.Key(), azureMetricRequest) return nil } diff --git a/pkg/controller/handler_test.go b/pkg/controller/handler_test.go index e0602784..efe0bd1c 100644 --- a/pkg/controller/handler_test.go +++ b/pkg/controller/handler_test.go @@ -48,7 +48,7 @@ func TestExternalMetricValueIsStored(t *testing.T) { t.Errorf("error after processing = %v, want %v", err, nil) } - metricRequest, exists := metriccache.GetAzureMonitorRequest(queueItem.Key()) + metricRequest, exists := metriccache.GetAzureMonitorRequest(externalMetric.Namespace, externalMetric.Name) if exists == false { t.Errorf("exist = %v, want %v", exists, true) @@ -84,7 +84,7 @@ func TestShouldBeAbleToStoreCustomAndExternalWithSameNameAndNamespace(t *testing t.Errorf("error after processing = %v, want %v", err, nil) } - externalRequest, exists := metriccache.GetAzureMonitorRequest(externalItem.Key()) + externalRequest, exists := metriccache.GetAzureMonitorRequest(externalMetric.Namespace, externalMetric.Name) if exists == false { t.Errorf("exist = %v, want %v", exists, true) @@ -92,7 +92,7 @@ func TestShouldBeAbleToStoreCustomAndExternalWithSameNameAndNamespace(t *testing validateExternalMetricResult(externalRequest, externalMetric, t) - metricRequest, exists := metriccache.GetAppInsightsRequest(customItem.Key()) + metricRequest, exists := metriccache.GetAppInsightsRequest(customMetric.Namespace, customMetric.Name) if exists == false { t.Errorf("exist = %v, want %v", exists, true) @@ -119,7 +119,7 @@ func TestCustomMetricValueIsStored(t *testing.T) { t.Errorf("error after processing = %v, want %v", err, nil) } - metricRequest, exists := metriccache.GetAppInsightsRequest(queueItem.Key()) + metricRequest, exists := metriccache.GetAppInsightsRequest(customMetric.Namespace, customMetric.Name) if exists == false { t.Errorf("exist = %v, want %v", exists, true) @@ -149,7 +149,7 @@ func TestShouldFailOnInvalidCacheKey(t *testing.T) { t.Errorf("error after processing nil, want non nil") } - _, exists := metriccache.GetAzureMonitorRequest(queueItem.Key()) + _, exists := metriccache.GetAzureMonitorRequest(externalMetric.Namespace, externalMetric.Name) if exists == true { t.Errorf("exist = %v, want %v", exists, false) @@ -176,7 +176,7 @@ func TestWhenExternalItemHasBeenDeleted(t *testing.T) { t.Errorf("error == %v, want nil", err) } - _, exists := metriccache.GetAzureMonitorRequest(queueItem.Key()) + _, exists := metriccache.GetAzureMonitorRequest(externalMetric.Namespace, externalMetric.Name) if exists == true { t.Errorf("exist = %v, want %v", exists, false) @@ -203,7 +203,7 @@ func TestWhenCustomItemHasBeenDeleted(t *testing.T) { t.Errorf("error == %v, want nil", err) } - _, exists := metriccache.GetAppInsightsRequest(queueItem.Key()) + _, exists := metriccache.GetAppInsightsRequest(customMetric.Namespace, customMetric.Name) if exists == true { t.Errorf("exist = %v, want %v", exists, false) @@ -220,7 +220,7 @@ func TestWhenItemKindIsUnknown(t *testing.T) { // add the item to the cache then test if it gets deleted queueItem := namespacedQueueItem{ - namespaceKey: "Default/unknown", + namespaceKey: "default/unknown", kind: "Unknown", } @@ -230,7 +230,7 @@ func TestWhenItemKindIsUnknown(t *testing.T) { t.Errorf("error == %v, want nil", err) } - _, exists := metriccache.GetAppInsightsRequest(queueItem.Key()) + _, exists := metriccache.GetAppInsightsRequest("default", "unkown") if exists == true { t.Errorf("exist = %v, want %v", exists, false) diff --git a/pkg/metriccache/metric_cache.go b/pkg/metriccache/metric_cache.go index feebc101..0ba36782 100644 --- a/pkg/metriccache/metric_cache.go +++ b/pkg/metriccache/metric_cache.go @@ -1,6 +1,7 @@ package metriccache import ( + "fmt" "sync" "github.com/Azure/azure-k8s-metrics-adapter/pkg/azure/appinsights" @@ -31,10 +32,11 @@ func (mc *MetricCache) Update(key string, metricRequest interface{}) { } // GetAzureMonitorRequest retrieves a metric request from the cache -func (mc *MetricCache) GetAzureMonitorRequest(key string) (monitor.AzureMetricRequest, bool) { +func (mc *MetricCache) GetAzureMonitorRequest(namepace, name string) (monitor.AzureMetricRequest, bool) { mc.metricMutext.RLock() defer mc.metricMutext.RUnlock() + key := externalMetricKey(namepace, name) metricRequest, exists := mc.metricRequests[key] if !exists { glog.V(2).Infof("metric not found %s", key) @@ -45,10 +47,11 @@ func (mc *MetricCache) GetAzureMonitorRequest(key string) (monitor.AzureMetricRe } // GetAppInsightsRequest retrieves a metric request from the cache -func (mc *MetricCache) GetAppInsightsRequest(key string) (appinsights.MetricRequest, bool) { +func (mc *MetricCache) GetAppInsightsRequest(namespace, name string) (appinsights.MetricRequest, bool) { mc.metricMutext.RLock() defer mc.metricMutext.RUnlock() + key := customMetricKey(namespace, name) metricRequest, exists := mc.metricRequests[key] if !exists { glog.V(2).Infof("metric not found %s", key) @@ -65,3 +68,11 @@ func (mc *MetricCache) Remove(key string) { delete(mc.metricRequests, key) } + +func externalMetricKey(namespace string, name string) string { + return fmt.Sprintf("%s/%s/ExternalMetric", namespace, name) +} + +func customMetricKey(namespace string, name string) string { + return fmt.Sprintf("%s/%s/CustomMetric", namespace, name) +} diff --git a/pkg/provider/provider_external.go b/pkg/provider/provider_external.go index 7731108b..80ad3ca8 100644 --- a/pkg/provider/provider_external.go +++ b/pkg/provider/provider_external.go @@ -3,8 +3,6 @@ package provider import ( - "fmt" - "github.com/Azure/azure-k8s-metrics-adapter/pkg/azure/monitor" "github.com/golang/glog" "github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/provider" @@ -70,9 +68,8 @@ func (p *AzureProvider) ListAllExternalMetrics() []provider.ExternalMetricInfo { } func (p *AzureProvider) getMetricRequest(namespace string, metricName string, metricSelector labels.Selector) (monitor.AzureMetricRequest, error) { - key := metricKey(namespace, metricName) - azMetricRequest, found := p.metricCache.GetAzureMonitorRequest(key) + azMetricRequest, found := p.metricCache.GetAzureMonitorRequest(namespace, metricName) if found { azMetricRequest.Timespan = monitor.TimeSpan() if azMetricRequest.SubscriptionID == "" { @@ -88,7 +85,3 @@ func (p *AzureProvider) getMetricRequest(namespace string, metricName string, me return azMetricRequest, nil } - -func metricKey(namespace string, name string) string { - return fmt.Sprintf("%s/%s/ExternalMetric", namespace, name) -} From ff05181658ff68312f838f142a254e20bacb6a2b Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Wed, 3 Oct 2018 16:20:49 -0700 Subject: [PATCH 05/17] Move to creating metric request outside client --- pkg/azure/appinsights/appinsights.go | 19 +++++++------------ pkg/provider/provider_custom.go | 10 +++++++++- pkg/provider/provider_custom_test.go | 4 +++- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/pkg/azure/appinsights/appinsights.go b/pkg/azure/appinsights/appinsights.go index 35c2b427..66a5e34b 100755 --- a/pkg/azure/appinsights/appinsights.go +++ b/pkg/azure/appinsights/appinsights.go @@ -11,7 +11,6 @@ import ( "log" "net/http" "os" - "strings" "github.com/Azure/azure-sdk-for-go/services/appinsights/v1/insights" "github.com/Azure/go-autorest/autorest/azure/auth" @@ -26,7 +25,7 @@ const ( // AzureAppInsightsClient provides methods for accessing App Insights via AD auth or App API Key type AzureAppInsightsClient interface { - GetCustomMetric(namespace string, metricName string) (float64, error) + GetCustomMetric(request MetricRequest) (float64, error) } // appinsightsClient is used to call Application Insights Api @@ -51,20 +50,16 @@ func NewClient() AzureAppInsightsClient { } // GetCustomMetric calls to Application Insights to retrieve the value of the metric requested -func (c appinsightsClient) GetCustomMetric(namespace string, metricName string) (float64, error) { - // because metrics names are multipart in AI and we can not pass an extra / - // through k8s api we convert - to / to get around that - convertedMetricName := strings.Replace(metricName, "-", "/", -1) - glog.V(2).Infof("New call to GetCustomMetric: %s", convertedMetricName) +func (c appinsightsClient) GetCustomMetric(request MetricRequest) (float64, error) { // get the last 5 mins and chunking into 30 seconds // this seems to be the best way to get the closest average rate at time of request // any smaller time intervals and the values come back null - metricRequestInfo := NewMetricRequest(convertedMetricName) - metricRequestInfo.Timespan = "PT5M" - metricRequestInfo.Interval = "PT30S" + // TODO make this configurable? + request.Timespan = "PT5M" + request.Interval = "PT30S" - metricsResult, err := c.getMetric(metricRequestInfo) + metricsResult, err := c.getMetric(request) if err != nil { return 0, err } @@ -80,7 +75,7 @@ func (c appinsightsClient) GetCustomMetric(namespace string, metricName string) } // grab just the last value which will be the latest value of the metric - metric := segments[len(segments)-1].AdditionalProperties[convertedMetricName] + metric := segments[len(segments)-1].AdditionalProperties[request.MetricName] metricMap := metric.(map[string]interface{}) value := metricMap["avg"] normalizedValue := normalizeValue(value) diff --git a/pkg/provider/provider_custom.go b/pkg/provider/provider_custom.go index b796415c..037a2cf0 100644 --- a/pkg/provider/provider_custom.go +++ b/pkg/provider/provider_custom.go @@ -3,8 +3,10 @@ package provider import ( + "strings" "time" + "github.com/Azure/azure-k8s-metrics-adapter/pkg/azure/appinsights" "github.com/golang/glog" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" @@ -34,8 +36,14 @@ func (p *AzureProvider) GetMetricBySelector(namespace string, selector labels.Se return nil, errors.NewBadRequest("label is set to not selectable. this should not happen") } + // because metrics names are multipart in AI and we can not pass an extra / + // through k8s api we convert - to / to get around that + convertedMetricName := strings.Replace(info.Metric, "-", "/", -1) + glog.V(2).Infof("New call to GetCustomMetric: %s", convertedMetricName) + metricRequestInfo := appinsights.NewMetricRequest(convertedMetricName) + // TODO use selector info to restric metric query to specific app. - val, err := p.appinsightsClient.GetCustomMetric(namespace, info.Metric) + val, err := p.appinsightsClient.GetCustomMetric(metricRequestInfo) if err != nil { glog.Errorf("bad request: %v", err) return nil, errors.NewBadRequest(err.Error()) diff --git a/pkg/provider/provider_custom_test.go b/pkg/provider/provider_custom_test.go index 3e003932..59a13920 100644 --- a/pkg/provider/provider_custom_test.go +++ b/pkg/provider/provider_custom_test.go @@ -5,6 +5,8 @@ import ( "testing" "time" + "github.com/Azure/azure-k8s-metrics-adapter/pkg/azure/appinsights" + "github.com/Azure/azure-k8s-metrics-adapter/pkg/metriccache" "github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/dynamicmapper" k8sprovider "github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/provider" @@ -107,6 +109,6 @@ type fakeAppInsightsClient struct { err error } -func (f fakeAppInsightsClient) GetCustomMetric(namespace string, metricName string) (float64, error) { +func (f fakeAppInsightsClient) GetCustomMetric(request appinsights.MetricRequest) (float64, error) { return f.result, f.err } From 8dbb38faa3b32a0fa484528a3e621c92a569b857 Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Wed, 3 Oct 2018 16:39:17 -0700 Subject: [PATCH 06/17] Wire in CRD for custom metric --- pkg/provider/provider_custom.go | 26 ++++++++---- pkg/provider/provider_custom_test.go | 59 ++++++++++++++++++++++++---- 2 files changed, 70 insertions(+), 15 deletions(-) diff --git a/pkg/provider/provider_custom.go b/pkg/provider/provider_custom.go index 037a2cf0..42608e31 100644 --- a/pkg/provider/provider_custom.go +++ b/pkg/provider/provider_custom.go @@ -36,11 +36,7 @@ func (p *AzureProvider) GetMetricBySelector(namespace string, selector labels.Se return nil, errors.NewBadRequest("label is set to not selectable. this should not happen") } - // because metrics names are multipart in AI and we can not pass an extra / - // through k8s api we convert - to / to get around that - convertedMetricName := strings.Replace(info.Metric, "-", "/", -1) - glog.V(2).Infof("New call to GetCustomMetric: %s", convertedMetricName) - metricRequestInfo := appinsights.NewMetricRequest(convertedMetricName) + metricRequestInfo := p.getCustomMetricRequest(namespace, selector, info) // TODO use selector info to restric metric query to specific app. val, err := p.appinsightsClient.GetCustomMetric(metricRequestInfo) @@ -59,10 +55,10 @@ func (p *AzureProvider) GetMetricBySelector(namespace string, selector labels.Se DescribedObject: custom_metrics.ObjectReference{ APIVersion: info.GroupResource.Group + "/" + runtime.APIVersionInternal, Kind: kind.Kind, - Name: info.Metric, + Name: metricRequestInfo.MetricName, Namespace: namespace, }, - MetricName: info.Metric, + MetricName: metricRequestInfo.MetricName, Timestamp: metav1.Time{time.Now()}, Value: *resource.NewMilliQuantity(int64(val*1000), resource.DecimalSI), } @@ -83,3 +79,19 @@ func (p *AzureProvider) ListAllMetrics() []provider.CustomMetricInfo { // not implemented yet return []provider.CustomMetricInfo{} } + +func (p *AzureProvider) getCustomMetricRequest(namespace string, selector labels.Selector, info provider.CustomMetricInfo) appinsights.MetricRequest { + + cachedRequest, found := p.metricCache.GetAppInsightsRequest(namespace, info.Metric) + if found { + return cachedRequest + } + + // because metrics names are multipart in AI and we can not pass an extra / + // through k8s api we convert - to / to get around that + convertedMetricName := strings.Replace(info.Metric, "-", "/", -1) + glog.V(2).Infof("New call to GetCustomMetric: %s", convertedMetricName) + metricRequestInfo := appinsights.NewMetricRequest(convertedMetricName) + + return metricRequestInfo +} diff --git a/pkg/provider/provider_custom_test.go b/pkg/provider/provider_custom_test.go index 59a13920..3d763713 100644 --- a/pkg/provider/provider_custom_test.go +++ b/pkg/provider/provider_custom_test.go @@ -19,7 +19,43 @@ import ( core "k8s.io/client-go/testing" ) -func TestReturnsCustomMetric(t *testing.T) { +func TestReturnsCustomMetricConverted(t *testing.T) { + + fakeClient := fakeAppInsightsClient{ + result: 15, + err: nil, + } + + selector, _ := labels.Parse("") + info := k8sprovider.CustomMetricInfo{ + Metric: "Metric-Name", + GroupResource: schema.GroupResource{ + Resource: "pods", + }, + } + + provider, _ := newFakeCustomProvider(fakeClient) + returnList, err := provider.GetMetricBySelector("default", selector, info) + + if err != nil { + t.Errorf("error after processing got: %v, want nil", err) + } + + if len(returnList.Items) != 1 { + t.Errorf("returnList.Items length = %v, want there 1", len(returnList.Items)) + } + + customMetric := returnList.Items[0] + if customMetric.MetricName != "Metric/Name" { + t.Errorf("customMetric.MetricName = %v, want there %v", customMetric.MetricName, "Metric/Name") + } + + if customMetric.Value.MilliValue() != int64(15000) { + t.Errorf("customMetric.Value.MilliValue() = %v, want there %v", customMetric.Value.MilliValue(), int64(15000)) + } +} + +func TestReturnsCustomMetricWhenInCache(t *testing.T) { fakeClient := fakeAppInsightsClient{ result: 15, @@ -34,7 +70,14 @@ func TestReturnsCustomMetric(t *testing.T) { }, } - provider := newFakeCustomProvider(fakeClient) + provider, cache := newFakeCustomProvider(fakeClient) + + request := appinsights.MetricRequest{ + MetricName: "cachedName", + } + + cache.Update("default/MetricName/CustomMetric", request) + returnList, err := provider.GetMetricBySelector("default", selector, info) if err != nil { @@ -46,12 +89,12 @@ func TestReturnsCustomMetric(t *testing.T) { } customMetric := returnList.Items[0] - if customMetric.MetricName != info.Metric { - t.Errorf("externalMetric.MetricName = %v, want there %v", customMetric.MetricName, info.Metric) + if customMetric.MetricName != request.MetricName { + t.Errorf("customMetric.MetricName = %v, want there %v", customMetric.MetricName, request.MetricName) } if customMetric.Value.MilliValue() != int64(15000) { - t.Errorf("externalMetric.Value.MilliValue() = %v, want there %v", customMetric.Value.MilliValue(), int64(15000)) + t.Errorf("customMetric.Value.MilliValue() = %v, want there %v", customMetric.Value.MilliValue(), int64(15000)) } } @@ -69,7 +112,7 @@ func TestReturnsErrorIfAppInsightsFails(t *testing.T) { }, } - provider := newFakeCustomProvider(fakeClient) + provider, _ := newFakeCustomProvider(fakeClient) _, err := provider.GetMetricBySelector("default", selector, info) if !k8serrors.IsBadRequest(err) { @@ -77,7 +120,7 @@ func TestReturnsErrorIfAppInsightsFails(t *testing.T) { } } -func newFakeCustomProvider(fakeclient fakeAppInsightsClient) AzureProvider { +func newFakeCustomProvider(fakeclient fakeAppInsightsClient) (AzureProvider, *metriccache.MetricCache) { metricCache := metriccache.NewMetricCache() // set up a fake mapper @@ -101,7 +144,7 @@ func newFakeCustomProvider(fakeclient fakeAppInsightsClient) AzureProvider { mapper: mapper, } - return provider + return provider, metricCache } type fakeAppInsightsClient struct { From 24be3ed64b9c68c7f8875f157fb63d815558d3f9 Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Wed, 17 Oct 2018 12:35:10 -0700 Subject: [PATCH 07/17] Include custom metrics in crd allowed to read --- charts/azure-k8s-metrics-adapter/templates/cluster-role.yaml | 1 + deploy/adapter.yaml | 1 + 2 files changed, 2 insertions(+) diff --git a/charts/azure-k8s-metrics-adapter/templates/cluster-role.yaml b/charts/azure-k8s-metrics-adapter/templates/cluster-role.yaml index 4e848bf7..c76f0701 100644 --- a/charts/azure-k8s-metrics-adapter/templates/cluster-role.yaml +++ b/charts/azure-k8s-metrics-adapter/templates/cluster-role.yaml @@ -67,6 +67,7 @@ rules: - azure.com resources: - "externalmetrics" + - "custommetrics" verbs: - list - get diff --git a/deploy/adapter.yaml b/deploy/adapter.yaml index 5e096c80..9c6929e9 100644 --- a/deploy/adapter.yaml +++ b/deploy/adapter.yaml @@ -130,6 +130,7 @@ rules: - azure.com resources: - "externalmetrics" + - "custommetrics" verbs: - list - get From e1910ec80e90f25096df0db8fa2fbc258d9f9726 Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Thu, 18 Oct 2018 08:32:59 -0700 Subject: [PATCH 08/17] Use type to calculate the Kind for caching --- pkg/controller/controller.go | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 2194c710..1a49af72 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -4,7 +4,7 @@ import ( "fmt" "time" - "k8s.io/apimachinery/pkg/api/meta" + "github.com/Azure/azure-k8s-metrics-adapter/pkg/apis/metrics/v1alpha1" "github.com/golang/glog" "k8s.io/apimachinery/pkg/util/runtime" @@ -144,13 +144,7 @@ func (c *Controller) enqueueExternalMetric(obj interface{}) { return } - t, err := meta.TypeAccessor(obj) - if err != nil { - runtime.HandleError(err) - return - } - - kind := t.GetKind() + kind := getKind(obj) glog.V(2).Infof("adding item to queue for '%s' with kind '%s'", key, kind) c.metricQueue.AddRateLimited(namespacedQueueItem{ @@ -167,3 +161,28 @@ type namespacedQueueItem struct { func (q namespacedQueueItem) Key() string { return fmt.Sprintf("%s/%s", q.namespaceKey, q.kind) } + +func getKind(obj interface{}) string { + // Due to this issue https://github.com/kubernetes/apiextensions-apiserver/issues/29 + // metadata is not set on freshly set CRD's + // So the following does not work: + // t, err := meta.TypeAccessor(obj) + // kind := t.GetKind() // Kind will be blank + // + // A possible alternative to switching on type would be to use + // https://github.com/kubernetes/kubernetes/blob/7f23a743e8c23ac6489340bbb34fa6f1d392db9d/pkg/kubectl/cmd/util/conversion.go + // v := cmdUtil.AsDefaultVersionedOrOriginal(item, nil) + // k := v.GetObjectKind().GroupVersionKind().Kind + // + // Instead use type to predict Kind which is good enough for our purposes: + + switch obj.(type) { + case *v1alpha1.ExternalMetric: + return "ExternalMetric" + case *v1alpha1.CustomMetric: + return "CustomMetric" + default: + glog.Error("No known type of object") + return "" + } +} From b4b88fb0b5786dfd764c14569e42e4289e772212 Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Thu, 18 Oct 2018 08:42:48 -0700 Subject: [PATCH 09/17] Encode the cache key type/namespace/name --- pkg/controller/controller.go | 2 +- pkg/metriccache/metric_cache.go | 4 ++-- pkg/provider/provider_custom_test.go | 2 +- pkg/provider/provider_external_test.go | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 1a49af72..54cc36dc 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -159,7 +159,7 @@ type namespacedQueueItem struct { } func (q namespacedQueueItem) Key() string { - return fmt.Sprintf("%s/%s", q.namespaceKey, q.kind) + return fmt.Sprintf("%s/%s", q.kind, q.namespaceKey) } func getKind(obj interface{}) string { diff --git a/pkg/metriccache/metric_cache.go b/pkg/metriccache/metric_cache.go index 0ba36782..8b940439 100644 --- a/pkg/metriccache/metric_cache.go +++ b/pkg/metriccache/metric_cache.go @@ -70,9 +70,9 @@ func (mc *MetricCache) Remove(key string) { } func externalMetricKey(namespace string, name string) string { - return fmt.Sprintf("%s/%s/ExternalMetric", namespace, name) + return fmt.Sprintf("ExternalMetric/%s/%s", namespace, name) } func customMetricKey(namespace string, name string) string { - return fmt.Sprintf("%s/%s/CustomMetric", namespace, name) + return fmt.Sprintf("CustomMetric/%s/%s", namespace, name) } diff --git a/pkg/provider/provider_custom_test.go b/pkg/provider/provider_custom_test.go index 3d763713..867a1bfc 100644 --- a/pkg/provider/provider_custom_test.go +++ b/pkg/provider/provider_custom_test.go @@ -76,7 +76,7 @@ func TestReturnsCustomMetricWhenInCache(t *testing.T) { MetricName: "cachedName", } - cache.Update("default/MetricName/CustomMetric", request) + cache.Update("CustomMetric/default/MetricName", request) returnList, err := provider.GetMetricBySelector("default", selector, info) diff --git a/pkg/provider/provider_external_test.go b/pkg/provider/provider_external_test.go index 4333c65a..1a5241c5 100644 --- a/pkg/provider/provider_external_test.go +++ b/pkg/provider/provider_external_test.go @@ -36,7 +36,7 @@ func TestFindMetricInCache(t *testing.T) { request := monitor.AzureMetricRequest{ MetricName: "MessageCount", } - metricCache.Update("default/metricname/ExternalMetric", request) + metricCache.Update("ExternalMetric/default/metricname", request) provider := AzureProvider{ metricCache: metricCache, @@ -70,7 +70,7 @@ func TestFindMetricInCacheUsesOverrideSubscriptionId(t *testing.T) { MetricName: "MessageCount", SubscriptionID: "9876", } - metricCache.Update("default/metricname/ExternalMetric", request) + metricCache.Update("ExternalMetric/default/metricname", request) provider := AzureProvider{ metricCache: metricCache, From 1e24df8b54f97c314795c4e2e7d13450506501a2 Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Thu, 18 Oct 2018 09:31:29 -0700 Subject: [PATCH 10/17] Add App Insights config to charts --- .../templates/deployment.yaml | 12 ++++++++++++ .../azure-k8s-metrics-adapter/templates/secret.yaml | 4 ++++ charts/azure-k8s-metrics-adapter/values.yaml | 6 ++++++ local-dev-values.yaml.example | 6 +++++- 4 files changed, 27 insertions(+), 1 deletion(-) diff --git a/charts/azure-k8s-metrics-adapter/templates/deployment.yaml b/charts/azure-k8s-metrics-adapter/templates/deployment.yaml index 3911f280..1588b5c8 100644 --- a/charts/azure-k8s-metrics-adapter/templates/deployment.yaml +++ b/charts/azure-k8s-metrics-adapter/templates/deployment.yaml @@ -73,6 +73,18 @@ spec: name: {{ template "azure-k8s-metrics-adapter.fullname" . }} key: azure-subscription-id {{- end }} + {{- if .Values.appInsights.appId }} + - name: APP_INSIGHTS_APP_ID + valueFrom: + secretKeyRef: + name: {{ template "azure-k8s-metrics-adapter.fullname" . }} + key: appinsights-appid + - name: APP_INSIGHTS_KEY + valueFrom: + secretKeyRef: + name: {{ template "azure-k8s-metrics-adapter.fullname" . }} + key: appinsights-key + {{- end }} {{- range $key, $value := .Values.extraEnv }} - name: {{ $key }} value: "{{ $value }}" diff --git a/charts/azure-k8s-metrics-adapter/templates/secret.yaml b/charts/azure-k8s-metrics-adapter/templates/secret.yaml index 2f2aca74..7389fe56 100644 --- a/charts/azure-k8s-metrics-adapter/templates/secret.yaml +++ b/charts/azure-k8s-metrics-adapter/templates/secret.yaml @@ -24,4 +24,8 @@ data: {{- if .Values.defaultSubscriptionId }} azure-subscription-id: {{ .Values.defaultSubscriptionId | b64enc | quote }} {{- end }} + {{- if .Values.appInsights.appId }} + appinsights-appid: {{ .Values.appInsights.appId | b64enc | quote }} + appinsights-key: {{ .Values.appInsights.key | b64enc | quote }} + {{- end }} {{- end }} \ No newline at end of file diff --git a/charts/azure-k8s-metrics-adapter/values.yaml b/charts/azure-k8s-metrics-adapter/values.yaml index 1d7da5dd..00e7e296 100644 --- a/charts/azure-k8s-metrics-adapter/values.yaml +++ b/charts/azure-k8s-metrics-adapter/values.yaml @@ -47,6 +47,12 @@ azureAuthentication: clientCertificatePath: "" clientCertificatePassword: "" +# It is possible to pass app insights app id and key instead of using service principle +# if no application insights key has been specified, then we will use AD authentication +appInsights: + appId: "" + key: "" + # Override the subscription if outside of Azure or for full control with `SUBSCRIPTION_ID` # See https://github.com/jsturtevant/azure-k8-metrics-adapter#subscription-information defaultSubscriptionId: "" diff --git a/local-dev-values.yaml.example b/local-dev-values.yaml.example index 9391bed0..089ce60a 100644 --- a/local-dev-values.yaml.example +++ b/local-dev-values.yaml.example @@ -6,4 +6,8 @@ azureAuthentication: clientID: "" clientSecret: "" -defaultSubscriptionId: "" \ No newline at end of file +defaultSubscriptionId: "" + +appInsights: + appId: "" + key: "" \ No newline at end of file From c4723a89f90abf701a1afa37b719a410f0818e56 Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Thu, 18 Oct 2018 09:32:00 -0700 Subject: [PATCH 11/17] Fix bug with Mapper to pass through --- pkg/provider/provider.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/provider/provider.go b/pkg/provider/provider.go index 8ca77243..38ae67ff 100755 --- a/pkg/provider/provider.go +++ b/pkg/provider/provider.go @@ -24,6 +24,7 @@ type AzureProvider struct { func NewAzureProvider(defaultSubscriptionID string, mapper apimeta.RESTMapper, appinsightsClient appinsights.AzureAppInsightsClient, monitorClient monitor.AzureMonitorClient, metricCache *metriccache.MetricCache) provider.MetricsProvider { return &AzureProvider{ defaultSubscriptionID: defaultSubscriptionID, + mapper: mapper, appinsightsClient: appinsightsClient, monitorClient: monitorClient, metricCache: metricCache, From e4c8c3f3b4ab38bb902df7a344d2cc5bf66501e1 Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Thu, 18 Oct 2018 15:31:46 -0700 Subject: [PATCH 12/17] Update readme.md --- .../deploy/custom-metric.yaml | 9 +++++ samples/request-per-second/deploy/hpa.yaml | 2 +- samples/request-per-second/readme.md | 38 ++++++++++++++----- samples/servicebus-queue/readme.md | 1 - 4 files changed, 38 insertions(+), 12 deletions(-) create mode 100644 samples/request-per-second/deploy/custom-metric.yaml diff --git a/samples/request-per-second/deploy/custom-metric.yaml b/samples/request-per-second/deploy/custom-metric.yaml new file mode 100644 index 00000000..882aaf28 --- /dev/null +++ b/samples/request-per-second/deploy/custom-metric.yaml @@ -0,0 +1,9 @@ +apiVersion: azure.com/v1alpha1 +kind: CustomMetric +metadata: + name: rps +spec: + metric: + metricName: performanceCounters/requestsPerSecond + #applicationID: #optional + #query: #not implemented yet \ No newline at end of file diff --git a/samples/request-per-second/deploy/hpa.yaml b/samples/request-per-second/deploy/hpa.yaml index 6bc6ca44..cf289566 100755 --- a/samples/request-per-second/deploy/hpa.yaml +++ b/samples/request-per-second/deploy/hpa.yaml @@ -12,5 +12,5 @@ spec: metrics: - type: Pods pods: - metricName: performanceCounters-requestsPerSecond + metricName: rps targetAverageValue: 10 \ No newline at end of file diff --git a/samples/request-per-second/readme.md b/samples/request-per-second/readme.md index 8f6ab355..44e28cdf 100755 --- a/samples/request-per-second/readme.md +++ b/samples/request-per-second/readme.md @@ -11,9 +11,12 @@ This is an example on using custom metric from Application insights to scale a d - [Using Azure Application Insights API Key](#using-azure-application-insights-api-key) - [Using Azure AD Pod Identity](#using-azure-ad-pod-identity) - [Deploy the app that will be scaled](#deploy-the-app-that-will-be-scaled) +- [there is probably a better way to get at that array](#there-is-probably-a-better-way-to-get-at-that-array) - [Scale on Requests per Second (RPS)](#scale-on-requests-per-second-rps) + - [Deploy the Custom Metric Configuration](#deploy-the-custom-metric-configuration) - [Deploy the HPA](#deploy-the-hpa) - [Put it under load and scale by RPS](#put-it-under-load-and-scale-by-rps) +- [100000 requests at 100 RPS](#100000-requests-at-100-rps) - [Watch it scale](#watch-it-scale) - [Clean up](#clean-up) @@ -44,21 +47,18 @@ After the application instance is created [get your instrumentation key](https:/ ### Get your appid and api key -Get your [appid and key](https://dev.applicationinsights.io/documentation/Authorization/API-key-and-App-ID). Then create a secret for the adapter to use: +Get your [appid and key](https://dev.applicationinsights.io/documentation/Authorization/API-key-and-App-ID). Then deploy the adapter. #### Using Azure Application Insights API Key -If you want use an Application Insight API key, create the following secret: - ```bash -kubectl create secret generic app-insights-api -n custom-metrics --from-literal=app-insights-app-id= --from-literal=app-insights-key= +helm install --name sample-release ../../charts/azure-k8s-metrics-adapter --namespace custom-metrics \ + --set appInsights.appId= \ + --set appInsights.key= \ + --set azureAuthentication.createSecret=true ``` -Deploy the modified [adapter.yaml](https://gist.github.com/jsturtevant/966371df82be922e14438bcbc81f1f65) that uses the secret just created: - -```bash -kubectl apply -f https://gist.githubusercontent.com/jsturtevant/966371df82be922e14438bcbc81f1f65/raw/2ca706bcc18d20af5956c66400df69c3bb83c002/deploy.yaml -``` +> note: if you plan to use the adapter with External Metrics you may need additional configuration. See the [Service Bus Queue Example](../servicebus-queue/). #### Using Azure AD Pod Identity @@ -99,6 +99,20 @@ curl http://$RPS_ENDPOINT ## Scale on Requests per Second (RPS) +### Deploy the Custom Metric Configuration + +```bash +kubectl apply -f deploy/custom-metric.yaml +``` + +> note: the CustomMetric configuration is deployed per namespace. + +You can list of the configured custom metrics via: + +``` +kubectl get acm #shortcut for custommetric +``` + ### Deploy the HPA Deploy the HPA: @@ -107,6 +121,9 @@ Deploy the HPA: kubectl apply -f deploy/hpa.yaml ``` +> note: the `metrics.pods.metricName` defined on the HPA must match the `metadata.name` on the CustomMetric declaration, in this case `rps` + + ### Put it under load and scale by RPS [Hey](https://github.com/rakyll/hey) is a simple way to create load on an api from the command line. @@ -152,6 +169,7 @@ Also remove resources created in cluster: ```bash kubectl delete -f deploy/hpa.yaml +kubectl delete -f deploy/custom-metric.yaml kubectl delete -f deploy/rps-deployment.yaml -kubectl detele -f https://raw.githubusercontent.com/Azure/azure-k8s-metrics-adapter/master/deploy/adapter.yaml +helm delete --purge sample-release ``` \ No newline at end of file diff --git a/samples/servicebus-queue/readme.md b/samples/servicebus-queue/readme.md index b72ebf6b..b2757282 100755 --- a/samples/servicebus-queue/readme.md +++ b/samples/servicebus-queue/readme.md @@ -170,7 +170,6 @@ You can list of the configured external metrics via: ``` kubectl get aem #shortcut for externalmetric - ``` ### Deploy the HPA From 34079d83aec5849288972ee896431b1eee89f551 Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Thu, 18 Oct 2018 15:54:07 -0700 Subject: [PATCH 13/17] run gen-api code --- .../externalversions/internalinterfaces/factory_interfaces.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/client/informers/externalversions/internalinterfaces/factory_interfaces.go b/pkg/client/informers/externalversions/internalinterfaces/factory_interfaces.go index 185b3f38..6f86a7b1 100644 --- a/pkg/client/informers/externalversions/internalinterfaces/factory_interfaces.go +++ b/pkg/client/informers/externalversions/internalinterfaces/factory_interfaces.go @@ -27,7 +27,6 @@ import ( cache "k8s.io/client-go/tools/cache" ) -// NewInformerFunc takes versioned.Interface and time.Duration to return a SharedIndexInformer. type NewInformerFunc func(versioned.Interface, time.Duration) cache.SharedIndexInformer // SharedInformerFactory a small interface to allow for adding an informer without an import cycle @@ -36,5 +35,4 @@ type SharedInformerFactory interface { InformerFor(obj runtime.Object, newFunc NewInformerFunc) cache.SharedIndexInformer } -// TweakListOptionsFunc is a function that transforms a v1.ListOptions. type TweakListOptionsFunc func(*v1.ListOptions) From 9ac2333b2e5fe332360807be8379dc287951f9a9 Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Thu, 18 Oct 2018 16:26:18 -0700 Subject: [PATCH 14/17] Fix typos --- samples/request-per-second/readme.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/samples/request-per-second/readme.md b/samples/request-per-second/readme.md index 44e28cdf..e03f2103 100755 --- a/samples/request-per-second/readme.md +++ b/samples/request-per-second/readme.md @@ -11,7 +11,6 @@ This is an example on using custom metric from Application insights to scale a d - [Using Azure Application Insights API Key](#using-azure-application-insights-api-key) - [Using Azure AD Pod Identity](#using-azure-ad-pod-identity) - [Deploy the app that will be scaled](#deploy-the-app-that-will-be-scaled) -- [there is probably a better way to get at that array](#there-is-probably-a-better-way-to-get-at-that-array) - [Scale on Requests per Second (RPS)](#scale-on-requests-per-second-rps) - [Deploy the Custom Metric Configuration](#deploy-the-custom-metric-configuration) - [Deploy the HPA](#deploy-the-hpa) @@ -91,7 +90,6 @@ kubectl apply -f deploy/rps-deployment.yaml Double check you can hit the endpoint: ```bash -# there is probably a better way to get at that array export RPS_ENDPOINT="$(k get svc rps-sample -o json | jq .status.loadBalancer.ingress | jq -r '.[0]'.ip)" curl http://$RPS_ENDPOINT @@ -129,10 +127,10 @@ kubectl apply -f deploy/hpa.yaml [Hey](https://github.com/rakyll/hey) is a simple way to create load on an api from the command line. ```bash -go get -u github.com/rakyll/hey http://$RPS_ENDPOINT +go get -u github.com/rakyll/hey # 100000 requests at 100 RPS -hey -n 10000 -q 10 -c 10 +hey -n 10000 -q 10 -c 10 http://$RPS_ENDPOINT ``` ### Watch it scale From 2f465be0f2d14e0b39d09455f67e05f59e3c1adc Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Wed, 24 Oct 2018 22:37:22 -0700 Subject: [PATCH 15/17] get pod refrences and add values for each pod --- Gopkg.lock | 9 +- Gopkg.toml | 2 +- main.go | 7 +- pkg/provider/provider.go | 5 +- pkg/provider/provider_custom.go | 36 +++--- .../pkg/provider/helpers/helpers.go | 106 ++++++++++++++++++ 6 files changed, 142 insertions(+), 23 deletions(-) create mode 100644 vendor/github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/provider/helpers/helpers.go diff --git a/Gopkg.lock b/Gopkg.lock index 1345faaf..e78cf8c9 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -309,7 +309,7 @@ version = "1.1.4" [[projects]] - digest = "1:4c2988e9c7777badccfaf87473ab09dad9935c67811d2b51e143e959a934e5f3" + digest = "1:cf71ca07e2ee5914c776d9eaec4a7f63f4ddb013d55e39ffd5689fe4cd2df861" name = "github.com/kubernetes-incubator/custom-metrics-apiserver" packages = [ "pkg/apiserver", @@ -318,11 +318,13 @@ "pkg/cmd/server", "pkg/dynamicmapper", "pkg/provider", + "pkg/provider/helpers", "pkg/registry/custom_metrics", "pkg/registry/external_metrics", ] pruneopts = "UT" - revision = "1f1cda41a301080789507a291b999807002ede46" + revision = "f54b0d6f31d8e0f0a2d7be372ddd837a2ef15d97" + version = "kubernetes-1.11.2" [[projects]] branch = "master" @@ -948,8 +950,8 @@ "github.com/Azure/go-autorest/autorest/azure/auth", "github.com/golang/glog", "github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/cmd", - "github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/dynamicmapper", "github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/provider", + "github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/provider/helpers", "k8s.io/apimachinery/pkg/api/errors", "k8s.io/apimachinery/pkg/api/meta", "k8s.io/apimachinery/pkg/api/resource", @@ -966,6 +968,7 @@ "k8s.io/apiserver/pkg/util/logs", "k8s.io/client-go/discovery", "k8s.io/client-go/discovery/fake", + "k8s.io/client-go/dynamic", "k8s.io/client-go/rest", "k8s.io/client-go/testing", "k8s.io/client-go/tools/cache", diff --git a/Gopkg.toml b/Gopkg.toml index 10426b51..a02e2c7a 100755 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -7,8 +7,8 @@ ignored = ["github.com/Azure/azure-service-bus-go"] # Kubernetes incubator deps [[constraint]] - revision = "1f1cda41a301080789507a291b999807002ede46" #revision for 1.11.2 release name = "github.com/kubernetes-incubator/custom-metrics-apiserver" + version = "kubernetes-1.11.2" [[constraint]] name = "github.com/Azure/azure-sdk-for-go" diff --git a/main.go b/main.go index 4bd11c6d..e286f1e7 100755 --- a/main.go +++ b/main.go @@ -61,11 +61,16 @@ func setupAzureProvider(cmd *basecmd.AdapterBase, metricsCache *metriccache.Metr glog.Fatalf("unable to construct discovery REST mapper: %v", err) } + dynamicClient, err := cmd.DynamicClient() + if err != nil { + glog.Fatalf("unable to construct dynamic k8s client: %v", err) + } + defaultSubscriptionID := getDefaultSubscriptionID() monitorClient := monitor.NewClient(defaultSubscriptionID) appinsightsClient := appinsights.NewClient() - azureProvider := azureprovider.NewAzureProvider(defaultSubscriptionID, mapper, appinsightsClient, monitorClient, metricsCache) + azureProvider := azureprovider.NewAzureProvider(defaultSubscriptionID, mapper, dynamicClient, appinsightsClient, monitorClient, metricsCache) cmd.WithCustomMetrics(azureProvider) cmd.WithExternalMetrics(azureProvider) } diff --git a/pkg/provider/provider.go b/pkg/provider/provider.go index 38ae67ff..2a6dd184 100755 --- a/pkg/provider/provider.go +++ b/pkg/provider/provider.go @@ -9,6 +9,7 @@ import ( "github.com/Azure/azure-k8s-metrics-adapter/pkg/azure/monitor" apimeta "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/client-go/dynamic" "github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/provider" ) @@ -16,15 +17,17 @@ import ( type AzureProvider struct { appinsightsClient appinsights.AzureAppInsightsClient mapper apimeta.RESTMapper + kubeClient dynamic.Interface monitorClient monitor.AzureMonitorClient metricCache *metriccache.MetricCache defaultSubscriptionID string } -func NewAzureProvider(defaultSubscriptionID string, mapper apimeta.RESTMapper, appinsightsClient appinsights.AzureAppInsightsClient, monitorClient monitor.AzureMonitorClient, metricCache *metriccache.MetricCache) provider.MetricsProvider { +func NewAzureProvider(defaultSubscriptionID string, mapper apimeta.RESTMapper, kubeClient dynamic.Interface, appinsightsClient appinsights.AzureAppInsightsClient, monitorClient monitor.AzureMonitorClient, metricCache *metriccache.MetricCache) provider.MetricsProvider { return &AzureProvider{ defaultSubscriptionID: defaultSubscriptionID, mapper: mapper, + kubeClient: kubeClient, appinsightsClient: appinsightsClient, monitorClient: monitorClient, metricCache: metricCache, diff --git a/pkg/provider/provider_custom.go b/pkg/provider/provider_custom.go index 42608e31..2da88bf3 100644 --- a/pkg/provider/provider_custom.go +++ b/pkg/provider/provider_custom.go @@ -3,6 +3,7 @@ package provider import ( + "fmt" "strings" "time" @@ -12,11 +13,11 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/metrics/pkg/apis/custom_metrics" "github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/provider" + "github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/provider/helpers" ) // GetMetricByName fetches a particular metric for a particular object. @@ -45,26 +46,27 @@ func (p *AzureProvider) GetMetricBySelector(namespace string, selector labels.Se return nil, errors.NewBadRequest(err.Error()) } - // TODO what does version do? - kind, err := p.mapper.KindFor(info.GroupResource.WithVersion("")) + resourceNames, err := helpers.ListObjectNames(p.mapper, p.kubeClient, namespace, selector, info) if err != nil { - return nil, errors.NewBadRequest(err.Error()) - } - - metricValue := custom_metrics.MetricValue{ - DescribedObject: custom_metrics.ObjectReference{ - APIVersion: info.GroupResource.Group + "/" + runtime.APIVersionInternal, - Kind: kind.Kind, - Name: metricRequestInfo.MetricName, - Namespace: namespace, - }, - MetricName: metricRequestInfo.MetricName, - Timestamp: metav1.Time{time.Now()}, - Value: *resource.NewMilliQuantity(int64(val*1000), resource.DecimalSI), + glog.Errorf("not able to list objects from api server: %v", err) + return nil, errors.NewInternalError(fmt.Errorf("not able to list objects from api server for this resource")) } metricList := make([]custom_metrics.MetricValue, 0) - metricList = append(metricList, metricValue) + for _, name := range resourceNames { + ref, err := helpers.ReferenceFor(p.mapper, types.NamespacedName{Namespace: namespace, Name: name}, info) + if err != nil { + return nil, err + } + + metricValue := custom_metrics.MetricValue{ + DescribedObject: ref, + MetricName: info.Metric, + Timestamp: metav1.Time{time.Now()}, + Value: *resource.NewMilliQuantity(int64(val*1000), resource.DecimalSI), + } + metricList = append(metricList, metricValue) + } return &custom_metrics.MetricValueList{ Items: metricList, diff --git a/vendor/github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/provider/helpers/helpers.go b/vendor/github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/provider/helpers/helpers.go new file mode 100644 index 00000000..4c2533c9 --- /dev/null +++ b/vendor/github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/provider/helpers/helpers.go @@ -0,0 +1,106 @@ +/* +Copyright 2018 The Kubernetes Authors. + +Licensed 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 helpers + +import ( + "fmt" + + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "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/apimachinery/pkg/types" + "k8s.io/client-go/dynamic" + "k8s.io/metrics/pkg/apis/custom_metrics" + + "github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/provider" +) + +// ResourceFor attempts to resolve a single qualified resource for the given metric. +// You can use this to resolve a particular piece of CustomMetricInfo to the underlying +// resource that it describes, so that you can list matching objects in the cluster. +func ResourceFor(mapper apimeta.RESTMapper, info provider.CustomMetricInfo) (schema.GroupVersionResource, error) { + fullResources, err := mapper.ResourcesFor(info.GroupResource.WithVersion("")) + if err == nil && len(fullResources) == 0 { + err = fmt.Errorf("no fully versioned resources known for group-resource %v", info.GroupResource) + } + if err != nil { + return schema.GroupVersionResource{}, fmt.Errorf("unable to find preferred version to list matching resource names: %v", err) + } + + return fullResources[0], nil +} + +// ReferenceFor returns a new ObjectReference for the given group-resource and name. +// The group-resource is converted into a group-version-kind using the given RESTMapper. +// You can use this to easily construct an object reference for use in the DescribedObject +// field of CustomMetricInfo. +func ReferenceFor(mapper apimeta.RESTMapper, name types.NamespacedName, info provider.CustomMetricInfo) (custom_metrics.ObjectReference, error) { + kind, err := mapper.KindFor(info.GroupResource.WithVersion("")) + if err != nil { + return custom_metrics.ObjectReference{}, err + } + + // NB: return straight value, not a reference, so that the object can easily + // be copied for use multiple times with a different name. + return custom_metrics.ObjectReference{ + APIVersion: kind.Group + "/" + kind.Version, + Kind: kind.Kind, + Name: name.Name, + Namespace: name.Namespace, + }, nil +} + +// ListObjectNames uses the given dynamic client to list the names of all objects +// of the given resource matching the given selector. Namespace may be empty +// if the metric is for a root-scoped resource. +func ListObjectNames(mapper apimeta.RESTMapper, client dynamic.Interface, namespace string, selector labels.Selector, info provider.CustomMetricInfo) ([]string, error) { + res, err := ResourceFor(mapper, info) + if err != nil { + return nil, err + } + + var resClient dynamic.ResourceInterface + if info.Namespaced { + resClient = client.Resource(res).Namespace(namespace) + } else { + resClient = client.Resource(res) + } + + matchingObjectsRaw, err := resClient.List(metav1.ListOptions{LabelSelector: selector.String()}) + if err != nil { + return nil, err + } + + if !apimeta.IsListType(matchingObjectsRaw) { + return nil, fmt.Errorf("result of label selector list operation was not a list") + } + + var names []string + err = apimeta.EachListItem(matchingObjectsRaw, func(item runtime.Object) error { + objName := item.(*unstructured.Unstructured).GetName() + names = append(names, objName) + return nil + }) + if err != nil { + return nil, err + } + + return names, nil +} From 7fc230a8428da320a1fafd91dc09ea1ea8d8b716 Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Thu, 1 Nov 2018 23:00:09 -0700 Subject: [PATCH 16/17] Add tests for multipe pods --- Gopkg.lock | 11 +- Gopkg.toml | 8 +- pkg/provider/provider_custom.go | 7 +- pkg/provider/provider_custom_test.go | 86 +++- pkg/provider/provider_test.go | 20 + .../k8s.io/client-go/dynamic/fake/simple.go | 368 ++++++++++++++++++ vendor/k8s.io/client-go/rest/request.go | 65 +++- .../k8s.io/client-go/restmapper/discovery.go | 14 +- .../client-go/transport/round_trippers.go | 111 +++++- 9 files changed, 660 insertions(+), 30 deletions(-) create mode 100644 pkg/provider/provider_test.go create mode 100644 vendor/k8s.io/client-go/dynamic/fake/simple.go diff --git a/Gopkg.lock b/Gopkg.lock index e78cf8c9..1e1efc1e 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -777,12 +777,13 @@ version = "kubernetes-1.11.2" [[projects]] - digest = "1:826229ec7a9f379a29b9f4023e15efa5d934e6cf9866f71304706bcad19341b6" + digest = "1:183ad96ad76347e84c18a84719bdf0fc134a7138ba2c43e99f57a7116c28760e" name = "k8s.io/client-go" packages = [ "discovery", "discovery/fake", "dynamic", + "dynamic/fake", "informers", "informers/admissionregistration", "informers/admissionregistration/v1alpha1", @@ -909,8 +910,8 @@ "util/workqueue", ] pruneopts = "UT" - revision = "1f13a808da65775f22cbf47862c4e5898d8f4ca1" - version = "kubernetes-1.11.2" + revision = "3db8bfc8858dc9a5d6e7ef5817f58a7ca30b0c6a" + version = "kubernetes-1.11.4" [[projects]] branch = "master" @@ -950,12 +951,15 @@ "github.com/Azure/go-autorest/autorest/azure/auth", "github.com/golang/glog", "github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/cmd", + "github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/dynamicmapper", "github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/provider", "github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/provider/helpers", + "k8s.io/api/core/v1", "k8s.io/apimachinery/pkg/api/errors", "k8s.io/apimachinery/pkg/api/meta", "k8s.io/apimachinery/pkg/api/resource", "k8s.io/apimachinery/pkg/apis/meta/v1", + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured", "k8s.io/apimachinery/pkg/labels", "k8s.io/apimachinery/pkg/runtime", "k8s.io/apimachinery/pkg/runtime/schema", @@ -969,6 +973,7 @@ "k8s.io/client-go/discovery", "k8s.io/client-go/discovery/fake", "k8s.io/client-go/dynamic", + "k8s.io/client-go/dynamic/fake", "k8s.io/client-go/rest", "k8s.io/client-go/testing", "k8s.io/client-go/tools/cache", diff --git a/Gopkg.toml b/Gopkg.toml index a02e2c7a..17d36aac 100755 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -13,7 +13,13 @@ ignored = ["github.com/Azure/azure-service-bus-go"] [[constraint]] name = "github.com/Azure/azure-sdk-for-go" version = "21.0.0" - + +# temp till Kubernetes incubator deps gets update +# https://github.com/kubernetes/kubernetes/pull/66078 +[[constraint]] + name = "k8s.io/client-go" + version = "kubernetes-1.11.4" + [prune] go-tests = true unused-packages = true \ No newline at end of file diff --git a/pkg/provider/provider_custom.go b/pkg/provider/provider_custom.go index 2da88bf3..2108fce6 100644 --- a/pkg/provider/provider_custom.go +++ b/pkg/provider/provider_custom.go @@ -39,7 +39,7 @@ func (p *AzureProvider) GetMetricBySelector(namespace string, selector labels.Se metricRequestInfo := p.getCustomMetricRequest(namespace, selector, info) - // TODO use selector info to restric metric query to specific app. + // TODO use selector info to restrict metric query to specific app. val, err := p.appinsightsClient.GetCustomMetric(metricRequestInfo) if err != nil { glog.Errorf("bad request: %v", err) @@ -52,6 +52,11 @@ func (p *AzureProvider) GetMetricBySelector(namespace string, selector labels.Se return nil, errors.NewInternalError(fmt.Errorf("not able to list objects from api server for this resource")) } + // TODO: Add support for app insights where pods are mapped 1 to 1. + // Currently App insights does not out of the box support kubernetes pod information + // so we are using the value from AI and passing to all instances of the pods. + // We should be passing pod level metric info to App insights but there is currently on the developer to wire that up and + // maping it here based on pod name. metricList := make([]custom_metrics.MetricValue, 0) for _, name := range resourceNames { ref, err := helpers.ReferenceFor(p.mapper, types.NamespacedName{Namespace: namespace, Name: name}, info) diff --git a/pkg/provider/provider_custom_test.go b/pkg/provider/provider_custom_test.go index 867a1bfc..801ca61b 100644 --- a/pkg/provider/provider_custom_test.go +++ b/pkg/provider/provider_custom_test.go @@ -2,19 +2,24 @@ package provider import ( "errors" + "fmt" "testing" "time" "github.com/Azure/azure-k8s-metrics-adapter/pkg/azure/appinsights" + "github.com/Azure/azure-k8s-metrics-adapter/pkg/client/clientset/versioned/scheme" "github.com/Azure/azure-k8s-metrics-adapter/pkg/metriccache" "github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/dynamicmapper" k8sprovider "github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/provider" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" k8serrors "k8s.io/apimachinery/pkg/api/errors" + k8sclient "k8s.io/client-go/dynamic/fake" core "k8s.io/client-go/testing" ) @@ -28,13 +33,18 @@ func TestReturnsCustomMetricConverted(t *testing.T) { selector, _ := labels.Parse("") info := k8sprovider.CustomMetricInfo{ - Metric: "Metric-Name", + Namespaced: true, + Metric: "Metric-Name", GroupResource: schema.GroupResource{ Resource: "pods", }, } - provider, _ := newFakeCustomProvider(fakeClient) + var storeObjects []runtime.Object + pod := newUnstructured("v1", "Pod", "default", "pod1") + storeObjects = append(storeObjects, pod) + + provider, _ := newFakeCustomProvider(fakeClient, storeObjects) returnList, err := provider.GetMetricBySelector("default", selector, info) if err != nil { @@ -46,7 +56,7 @@ func TestReturnsCustomMetricConverted(t *testing.T) { } customMetric := returnList.Items[0] - if customMetric.MetricName != "Metric/Name" { + if customMetric.MetricName != "Metric-Name" { t.Errorf("customMetric.MetricName = %v, want there %v", customMetric.MetricName, "Metric/Name") } @@ -55,6 +65,53 @@ func TestReturnsCustomMetricConverted(t *testing.T) { } } +func TestReturnsCustomMetricConvertedWithMultiplePods(t *testing.T) { + fakeClient := fakeAppInsightsClient{ + result: 15, + err: nil, + } + + selector, _ := labels.Parse("") + info := k8sprovider.CustomMetricInfo{ + Namespaced: true, + Metric: "Metric-Name", + GroupResource: schema.GroupResource{ + Resource: "pods", + }, + } + + var storeObjects []runtime.Object + pod := newUnstructured("v1", "Pod", "default", "pod0") + pod2 := newUnstructured("v1", "Pod", "default", "pod1") + pod3 := newUnstructured("v1", "Pod", "default", "pod2") + storeObjects = append(storeObjects, pod, pod2, pod3) + + provider, _ := newFakeCustomProvider(fakeClient, storeObjects) + returnList, err := provider.GetMetricBySelector("default", selector, info) + + if err != nil { + t.Errorf("error after processing got: %v, want nil", err) + } + + if len(returnList.Items) != 3 { + t.Errorf("returnList.Items length = %v, want there 3", len(returnList.Items)) + } + + for i, customMetric := range returnList.Items { + if customMetric.MetricName != "Metric-Name" { + t.Errorf("customMetric.MetricName = %v, want there %v", customMetric.MetricName, "Metric/Name") + } + + if customMetric.Value.MilliValue() != int64(15000) { + t.Errorf("customMetric.Value.MilliValue() = %v, want there %v", customMetric.Value.MilliValue(), int64(15000)) + } + + if customMetric.DescribedObject.Name != fmt.Sprintf("pod%d", i) { + t.Errorf("customMetric.Value.MilliValue() = %v, want there %v", customMetric.Value.MilliValue(), int64(15000)) + } + } +} + func TestReturnsCustomMetricWhenInCache(t *testing.T) { fakeClient := fakeAppInsightsClient{ @@ -70,7 +127,11 @@ func TestReturnsCustomMetricWhenInCache(t *testing.T) { }, } - provider, cache := newFakeCustomProvider(fakeClient) + var storeObjects []runtime.Object + pod := newUnstructured("v1", "Pod", "default", "pod1") + storeObjects = append(storeObjects, pod) + + provider, cache := newFakeCustomProvider(fakeClient, storeObjects) request := appinsights.MetricRequest{ MetricName: "cachedName", @@ -89,7 +150,7 @@ func TestReturnsCustomMetricWhenInCache(t *testing.T) { } customMetric := returnList.Items[0] - if customMetric.MetricName != request.MetricName { + if customMetric.MetricName != "MetricName" { t.Errorf("customMetric.MetricName = %v, want there %v", customMetric.MetricName, request.MetricName) } @@ -112,7 +173,11 @@ func TestReturnsErrorIfAppInsightsFails(t *testing.T) { }, } - provider, _ := newFakeCustomProvider(fakeClient) + var storeObjects []runtime.Object + pod := newUnstructured("v1", "Pod", "default", "pod1") + storeObjects = append(storeObjects, pod) + + provider, _ := newFakeCustomProvider(fakeClient, storeObjects) _, err := provider.GetMetricBySelector("default", selector, info) if !k8serrors.IsBadRequest(err) { @@ -120,7 +185,7 @@ func TestReturnsErrorIfAppInsightsFails(t *testing.T) { } } -func newFakeCustomProvider(fakeclient fakeAppInsightsClient) (AzureProvider, *metriccache.MetricCache) { +func newFakeCustomProvider(fakeclient fakeAppInsightsClient, store []runtime.Object) (AzureProvider, *metriccache.MetricCache) { metricCache := metriccache.NewMetricCache() // set up a fake mapper @@ -138,10 +203,17 @@ func newFakeCustomProvider(fakeclient fakeAppInsightsClient) (AzureProvider, *me mapper.RegenerateMappings() + // set up fake dynamic client + s := scheme.Scheme + corev1.SchemeBuilder.AddToScheme(s) + + fakeK8sClient := k8sclient.NewSimpleDynamicClient(s, store...) + provider := AzureProvider{ metricCache: metricCache, appinsightsClient: fakeclient, mapper: mapper, + kubeClient: fakeK8sClient, } return provider, metricCache diff --git a/pkg/provider/provider_test.go b/pkg/provider/provider_test.go new file mode 100644 index 00000000..b70048d2 --- /dev/null +++ b/pkg/provider/provider_test.go @@ -0,0 +1,20 @@ +package provider + +import "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + +// This function is from dynamic fake +// and is licensed under the Apache License, Version 2.0 (the "License"); +// https://github.com/kubernetes/kubernetes/blob/bf9a868e8ea3d3a8fa53cbb22f566771b3f8068b/staging/src/k8s.io/client-go/dynamic/fake/simple_test.go#L30 +func newUnstructured(apiVersion, kind, namespace, name string) *unstructured.Unstructured { + return &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": apiVersion, + "kind": kind, + "metadata": map[string]interface{}{ + "namespace": namespace, + "name": name, + }, + "spec": name, + }, + } +} diff --git a/vendor/k8s.io/client-go/dynamic/fake/simple.go b/vendor/k8s.io/client-go/dynamic/fake/simple.go new file mode 100644 index 00000000..7be914c9 --- /dev/null +++ b/vendor/k8s.io/client-go/dynamic/fake/simple.go @@ -0,0 +1,368 @@ +/* +Copyright 2018 The Kubernetes Authors. + +Licensed 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 fake + +import ( + "strings" + + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "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/apimachinery/pkg/runtime/serializer" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/testing" +) + +func NewSimpleDynamicClient(scheme *runtime.Scheme, objects ...runtime.Object) *FakeDynamicClient { + // In order to use List with this client, you have to have the v1.List registered in your scheme. Neat thing though + // it does NOT have to be the *same* list + scheme.AddKnownTypeWithName(schema.GroupVersionKind{Group: "fake-dynamic-client-group", Version: "v1", Kind: "List"}, &unstructured.UnstructuredList{}) + + codecs := serializer.NewCodecFactory(scheme) + o := testing.NewObjectTracker(scheme, codecs.UniversalDecoder()) + for _, obj := range objects { + if err := o.Add(obj); err != nil { + panic(err) + } + } + + cs := &FakeDynamicClient{} + cs.AddReactor("*", "*", testing.ObjectReaction(o)) + cs.AddWatchReactor("*", func(action testing.Action) (handled bool, ret watch.Interface, err error) { + gvr := action.GetResource() + ns := action.GetNamespace() + watch, err := o.Watch(gvr, ns) + if err != nil { + return false, nil, err + } + return true, watch, nil + }) + + return cs +} + +// Clientset implements clientset.Interface. Meant to be embedded into a +// struct to get a default implementation. This makes faking out just the method +// you want to test easier. +type FakeDynamicClient struct { + testing.Fake + scheme *runtime.Scheme +} + +type dynamicResourceClient struct { + client *FakeDynamicClient + namespace string + resource schema.GroupVersionResource +} + +var _ dynamic.Interface = &FakeDynamicClient{} + +func (c *FakeDynamicClient) Resource(resource schema.GroupVersionResource) dynamic.NamespaceableResourceInterface { + return &dynamicResourceClient{client: c, resource: resource} +} + +func (c *dynamicResourceClient) Namespace(ns string) dynamic.ResourceInterface { + ret := *c + ret.namespace = ns + return &ret +} + +func (c *dynamicResourceClient) Create(obj *unstructured.Unstructured, subresources ...string) (*unstructured.Unstructured, error) { + var uncastRet runtime.Object + var err error + switch { + case len(c.namespace) == 0 && len(subresources) == 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewRootCreateAction(c.resource, obj), obj) + + case len(c.namespace) == 0 && len(subresources) > 0: + accessor, err := meta.Accessor(obj) + if err != nil { + return nil, err + } + name := accessor.GetName() + uncastRet, err = c.client.Fake. + Invokes(testing.NewRootCreateSubresourceAction(c.resource, name, strings.Join(subresources, "/"), obj), obj) + + case len(c.namespace) > 0 && len(subresources) == 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewCreateAction(c.resource, c.namespace, obj), obj) + + case len(c.namespace) > 0 && len(subresources) > 0: + accessor, err := meta.Accessor(obj) + if err != nil { + return nil, err + } + name := accessor.GetName() + uncastRet, err = c.client.Fake. + Invokes(testing.NewCreateSubresourceAction(c.resource, name, strings.Join(subresources, "/"), c.namespace, obj), obj) + + } + + if err != nil { + return nil, err + } + if uncastRet == nil { + return nil, err + } + + ret := &unstructured.Unstructured{} + if err := c.client.scheme.Convert(uncastRet, ret, nil); err != nil { + return nil, err + } + return ret, err +} + +func (c *dynamicResourceClient) Update(obj *unstructured.Unstructured, subresources ...string) (*unstructured.Unstructured, error) { + var uncastRet runtime.Object + var err error + switch { + case len(c.namespace) == 0 && len(subresources) == 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewRootUpdateAction(c.resource, obj), obj) + + case len(c.namespace) == 0 && len(subresources) > 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewRootUpdateSubresourceAction(c.resource, strings.Join(subresources, "/"), obj), obj) + + case len(c.namespace) > 0 && len(subresources) == 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewUpdateAction(c.resource, c.namespace, obj), obj) + + case len(c.namespace) > 0 && len(subresources) > 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewUpdateSubresourceAction(c.resource, strings.Join(subresources, "/"), c.namespace, obj), obj) + + } + + if err != nil { + return nil, err + } + if uncastRet == nil { + return nil, err + } + + ret := &unstructured.Unstructured{} + if err := c.client.scheme.Convert(uncastRet, ret, nil); err != nil { + return nil, err + } + return ret, err +} + +func (c *dynamicResourceClient) UpdateStatus(obj *unstructured.Unstructured) (*unstructured.Unstructured, error) { + var uncastRet runtime.Object + var err error + switch { + case len(c.namespace) == 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewRootUpdateSubresourceAction(c.resource, "status", obj), obj) + + case len(c.namespace) > 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewUpdateSubresourceAction(c.resource, "status", c.namespace, obj), obj) + + } + + if err != nil { + return nil, err + } + if uncastRet == nil { + return nil, err + } + + ret := &unstructured.Unstructured{} + if err := c.client.scheme.Convert(uncastRet, ret, nil); err != nil { + return nil, err + } + return ret, err +} + +func (c *dynamicResourceClient) Delete(name string, opts *metav1.DeleteOptions, subresources ...string) error { + var err error + switch { + case len(c.namespace) == 0 && len(subresources) == 0: + _, err = c.client.Fake. + Invokes(testing.NewRootDeleteAction(c.resource, name), &metav1.Status{Status: "dynamic delete fail"}) + + case len(c.namespace) == 0 && len(subresources) > 0: + _, err = c.client.Fake. + Invokes(testing.NewRootDeleteSubresourceAction(c.resource, strings.Join(subresources, "/"), name), &metav1.Status{Status: "dynamic delete fail"}) + + case len(c.namespace) > 0 && len(subresources) == 0: + _, err = c.client.Fake. + Invokes(testing.NewDeleteAction(c.resource, c.namespace, name), &metav1.Status{Status: "dynamic delete fail"}) + + case len(c.namespace) > 0 && len(subresources) > 0: + _, err = c.client.Fake. + Invokes(testing.NewDeleteSubresourceAction(c.resource, strings.Join(subresources, "/"), c.namespace, name), &metav1.Status{Status: "dynamic delete fail"}) + } + + return err +} + +func (c *dynamicResourceClient) DeleteCollection(opts *metav1.DeleteOptions, listOptions metav1.ListOptions) error { + var err error + switch { + case len(c.namespace) == 0: + action := testing.NewRootDeleteCollectionAction(c.resource, listOptions) + _, err = c.client.Fake.Invokes(action, &metav1.Status{Status: "dynamic deletecollection fail"}) + + case len(c.namespace) > 0: + action := testing.NewDeleteCollectionAction(c.resource, c.namespace, listOptions) + _, err = c.client.Fake.Invokes(action, &metav1.Status{Status: "dynamic deletecollection fail"}) + + } + + return err +} + +func (c *dynamicResourceClient) Get(name string, opts metav1.GetOptions, subresources ...string) (*unstructured.Unstructured, error) { + var uncastRet runtime.Object + var err error + switch { + case len(c.namespace) == 0 && len(subresources) == 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewRootGetAction(c.resource, name), &metav1.Status{Status: "dynamic get fail"}) + + case len(c.namespace) == 0 && len(subresources) > 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewRootGetSubresourceAction(c.resource, strings.Join(subresources, "/"), name), &metav1.Status{Status: "dynamic get fail"}) + + case len(c.namespace) > 0 && len(subresources) == 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewGetAction(c.resource, c.namespace, name), &metav1.Status{Status: "dynamic get fail"}) + + case len(c.namespace) > 0 && len(subresources) > 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewGetSubresourceAction(c.resource, c.namespace, strings.Join(subresources, "/"), name), &metav1.Status{Status: "dynamic get fail"}) + } + + if err != nil { + return nil, err + } + if uncastRet == nil { + return nil, err + } + + ret := &unstructured.Unstructured{} + if err := c.client.scheme.Convert(uncastRet, ret, nil); err != nil { + return nil, err + } + return ret, err +} + +func (c *dynamicResourceClient) List(opts metav1.ListOptions) (*unstructured.UnstructuredList, error) { + var obj runtime.Object + var err error + switch { + case len(c.namespace) == 0: + obj, err = c.client.Fake. + Invokes(testing.NewRootListAction(c.resource, schema.GroupVersionKind{Group: "fake-dynamic-client-group", Version: "v1", Kind: "" /*List is appended by the tracker automatically*/}, opts), &metav1.Status{Status: "dynamic list fail"}) + + case len(c.namespace) > 0: + obj, err = c.client.Fake. + Invokes(testing.NewListAction(c.resource, schema.GroupVersionKind{Group: "fake-dynamic-client-group", Version: "v1", Kind: "" /*List is appended by the tracker automatically*/}, c.namespace, opts), &metav1.Status{Status: "dynamic list fail"}) + + } + + if obj == nil { + return nil, err + } + + label, _, _ := testing.ExtractFromListOptions(opts) + if label == nil { + label = labels.Everything() + } + + retUnstructured := &unstructured.Unstructured{} + if err := c.client.scheme.Convert(obj, retUnstructured, nil); err != nil { + return nil, err + } + entireList, err := retUnstructured.ToList() + if err != nil { + return nil, err + } + + list := &unstructured.UnstructuredList{} + for i := range entireList.Items { + item := &entireList.Items[i] + metadata, err := meta.Accessor(item) + if err != nil { + return nil, err + } + if label.Matches(labels.Set(metadata.GetLabels())) { + list.Items = append(list.Items, *item) + } + } + return list, nil +} + +func (c *dynamicResourceClient) Watch(opts metav1.ListOptions) (watch.Interface, error) { + switch { + case len(c.namespace) == 0: + return c.client.Fake. + InvokesWatch(testing.NewRootWatchAction(c.resource, opts)) + + case len(c.namespace) > 0: + return c.client.Fake. + InvokesWatch(testing.NewWatchAction(c.resource, c.namespace, opts)) + + } + + panic("math broke") +} + +func (c *dynamicResourceClient) Patch(name string, pt types.PatchType, data []byte, subresources ...string) (*unstructured.Unstructured, error) { + var uncastRet runtime.Object + var err error + switch { + case len(c.namespace) == 0 && len(subresources) == 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewRootPatchAction(c.resource, name, data), &metav1.Status{Status: "dynamic patch fail"}) + + case len(c.namespace) == 0 && len(subresources) > 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewRootPatchSubresourceAction(c.resource, name, data, subresources...), &metav1.Status{Status: "dynamic patch fail"}) + + case len(c.namespace) > 0 && len(subresources) == 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewPatchAction(c.resource, c.namespace, name, data), &metav1.Status{Status: "dynamic patch fail"}) + + case len(c.namespace) > 0 && len(subresources) > 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewPatchSubresourceAction(c.resource, c.namespace, name, data, subresources...), &metav1.Status{Status: "dynamic patch fail"}) + + } + + if err != nil { + return nil, err + } + if uncastRet == nil { + return nil, err + } + + ret := &unstructured.Unstructured{} + if err := c.client.scheme.Convert(uncastRet, ret, nil); err != nil { + return nil, err + } + return ret, err +} diff --git a/vendor/k8s.io/client-go/rest/request.go b/vendor/k8s.io/client-go/rest/request.go index 09ffd76d..6c67c650 100644 --- a/vendor/k8s.io/client-go/rest/request.go +++ b/vendor/k8s.io/client-go/rest/request.go @@ -455,17 +455,9 @@ func (r *Request) URL() *url.URL { // finalURLTemplate is similar to URL(), but will make all specific parameter values equal // - instead of name or namespace, "{name}" and "{namespace}" will be used, and all query -// parameters will be reset. This creates a copy of the request so as not to change the -// underlying object. This means some useful request info (like the types of field -// selectors in use) will be lost. -// TODO: preserve field selector keys +// parameters will be reset. This creates a copy of the url so as not to change the +// underlying object. func (r Request) finalURLTemplate() url.URL { - if len(r.resourceName) != 0 { - r.resourceName = "{name}" - } - if r.namespaceSet && len(r.namespace) != 0 { - r.namespace = "{namespace}" - } newParams := url.Values{} v := []string{"{value}"} for k := range r.params { @@ -473,6 +465,59 @@ func (r Request) finalURLTemplate() url.URL { } r.params = newParams url := r.URL() + segments := strings.Split(r.URL().Path, "/") + groupIndex := 0 + index := 0 + if r.URL() != nil && r.baseURL != nil && strings.Contains(r.URL().Path, r.baseURL.Path) { + groupIndex += len(strings.Split(r.baseURL.Path, "/")) + } + if groupIndex >= len(segments) { + return *url + } + + const CoreGroupPrefix = "api" + const NamedGroupPrefix = "apis" + isCoreGroup := segments[groupIndex] == CoreGroupPrefix + isNamedGroup := segments[groupIndex] == NamedGroupPrefix + if isCoreGroup { + // checking the case of core group with /api/v1/... format + index = groupIndex + 2 + } else if isNamedGroup { + // checking the case of named group with /apis/apps/v1/... format + index = groupIndex + 3 + } else { + // this should not happen that the only two possibilities are /api... and /apis..., just want to put an + // outlet here in case more API groups are added in future if ever possible: + // https://kubernetes.io/docs/concepts/overview/kubernetes-api/#api-groups + // if a wrong API groups name is encountered, return the {prefix} for url.Path + url.Path = "/{prefix}" + url.RawQuery = "" + return *url + } + //switch segLength := len(segments) - index; segLength { + switch { + // case len(segments) - index == 1: + // resource (with no name) do nothing + case len(segments)-index == 2: + // /$RESOURCE/$NAME: replace $NAME with {name} + segments[index+1] = "{name}" + case len(segments)-index == 3: + if segments[index+2] == "finalize" || segments[index+2] == "status" { + // /$RESOURCE/$NAME/$SUBRESOURCE: replace $NAME with {name} + segments[index+1] = "{name}" + } else { + // /namespace/$NAMESPACE/$RESOURCE: replace $NAMESPACE with {namespace} + segments[index+1] = "{namespace}" + } + case len(segments)-index >= 4: + segments[index+1] = "{namespace}" + // /namespace/$NAMESPACE/$RESOURCE/$NAME: replace $NAMESPACE with {namespace}, $NAME with {name} + if segments[index+3] != "finalize" && segments[index+3] != "status" { + // /$RESOURCE/$NAME/$SUBRESOURCE: replace $NAME with {name} + segments[index+3] = "{name}" + } + } + url.Path = path.Join(segments...) return *url } diff --git a/vendor/k8s.io/client-go/restmapper/discovery.go b/vendor/k8s.io/client-go/restmapper/discovery.go index 58887cd8..aa158626 100644 --- a/vendor/k8s.io/client-go/restmapper/discovery.go +++ b/vendor/k8s.io/client-go/restmapper/discovery.go @@ -99,18 +99,20 @@ func NewDiscoveryRESTMapper(groupResources []*APIGroupResources) meta.RESTMapper scope = meta.RESTScopeRoot } - // this is for legacy resources and servers which don't list singular forms. For those we must still guess. - if len(resource.SingularName) == 0 { - versionMapper.Add(gv.WithKind(resource.Kind), scope) - // TODO this is producing unsafe guesses that don't actually work, but it matches previous behavior - versionMapper.Add(gv.WithKind(resource.Kind+"List"), scope) + // if we have a slash, then this is a subresource and we shouldn't create mappings for those. + if strings.Contains(resource.Name, "/") { continue } plural := gv.WithResource(resource.Name) singular := gv.WithResource(resource.SingularName) - versionMapper.AddSpecific(gv.WithKind(resource.Kind), plural, singular, scope) + // this is for legacy resources and servers which don't list singular forms. For those we must still guess. + if len(resource.SingularName) == 0 { + _, singular = meta.UnsafeGuessKindToResource(gv.WithKind(resource.Kind)) + } + versionMapper.AddSpecific(gv.WithKind(strings.ToLower(resource.Kind)), plural, singular, scope) + versionMapper.AddSpecific(gv.WithKind(resource.Kind), plural, singular, scope) // TODO this is producing unsafe guesses that don't actually work, but it matches previous behavior versionMapper.Add(gv.WithKind(resource.Kind+"List"), scope) } diff --git a/vendor/k8s.io/client-go/transport/round_trippers.go b/vendor/k8s.io/client-go/transport/round_trippers.go index 459a9376..316a5c0d 100644 --- a/vendor/k8s.io/client-go/transport/round_trippers.go +++ b/vendor/k8s.io/client-go/transport/round_trippers.go @@ -129,7 +129,7 @@ func SetAuthProxyHeaders(req *http.Request, username string, groups []string, ex } for key, values := range extra { for _, value := range values { - req.Header.Add("X-Remote-Extra-"+key, value) + req.Header.Add("X-Remote-Extra-"+headerKeyEscape(key), value) } } } @@ -246,7 +246,7 @@ func (rt *impersonatingRoundTripper) RoundTrip(req *http.Request) (*http.Respons } for k, vv := range rt.impersonate.Extra { for _, v := range vv { - req.Header.Add(ImpersonateUserExtraHeaderPrefix+k, v) + req.Header.Add(ImpersonateUserExtraHeaderPrefix+headerKeyEscape(k), v) } } @@ -422,3 +422,110 @@ func (rt *debuggingRoundTripper) RoundTrip(req *http.Request) (*http.Response, e func (rt *debuggingRoundTripper) WrappedRoundTripper() http.RoundTripper { return rt.delegatedRoundTripper } + +func legalHeaderByte(b byte) bool { + return int(b) < len(legalHeaderKeyBytes) && legalHeaderKeyBytes[b] +} + +func shouldEscape(b byte) bool { + // url.PathUnescape() returns an error if any '%' is not followed by two + // hexadecimal digits, so we'll intentionally encode it. + return !legalHeaderByte(b) || b == '%' +} + +func headerKeyEscape(key string) string { + buf := strings.Builder{} + for i := 0; i < len(key); i++ { + b := key[i] + if shouldEscape(b) { + // %-encode bytes that should be escaped: + // https://tools.ietf.org/html/rfc3986#section-2.1 + fmt.Fprintf(&buf, "%%%02X", b) + continue + } + buf.WriteByte(b) + } + return buf.String() +} + +// legalHeaderKeyBytes was copied from net/http/lex.go's isTokenTable. +// See https://httpwg.github.io/specs/rfc7230.html#rule.token.separators +var legalHeaderKeyBytes = [127]bool{ + '%': true, + '!': true, + '#': true, + '$': true, + '&': true, + '\'': true, + '*': true, + '+': true, + '-': true, + '.': true, + '0': true, + '1': true, + '2': true, + '3': true, + '4': true, + '5': true, + '6': true, + '7': true, + '8': true, + '9': true, + 'A': true, + 'B': true, + 'C': true, + 'D': true, + 'E': true, + 'F': true, + 'G': true, + 'H': true, + 'I': true, + 'J': true, + 'K': true, + 'L': true, + 'M': true, + 'N': true, + 'O': true, + 'P': true, + 'Q': true, + 'R': true, + 'S': true, + 'T': true, + 'U': true, + 'W': true, + 'V': true, + 'X': true, + 'Y': true, + 'Z': true, + '^': true, + '_': true, + '`': true, + 'a': true, + 'b': true, + 'c': true, + 'd': true, + 'e': true, + 'f': true, + 'g': true, + 'h': true, + 'i': true, + 'j': true, + 'k': true, + 'l': true, + 'm': true, + 'n': true, + 'o': true, + 'p': true, + 'q': true, + 'r': true, + 's': true, + 't': true, + 'u': true, + 'v': true, + 'w': true, + 'x': true, + 'y': true, + 'z': true, + '|': true, + '~': true, +} From dd2bae69c9d8ab3034efc0fcd494c157b41f6f30 Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Thu, 1 Nov 2018 23:07:40 -0700 Subject: [PATCH 17/17] update readme --- README.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/README.md b/README.md index 5bcd6876..06b9d933 100755 --- a/README.md +++ b/README.md @@ -95,9 +95,7 @@ Common external metrics to use for autoscaling are: ## Custom Metrics -Custom metrics are currently retrieved from Application Insights. Currently you will need to use an [AppId and API key](https://dev.applicationinsights.io/documentation/Authorization/API-key-and-App-ID) to authenticate and retrieve metrics from Application Insights. View a list of basic metrics that come out of the box and see sample values at the [AI api explorer](https://dev.applicationinsights.io/apiexplorer/metrics). - -> note: Metrics that have multi parts currently need will be passed as with a separator of `-` in place of AI separator of `/`. An example is `performanceCounters/requestsPerSecond` should be specified as `performanceCounters-requestsPerSecond` +Custom metrics are currently retrieved from Application Insights. View a list of basic metrics that come out of the box and see sample values at the [AI api explorer](https://dev.applicationinsights.io/apiexplorer/metrics). Common Custom Metrics are: