From 1acecf875498e9b39a666049d3f8a00a9b220a72 Mon Sep 17 00:00:00 2001 From: lyndon-li <98304688+Lyndon-Li@users.noreply.github.com> Date: Mon, 28 Sep 2026 11:18:50 +0800 Subject: [PATCH] enable VGDP soothing by default and set queue length as 5 (#10578) Signed-off-by: Lyndon-Li --- changelogs/unreleased/10578-Lyndon-Li | 1 + pkg/cmd/cli/nodeagent/server.go | 26 +++-- pkg/cmd/cli/nodeagent/server_test.go | 139 ++++++++++++++++++++++++++ 3 files changed, 158 insertions(+), 8 deletions(-) create mode 100644 changelogs/unreleased/10578-Lyndon-Li diff --git a/changelogs/unreleased/10578-Lyndon-Li b/changelogs/unreleased/10578-Lyndon-Li new file mode 100644 index 000000000..f48150603 --- /dev/null +++ b/changelogs/unreleased/10578-Lyndon-Li @@ -0,0 +1 @@ +Fix issue #10575, enable VGDP soothing by default and set queue length as 5 \ No newline at end of file diff --git a/pkg/cmd/cli/nodeagent/server.go b/pkg/cmd/cli/nodeagent/server.go index 973e993dd..f4e2939b0 100644 --- a/pkg/cmd/cli/nodeagent/server.go +++ b/pkg/cmd/cli/nodeagent/server.go @@ -78,6 +78,7 @@ const ( defaultResourceTimeout = 10 * time.Minute defaultDataMoverPrepareTimeout = 30 * time.Minute defaultDataPathConcurrentNum = 1 + defaultPrepareQueueLength = 5 ) type nodeAgentServerConfig struct { @@ -353,14 +354,7 @@ func (s *nodeAgentServer) run() { } } - if s.dataPathConfigs != nil && s.dataPathConfigs.LoadConcurrency != nil && s.dataPathConfigs.LoadConcurrency.PrepareQueueLength > 0 { - if counter, err := exposer.StartVgdpCounter(s.ctx, s.mgr, s.dataPathConfigs.LoadConcurrency.PrepareQueueLength); err != nil { - s.logger.WithError(err).Warnf("Failed to start VGDP counter, VDGP loads are not constrained") - } else { - s.vgdpCounter = counter - s.logger.Infof("VGDP loads are constrained with %d", s.dataPathConfigs.LoadConcurrency.PrepareQueueLength) - } - } + s.initVgdpCounter() var cachePVCConfig *velerotypes.CachePVC if s.dataPathConfigs != nil && s.dataPathConfigs.CachePVCConfig != nil { @@ -728,3 +722,19 @@ func (s *nodeAgentServer) validateCachePVCConfig(config velerotypes.CachePVC) er return nil } + +var startVgdpCounterFunc = exposer.StartVgdpCounter + +func (s *nodeAgentServer) initVgdpCounter() { + prepareQueueLength := defaultPrepareQueueLength + if s.dataPathConfigs != nil && s.dataPathConfigs.LoadConcurrency != nil && s.dataPathConfigs.LoadConcurrency.PrepareQueueLength > 0 { + prepareQueueLength = s.dataPathConfigs.LoadConcurrency.PrepareQueueLength + } + + if counter, err := startVgdpCounterFunc(s.ctx, s.mgr, prepareQueueLength); err != nil { + s.logger.WithError(err).Warnf("Failed to start VGDP counter with length %d, VDGP loads are not constrained", prepareQueueLength) + } else { + s.vgdpCounter = counter + s.logger.Infof("VGDP loads are constrained with %d", prepareQueueLength) + } +} diff --git a/pkg/cmd/cli/nodeagent/server_test.go b/pkg/cmd/cli/nodeagent/server_test.go index 152016ab6..4edf66872 100644 --- a/pkg/cmd/cli/nodeagent/server_test.go +++ b/pkg/cmd/cli/nodeagent/server_test.go @@ -30,8 +30,10 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" + "sigs.k8s.io/controller-runtime/pkg/manager" "github.com/vmware-tanzu/velero/pkg/builder" + "github.com/vmware-tanzu/velero/pkg/exposer" "github.com/vmware-tanzu/velero/pkg/nodeagent" testutil "github.com/vmware-tanzu/velero/pkg/test" velerotypes "github.com/vmware-tanzu/velero/pkg/types" @@ -530,3 +532,140 @@ func TestValidateCachePVCConfig(t *testing.T) { }) } } + +func Test_initVgdpCounter(t *testing.T) { + origStartVgdpCounterFunc := startVgdpCounterFunc + defer func() { + startVgdpCounterFunc = origStartVgdpCounterFunc + }() + + mockCounter := &exposer.VgdpCounter{} + + tests := []struct { + name string + dataPathConfigs *velerotypes.NodeAgentConfigs + mockFunc func(ctx context.Context, mgr manager.Manager, queueLength int) (*exposer.VgdpCounter, error) + expectQueueLength int + expectVgdpCounterSet bool + expectLog string + }{ + { + name: "default configs, startVgdpCounter succeeds", + dataPathConfigs: nil, + mockFunc: func(ctx context.Context, mgr manager.Manager, queueLength int) (*exposer.VgdpCounter, error) { + return mockCounter, nil + }, + expectQueueLength: defaultPrepareQueueLength, + expectVgdpCounterSet: true, + expectLog: fmt.Sprintf("VGDP loads are constrained with %d", defaultPrepareQueueLength), + }, + { + name: "custom queue length, startVgdpCounter succeeds", + dataPathConfigs: &velerotypes.NodeAgentConfigs{ + LoadConcurrency: &velerotypes.LoadConcurrency{ + PrepareQueueLength: 10, + }, + }, + mockFunc: func(ctx context.Context, mgr manager.Manager, queueLength int) (*exposer.VgdpCounter, error) { + return mockCounter, nil + }, + expectQueueLength: 10, + expectVgdpCounterSet: true, + expectLog: "VGDP loads are constrained with 10", + }, + { + name: "LoadConcurrency is nil, uses default queue length", + dataPathConfigs: &velerotypes.NodeAgentConfigs{ + LoadConcurrency: nil, + }, + mockFunc: func(ctx context.Context, mgr manager.Manager, queueLength int) (*exposer.VgdpCounter, error) { + return mockCounter, nil + }, + expectQueueLength: defaultPrepareQueueLength, + expectVgdpCounterSet: true, + expectLog: fmt.Sprintf("VGDP loads are constrained with %d", defaultPrepareQueueLength), + }, + { + name: "PrepareQueueLength is 0, uses default queue length", + dataPathConfigs: &velerotypes.NodeAgentConfigs{ + LoadConcurrency: &velerotypes.LoadConcurrency{ + PrepareQueueLength: 0, + }, + }, + mockFunc: func(ctx context.Context, mgr manager.Manager, queueLength int) (*exposer.VgdpCounter, error) { + return mockCounter, nil + }, + expectQueueLength: defaultPrepareQueueLength, + expectVgdpCounterSet: true, + expectLog: fmt.Sprintf("VGDP loads are constrained with %d", defaultPrepareQueueLength), + }, + { + name: "PrepareQueueLength is negative, uses default queue length", + dataPathConfigs: &velerotypes.NodeAgentConfigs{ + LoadConcurrency: &velerotypes.LoadConcurrency{ + PrepareQueueLength: -1, + }, + }, + mockFunc: func(ctx context.Context, mgr manager.Manager, queueLength int) (*exposer.VgdpCounter, error) { + return mockCounter, nil + }, + expectQueueLength: defaultPrepareQueueLength, + expectVgdpCounterSet: true, + expectLog: fmt.Sprintf("VGDP loads are constrained with %d", defaultPrepareQueueLength), + }, + { + name: "startVgdpCounter fails with default queue length", + dataPathConfigs: nil, + mockFunc: func(ctx context.Context, mgr manager.Manager, queueLength int) (*exposer.VgdpCounter, error) { + return nil, errors.New("fake-start-error") + }, + expectQueueLength: defaultPrepareQueueLength, + expectVgdpCounterSet: false, + expectLog: fmt.Sprintf("Failed to start VGDP counter with length %d, VDGP loads are not constrained", defaultPrepareQueueLength), + }, + { + name: "startVgdpCounter fails with custom queue length", + dataPathConfigs: &velerotypes.NodeAgentConfigs{ + LoadConcurrency: &velerotypes.LoadConcurrency{ + PrepareQueueLength: 8, + }, + }, + mockFunc: func(ctx context.Context, mgr manager.Manager, queueLength int) (*exposer.VgdpCounter, error) { + return nil, errors.New("fake-start-error") + }, + expectQueueLength: 8, + expectVgdpCounterSet: false, + expectLog: "Failed to start VGDP counter with length 8, VDGP loads are not constrained", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var recordedQueueLength int + startVgdpCounterFunc = func(ctx context.Context, mgr manager.Manager, queueLength int) (*exposer.VgdpCounter, error) { + recordedQueueLength = queueLength + return test.mockFunc(ctx, mgr, queueLength) + } + + logBuffer := "" + s := &nodeAgentServer{ + ctx: t.Context(), + dataPathConfigs: test.dataPathConfigs, + logger: testutil.NewSingleLogger(&logBuffer), + } + + s.initVgdpCounter() + + assert.Equal(t, test.expectQueueLength, recordedQueueLength) + if test.expectVgdpCounterSet { + assert.Equal(t, mockCounter, s.vgdpCounter) + } else { + assert.Nil(t, s.vgdpCounter) + } + + if test.expectLog != "" { + assert.Contains(t, logBuffer, test.expectLog) + } + }) + } +}