diff --git a/pkg/koordlet/reporter/reporter.go b/pkg/koordlet/reporter/reporter.go index f24044be2..0a6df0837 100644 --- a/pkg/koordlet/reporter/reporter.go +++ b/pkg/koordlet/reporter/reporter.go @@ -199,7 +199,7 @@ func (r *reporter) sync() { nodeMetric, err := r.nodeMetricLister.Get(r.nodeName) if errors.IsNotFound(err) { klog.Warningf("nodeMetric %v not found, skip", r.nodeName) - return err + return nil } else if err != nil { klog.Warningf("failed to get %s nodeMetric: %v", r.nodeName, err) return err diff --git a/pkg/koordlet/reporter/reporter_test.go b/pkg/koordlet/reporter/reporter_test.go index 68d05143c..118ffe74d 100644 --- a/pkg/koordlet/reporter/reporter_test.go +++ b/pkg/koordlet/reporter/reporter_test.go @@ -17,20 +17,34 @@ limitations under the License. package reporter import ( + "context" "testing" "time" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/utils/pointer" + "github.com/golang/mock/gomock" slov1alpha1 "github.com/koordinator-sh/koordinator/apis/slo/v1alpha1" + clientbeta1 "github.com/koordinator-sh/koordinator/pkg/client/clientset/versioned/typed/slo/v1alpha1" + fakeclientslov1alpha1 "github.com/koordinator-sh/koordinator/pkg/client/clientset/versioned/typed/slo/v1alpha1/fake" listerbeta1 "github.com/koordinator-sh/koordinator/pkg/client/listers/slo/v1alpha1" + "github.com/koordinator-sh/koordinator/pkg/koordlet/metriccache" + mock_metriccache "github.com/koordinator-sh/koordinator/pkg/koordlet/metriccache/mockmetriccache" + "github.com/koordinator-sh/koordinator/pkg/koordlet/statesinformer" + mock_statesinformer "github.com/koordinator-sh/koordinator/pkg/koordlet/statesinformer/mockstatesinformer" + "github.com/stretchr/testify/assert" ) var _ listerbeta1.NodeMetricLister = &fakeNodeMetricLister{} type fakeNodeMetricLister struct { nodeMetrics *slov1alpha1.NodeMetric + getErr error } func (f *fakeNodeMetricLister) List(selector labels.Selector) (ret []*slov1alpha1.NodeMetric, err error) { @@ -38,7 +52,7 @@ func (f *fakeNodeMetricLister) List(selector labels.Selector) (ret []*slov1alpha } func (f *fakeNodeMetricLister) Get(name string) (*slov1alpha1.NodeMetric, error) { - return f.nodeMetrics, nil + return f.nodeMetrics, f.getErr } func Test_reporter_isNodeMetricInited(t *testing.T) { @@ -90,3 +104,228 @@ func Test_getNodeMetricReportInterval(t *testing.T) { t.Errorf("expect reportInterval %d but got %d", expectReportInterval, reportInterval) } } + +type fakeNodeMetricClient struct { + fakeclientslov1alpha1.FakeNodeMetrics + nodeMetrics map[string]*slov1alpha1.NodeMetric +} + +func (c *fakeNodeMetricClient) Get(ctx context.Context, name string, opts metav1.GetOptions) (*slov1alpha1.NodeMetric, error) { + nodeMetric, ok := c.nodeMetrics[name] + if !ok { + return &slov1alpha1.NodeMetric{}, errors.NewNotFound(schema.GroupResource{Group: "slo.koordinator.sh", Resource: "nodemetrics"}, name) + } + return nodeMetric, nil +} + +func (c *fakeNodeMetricClient) UpdateStatus(ctx context.Context, nodeMetric *slov1alpha1.NodeMetric, opts metav1.UpdateOptions) (*slov1alpha1.NodeMetric, error) { + currentNodeMetric, ok := c.nodeMetrics[nodeMetric.Name] + if !ok { + return &slov1alpha1.NodeMetric{}, errors.NewNotFound(schema.GroupResource{Group: "slo.koordinator.sh", Resource: "nodemetrics"}, nodeMetric.Name) + } + currentNodeMetric.Status = nodeMetric.Status + c.nodeMetrics[nodeMetric.Name] = currentNodeMetric + return currentNodeMetric, nil +} + +func Test_reporter_sync(t *testing.T) { + type fields struct { + nodeName string + nodeMetric *slov1alpha1.NodeMetric + metricCache func(ctrl *gomock.Controller) metriccache.MetricCache + statesInformer func(ctrl *gomock.Controller) statesinformer.StatesInformer + nodeMetricLister listerbeta1.NodeMetricLister + nodeMetricClient clientbeta1.NodeMetricInterface + } + tests := []struct { + name string + fields fields + want *slov1alpha1.NodeMetric + wantErr bool + }{ + { + name: "nodeMetric not initialized", + fields: fields{ + nodeName: "test", + nodeMetric: nil, + metricCache: func(ctrl *gomock.Controller) metriccache.MetricCache { + return nil + }, + statesInformer: func(ctrl *gomock.Controller) statesinformer.StatesInformer { + return nil + }, + nodeMetricLister: nil, + nodeMetricClient: &fakeNodeMetricClient{}, + }, + want: &slov1alpha1.NodeMetric{}, + wantErr: true, + }, + { + name: "collect nil nodeMetric", + fields: fields{ + nodeName: "test", + nodeMetric: &slov1alpha1.NodeMetric{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + }, + metricCache: func(ctrl *gomock.Controller) metriccache.MetricCache { + c := mock_metriccache.NewMockMetricCache(ctrl) + c.EXPECT().GetNodeResourceMetric(gomock.Any()).Return(metriccache.NodeResourceQueryResult{}).Times(1) + return c + }, + statesInformer: func(ctrl *gomock.Controller) statesinformer.StatesInformer { + i := mock_statesinformer.NewMockStatesInformer(ctrl) + i.EXPECT().GetAllPods().Return(nil).Times(1) + return i + }, + nodeMetricLister: nil, + nodeMetricClient: &fakeNodeMetricClient{}, + }, + want: &slov1alpha1.NodeMetric{}, + wantErr: true, + }, + { + name: "successfully report nodeMetric", + fields: fields{ + nodeName: "test", + nodeMetric: &slov1alpha1.NodeMetric{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + }, + metricCache: func(ctrl *gomock.Controller) metriccache.MetricCache { + c := mock_metriccache.NewMockMetricCache(ctrl) + c.EXPECT().GetNodeResourceMetric(gomock.Any()).Return(metriccache.NodeResourceQueryResult{ + Metric: &metriccache.NodeResourceMetric{ + CPUUsed: metriccache.CPUMetric{ + CPUUsed: resource.MustParse("1000"), + }, + MemoryUsed: metriccache.MemoryMetric{ + MemoryWithoutCache: resource.MustParse("1Gi"), + }, + }, + }).Times(1) + return c + }, + statesInformer: func(ctrl *gomock.Controller) statesinformer.StatesInformer { + i := mock_statesinformer.NewMockStatesInformer(ctrl) + i.EXPECT().GetAllPods().Return(nil).Times(1) + return i + }, + nodeMetricLister: &fakeNodeMetricLister{ + nodeMetrics: &slov1alpha1.NodeMetric{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + }, + }, + nodeMetricClient: &fakeNodeMetricClient{ + nodeMetrics: map[string]*slov1alpha1.NodeMetric{ + "test": { + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + }, + }, + }, + }, + want: &slov1alpha1.NodeMetric{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Status: slov1alpha1.NodeMetricStatus{ + NodeMetric: &slov1alpha1.NodeMetricInfo{ + NodeUsage: *convertNodeMetricToResourceMap(&metriccache.NodeResourceMetric{ + CPUUsed: metriccache.CPUMetric{ + CPUUsed: resource.MustParse("1000"), + }, + MemoryUsed: metriccache.MemoryMetric{ + MemoryWithoutCache: resource.MustParse("1Gi"), + }, + }), + }, + PodsMetric: []*slov1alpha1.PodMetricInfo{}, + }, + }, + wantErr: false, + }, + { + name: "skip for nodeMetric not found", + fields: fields{ + nodeName: "test", + nodeMetric: &slov1alpha1.NodeMetric{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + }, + metricCache: func(ctrl *gomock.Controller) metriccache.MetricCache { + c := mock_metriccache.NewMockMetricCache(ctrl) + c.EXPECT().GetNodeResourceMetric(gomock.Any()).Return(metriccache.NodeResourceQueryResult{ + Metric: &metriccache.NodeResourceMetric{ + CPUUsed: metriccache.CPUMetric{ + CPUUsed: resource.MustParse("1000"), + }, + MemoryUsed: metriccache.MemoryMetric{ + MemoryWithoutCache: resource.MustParse("1Gi"), + }, + }, + }).Times(1) + return c + }, + statesInformer: func(ctrl *gomock.Controller) statesinformer.StatesInformer { + i := mock_statesinformer.NewMockStatesInformer(ctrl) + i.EXPECT().GetAllPods().Return(nil).Times(1) + return i + }, + nodeMetricLister: &fakeNodeMetricLister{ + nodeMetrics: &slov1alpha1.NodeMetric{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + }, + getErr: errors.NewNotFound(schema.GroupResource{Group: "slo.koordinator.sh", Resource: "nodemetrics"}, "test"), + }, + nodeMetricClient: &fakeNodeMetricClient{ + nodeMetrics: map[string]*slov1alpha1.NodeMetric{ + "test": { + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + }, + }, + }, + }, + want: &slov1alpha1.NodeMetric{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + r := &reporter{ + nodeMetric: tt.fields.nodeMetric, + metricCache: tt.fields.metricCache(ctrl), + statesInformer: tt.fields.statesInformer(ctrl), + nodeMetricLister: tt.fields.nodeMetricLister, + statusUpdater: newStatusUpdater(tt.fields.nodeMetricClient), + } + r.sync() + + nodeMetric, err := r.statusUpdater.nodeMetricClient.Get(context.TODO(), tt.fields.nodeName, metav1.GetOptions{}) + assert.Equal(t, tt.wantErr, err != nil) + if tt.wantErr { + assert.Equal(t, tt.want, nodeMetric) + } else { + assert.NotNil(t, nodeMetric) + assert.Equal(t, tt.want.Status.NodeMetric, nodeMetric.Status.NodeMetric) + assert.Equal(t, tt.want.Status.PodsMetric, nodeMetric.Status.PodsMetric) + } + }) + } +} diff --git a/pkg/slo-controller/noderesource/noderesource_controller.go b/pkg/slo-controller/noderesource/noderesource_controller.go index 672ae1a43..cbe1ac7bc 100644 --- a/pkg/slo-controller/noderesource/noderesource_controller.go +++ b/pkg/slo-controller/noderesource/noderesource_controller.go @@ -55,8 +55,9 @@ func (r *NodeResourceReconciler) Reconcile(ctx context.Context, req ctrl.Request node := &corev1.Node{} if err := r.Client.Get(context.TODO(), req.NamespacedName, node); err != nil { if errors.IsNotFound(err) { - klog.V(3).Infof("node %v not found: %v", req.Name) - return ctrl.Result{Requeue: false}, err + // skip non-existing node and return no error to forget the request + klog.V(3).Infof("skip for node %v not found", req.Name) + return ctrl.Result{Requeue: false}, nil } else { klog.Errorf("failed to get node %v, error: %v", req.Name, err) return ctrl.Result{Requeue: true}, err @@ -75,8 +76,9 @@ func (r *NodeResourceReconciler) Reconcile(ctx context.Context, req ctrl.Request nodeMetric := &slov1alpha1.NodeMetric{} if err := r.Client.Get(context.TODO(), req.NamespacedName, nodeMetric); err != nil { if errors.IsNotFound(err) { - klog.V(3).Infof("nodemetric %v not found: %v", req.Name) - return ctrl.Result{Requeue: false}, err + // skip non-existing node metric and return no error to forget the request + klog.V(3).Infof("skip for nodemetric %v not found", req.Name) + return ctrl.Result{Requeue: false}, nil } else { klog.Errorf("failed to get nodemetric %v, error: %v", req.Name, err) return ctrl.Result{Requeue: true}, err diff --git a/pkg/slo-controller/noderesource/noderesource_controller_test.go b/pkg/slo-controller/noderesource/noderesource_controller_test.go index d781163e4..4b0182e09 100644 --- a/pkg/slo-controller/noderesource/noderesource_controller_test.go +++ b/pkg/slo-controller/noderesource/noderesource_controller_test.go @@ -22,7 +22,6 @@ import ( "time" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -78,12 +77,8 @@ func Test_NodeResourceController_NodeNotFound(t *testing.T) { nodeReq := ctrl.Request{NamespacedName: key} result, err := r.Reconcile(ctx, nodeReq) - if !errors.IsNotFound(err) { - t.Fatal(err) - } - if result.Requeue != false { - t.Errorf("failed to reconcile") - } + assert.NoError(t, err) + assert.Equal(t, false, result.Requeue) } func Test_NodeResourceController_NodeMetricNotExist(t *testing.T) { @@ -122,12 +117,8 @@ func Test_NodeResourceController_NodeMetricNotExist(t *testing.T) { nodeReq := ctrl.Request{NamespacedName: key} result, err := r.Reconcile(ctx, nodeReq) - if !errors.IsNotFound(err) { - t.Fatal(err) - } - if result.Requeue != false { - t.Errorf("failed to reconcile") - } + assert.NoError(t, err) + assert.Equal(t, false, result.Requeue) } func Test_NodeResourceController_ColocationEnabled(t *testing.T) {