diff --git a/design/node-agent-concurrency.md b/design/node-agent-concurrency.md index c3d84e69e3..71dd33e211 100644 --- a/design/node-agent-concurrency.md +++ b/design/node-agent-concurrency.md @@ -28,10 +28,15 @@ Therefore, in order to gain the optimized performance with the limited resources We introduce a configMap named ```node-agent-configs``` for users to specify the node-agent related configurations. This configMap is not created by Velero, users should create it manually on demand. The configMap should be in the same namespace where Velero is installed. If multiple Velero instances are installed in different namespaces, there should be one configMap in each namespace which applies to node-agent in that namespace only. Node-agent server checks these configurations at startup time and use it to initiate the related VGDP modules. Therefore, users could edit this configMap any time, but in order to make the changes effective, node-agent server needs to be restarted. -The ```node-agent-configs``` configMap may be used for other purpose of configuring node-agent in future, at present, there is only one kind of configuration as the data in the configMap, the name is ```data-path-concurrency```. +The ```node-agent-configs``` configMap may be used for other purpose of configuring node-agent in future, at present, there is only one kind of configuration as the data in the configMap, the name is ```dataPathConcurrency```. -The data structure for ```data-path-concurrency``` is as below: +The data structure for ```node-agent-configs``` is as below: ```go +type Configs struct { + // DataPathConcurrency is the config for data path concurrency per node. + DataPathConcurrency *DataPathConcurrency `json:"dataPathConcurrency,omitempty"` +} + type DataPathConcurrency struct { // GlobalConfig specifies the concurrency number to all nodes for which per-node config is not specified GlobalConfig int `json:"globalConfig,omitempty"` @@ -50,7 +55,7 @@ type RuledConfigs struct { ``` ### Global concurrent number -We allow users to specify a concurrent number that will be applied to all nodes if the per-node number is not specified. This number is set through ```globalConfig``` field in ```data-path-concurrency```. +We allow users to specify a concurrent number that will be applied to all nodes if the per-node number is not specified. This number is set through ```globalConfig``` field in ```dataPathConcurrency```. The number starts from 1 which means there is no concurrency, only one instance of VGDP is allowed. There is no roof limit. If this number is not specified or not valid, a hard-coded default value will be used, the value is set to 1. @@ -62,7 +67,7 @@ We allow users to specify different concurrent number per node, for example, use The range of Per-node concurrent number is the same with Global concurrent number. Per-node concurrent number is preferable to Global concurrent number, so it will overwrite the Global concurrent number for that node. -Per-node concurrent number is implemented through ```perNodeConfig``` field in ```data-path-concurrency```. +Per-node concurrent number is implemented through ```perNodeConfig``` field in ```dataPathConcurrency```. ```perNodeConfig``` is a list of ```RuledConfigs``` each item of which matches one or more nodes by label selectors and specify the concurrent number for the matched nodes. This means, the nodes are identified by labels. @@ -80,30 +85,32 @@ If one node falls into more than one rules, e.g., if node1 also has the label `` A sample of the ```node-agent-configs``` configMap is as below: ```json { - "globalConfig": 2, - "perNodeConfig": [ - { - "nodeSelector": { - "matchLabels": { - "kubernetes.io/hostname": "node1" - } - }, - "number": 3 - }, - { - "nodeSelector": { - "matchLabels": { - "beta.kubernetes.io/instance-type": "Standard_B4ms" - } + "dataPathConcurrency": { + "globalConfig": 2, + "perNodeConfig": [ + { + "nodeSelector": { + "matchLabels": { + "kubernetes.io/hostname": "node1" + } + }, + "number": 3 }, - "number": 5 - } - ] + { + "nodeSelector": { + "matchLabels": { + "beta.kubernetes.io/instance-type": "Standard_B4ms" + } + }, + "number": 5 + } + ] + } } ``` To create the configMap, users need to save something like the above sample to a json file and then run below command: ``` -kubectl create cm node-agent-configs -n velero --from-file=data-path-concurrency= +kubectl create cm node-agent-configs -n velero --from-file= ``` ### Global data path manager diff --git a/pkg/cmd/cli/nodeagent/server.go b/pkg/cmd/cli/nodeagent/server.go index 1339920063..835b899c36 100644 --- a/pkg/cmd/cli/nodeagent/server.go +++ b/pkg/cmd/cli/nodeagent/server.go @@ -499,7 +499,7 @@ func (s *nodeAgentServer) getDataPathConcurrentNum(defaultNum int) int { } if configs == nil || configs.DataPathConcurrency == nil { - s.logger.Infof("Node agent configs are not found, use the default number %v", defaultNum) + s.logger.Infof("Concurrency configs are not found, use the default number %v", defaultNum) return defaultNum } diff --git a/pkg/cmd/cli/nodeagent/server_test.go b/pkg/cmd/cli/nodeagent/server_test.go index f3115923b9..4472dfce12 100644 --- a/pkg/cmd/cli/nodeagent/server_test.go +++ b/pkg/cmd/cli/nodeagent/server_test.go @@ -161,7 +161,7 @@ func Test_getDataPathConcurrentNum(t *testing.T) { getFunc: func(context.Context, string, kubernetes.Interface) (*nodeagent.Configs, error) { return nil, nil }, - expectLog: fmt.Sprintf("Node agent configs are not found, use the default number %v", defaultNum), + expectLog: fmt.Sprintf("Concurrency configs are not found, use the default number %v", defaultNum), expectNum: defaultNum, }, { @@ -169,7 +169,7 @@ func Test_getDataPathConcurrentNum(t *testing.T) { getFunc: func(context.Context, string, kubernetes.Interface) (*nodeagent.Configs, error) { return &nodeagent.Configs{}, nil }, - expectLog: fmt.Sprintf("Node agent configs are not found, use the default number %v", defaultNum), + expectLog: fmt.Sprintf("Concurrency configs are not found, use the default number %v", defaultNum), expectNum: defaultNum, }, { diff --git a/pkg/nodeagent/node_agent.go b/pkg/nodeagent/node_agent.go index 51357766bd..ff93ed5967 100644 --- a/pkg/nodeagent/node_agent.go +++ b/pkg/nodeagent/node_agent.go @@ -61,7 +61,7 @@ type RuledConfigs struct { type Configs struct { // DataPathConcurrency is the config for data path concurrency per node. - DataPathConcurrency *DataPathConcurrency + DataPathConcurrency *DataPathConcurrency `json:"dataPathConcurrency,omitempty"` } // IsRunning checks if the node agent daemonset is running properly. If not, return the error found @@ -128,16 +128,16 @@ func GetConfigs(ctx context.Context, namespace string, kubeClient kubernetes.Int return nil, errors.Errorf("data is not available in config map %s", configName) } - jsonString, exist := cm.Data[dataPathConConfigName] - if !exist { - return nil, nil + jsonString := "" + for _, v := range cm.Data { + jsonString = v } - concurrencyConfigs := &DataPathConcurrency{} - err = json.Unmarshal([]byte(jsonString), concurrencyConfigs) + configs := &Configs{} + err = json.Unmarshal([]byte(jsonString), configs) if err != nil { - return nil, errors.Wrapf(err, "error to unmarshall data path concurrency configs from %s", configName) + return nil, errors.Wrapf(err, "error to unmarshall configs from %s", configName) } - return &Configs{DataPathConcurrency: concurrencyConfigs}, nil + return configs, nil } diff --git a/pkg/nodeagent/node_agent_test.go b/pkg/nodeagent/node_agent_test.go index 9ef8c00b15..a18e45b140 100644 --- a/pkg/nodeagent/node_agent_test.go +++ b/pkg/nodeagent/node_agent_test.go @@ -242,16 +242,16 @@ func TestGetPodSpec(t *testing.T) { func TestGetConfigs(t *testing.T) { cm := builder.ForConfigMap("fake-ns", "node-agent-configs").Result() - cmWithInvalidData := builder.ForConfigMap("fake-ns", "node-agent-configs").Data("fake-key", "fake-value").Result() - cmWithInvalidDataFormat := builder.ForConfigMap("fake-ns", "node-agent-configs").Data("data-path-concurrency", "wrong").Result() - cmWithValidData := builder.ForConfigMap("fake-ns", "node-agent-configs").Data("data-path-concurrency", "{\"globalConfig\": 5}").Result() + cmWithInvalidDataFormat := builder.ForConfigMap("fake-ns", "node-agent-configs").Data("fake-key", "wrong").Result() + cmWithoutCocurrentData := builder.ForConfigMap("fake-ns", "node-agent-configs").Data("fake-key", "{\"someothers\":{\"someother\": 10}}").Result() + cmWithValidData := builder.ForConfigMap("fake-ns", "node-agent-configs").Data("fake-key", "{\"dataPathConcurrency\":{\"globalConfig\": 5}}").Result() tests := []struct { name string kubeClientObj []runtime.Object namespace string kubeReactors []reactor - expectResult *DataPathConcurrency + expectResult *Configs expectErr string }{ { @@ -281,19 +281,20 @@ func TestGetConfigs(t *testing.T) { expectErr: "data is not available in config map node-agent-configs", }, { - name: "cm's data is not found", + name: "cm's data is with invalid format", namespace: "fake-ns", kubeClientObj: []runtime.Object{ - cmWithInvalidData, + cmWithInvalidDataFormat, }, + expectErr: "error to unmarshall configs from node-agent-configs: invalid character 'w' looking for beginning of value", }, { - name: "cm's data is with invalid format", + name: "concurrency configs are not found", namespace: "fake-ns", kubeClientObj: []runtime.Object{ - cmWithInvalidDataFormat, + cmWithoutCocurrentData, }, - expectErr: "error to unmarshall data path concurrency configs from node-agent-configs: invalid character 'w' looking for beginning of value", + expectResult: &Configs{nil}, }, { name: "success", @@ -301,8 +302,10 @@ func TestGetConfigs(t *testing.T) { kubeClientObj: []runtime.Object{ cmWithValidData, }, - expectResult: &DataPathConcurrency{ - GlobalConfig: 5, + expectResult: &Configs{ + DataPathConcurrency: &DataPathConcurrency{ + GlobalConfig: 5, + }, }, }, } @@ -321,8 +324,10 @@ func TestGetConfigs(t *testing.T) { if test.expectResult == nil { assert.Nil(t, result) + } else if test.expectResult.DataPathConcurrency == nil { + assert.Nil(t, result.DataPathConcurrency) } else { - assert.Equal(t, *test.expectResult, *result.DataPathConcurrency) + assert.Equal(t, *test.expectResult.DataPathConcurrency, *result.DataPathConcurrency) } } else { assert.EqualError(t, err, test.expectErr)