diff --git a/asconfig/as_setconfig.go b/asconfig/as_setconfig.go index dab2c5c..178daa3 100644 --- a/asconfig/as_setconfig.go +++ b/asconfig/as_setconfig.go @@ -1,12 +1,9 @@ package asconfig import ( - "container/list" "fmt" "strings" - "github.com/go-logr/logr" - aero "github.com/aerospike/aerospike-client-go/v7" "github.com/aerospike/aerospike-management-lib/deployment" "github.com/aerospike/aerospike-management-lib/info" @@ -303,14 +300,7 @@ func createSetConfigXDRCmdList(tokens []string, operationValueMap map[OpType][]s // example of a command: "set-config:context=xdr;dc=dc1;node-address-port=192.168.55.210:3000;action=add if prevToken == keyNodeAddressPorts { - val := v - - tokens := strings.Split(v, colon) - if len(tokens) >= 2 { - val = tokens[0] + colon + tokens[1] - } - - finalCMD = cmd + semicolon + SingularOf(prevToken) + equal + val + semicolon + "action" + equal + string(op) + finalCMD = cmd + semicolon + SingularOf(prevToken) + equal + v + semicolon + "action" + equal + string(op) } else { finalCMD = cmd + semicolon + SingularOf(prevToken) + equal + v } @@ -323,14 +313,12 @@ func createSetConfigXDRCmdList(tokens []string, operationValueMap map[OpType][]s } // CreateSetConfigCmdList creates set-config commands for given config. -func CreateSetConfigCmdList( - log logr.Logger, configMap DynamicConfigMap, conn deployment.ASConnInterface, +func CreateSetConfigCmdList(configMap DynamicConfigMap, conn deployment.ASConnInterface, aerospikePolicy *aero.ClientPolicy, ) ([]string, error) { cmdList := make([]string, 0, len(configMap)) - orderedConfList := rearrangeConfigMap(log, configMap) - for _, c := range orderedConfList { + for c := range configMap { tokens := strings.Split(c, sep) context := tokens[0] @@ -368,6 +356,7 @@ func CreateSetConfigCmdList( return cmdList, nil } +/* // Returns a list of config keys in the order in which they should be applied. // The order is as follows: // 1. Removed Namespaces -- If user has to change some of the DC direct fields, they will have to remove the namespace @@ -380,8 +369,8 @@ func rearrangeConfigMap(log logr.Logger, configMap DynamicConfigMap) []string { finalList := make([]string, 0, len(configMap)) var ( - lastDC *list.Element // Last DC name - lastNAP *list.Element // Last DC direct field eg. node-address-ports + lastDC *list.Element // Last DC name + lastDCConfig *list.Element // Last DC config eg. node-address-ports ) for k, v := range configMap { @@ -404,14 +393,14 @@ func rearrangeConfigMap(log logr.Logger, configMap DynamicConfigMap) []string { finalList = append(finalList, k) } else { // If namespace is added, add it after all DCs and their direct fields - if lastNAP == nil { + if lastDCConfig == nil { if lastDC != nil { rearrangedConfigMap.InsertAfter(k, lastDC) } else { rearrangedConfigMap.PushFront(k) } } else { - rearrangedConfigMap.InsertAfter(k, lastNAP) + rearrangedConfigMap.InsertAfter(k, lastDCConfig) } } } @@ -424,14 +413,34 @@ func rearrangeConfigMap(log logr.Logger, configMap DynamicConfigMap) []string { // Handle DC direct fields if tokens[len(tokens)-3] == info.ConfigDCContext { var nap *list.Element + // Check if the key is related to 'node-address-ports' + isNodeAddressPortsKey := strings.HasSuffix(k, sep+keyNodeAddressPorts) + + if isNodeAddressPortsKey { + if _, ok := v[Remove]; ok { + dc := rearrangedConfigMap.PushFront(k) + if lastDC == nil { + lastDC = dc + } + continue + } else { + if lastDCConfig != nil { + // Add 'node-address-ports' after all DC direct fields + // There are certain fields that must be set before 'node-address-ports', for example, 'tls-name'. + lastDCConfig = rearrangedConfigMap.InsertAfter(k, lastDCConfig) + continue + } + } + } + if lastDC == nil { nap = rearrangedConfigMap.PushFront(k) } else { // Add modified DC direct fields after the DC names and before the namespaces nap = rearrangedConfigMap.InsertAfter(k, lastDC) } - if lastNAP == nil { - lastNAP = nap + if lastDCConfig == nil { + lastDCConfig = nap } } else { rearrangedConfigMap.PushBack(k) @@ -445,6 +454,7 @@ func rearrangeConfigMap(log logr.Logger, configMap DynamicConfigMap) []string { return finalList } +*/ func ValidConfigOperations() []OpType { return []OpType{Add, Update, Remove} @@ -487,6 +497,12 @@ func CreateConfigSetCmdsUsingPatch( continue } + if ok, _ := isListField(k); ok { + // Ignore these fields as these operations are not update operations + //TODO: Should we through an error if these fields are present in the configMap patch? + continue + } + valueMap := make(map[OpType]interface{}) valueMap[Update] = v asConfChange[k] = valueMap @@ -501,7 +517,7 @@ func CreateConfigSetCmdsUsingPatch( return nil, fmt.Errorf("static field has been changed, cannot change config dynamically") } - return CreateSetConfigCmdList(conn.Log, asConfChange, conn, aerospikePolicy) + return CreateSetConfigCmdList(asConfChange, conn, aerospikePolicy) } func CreateConfigSetCmdsUsingOperation( @@ -538,5 +554,5 @@ func CreateConfigSetCmdsUsingOperation( return nil, fmt.Errorf("static field has been changed, cannot change config dynamically") } - return CreateSetConfigCmdList(conn.Log, asConfChange, conn, aerospikePolicy) + return CreateSetConfigCmdList(asConfChange, conn, aerospikePolicy) } diff --git a/asconfig/as_setconfig_test.go b/asconfig/as_setconfig_test.go index 937de8b..9774c6b 100644 --- a/asconfig/as_setconfig_test.go +++ b/asconfig/as_setconfig_test.go @@ -3,7 +3,6 @@ package asconfig import ( "testing" - "github.com/go-logr/logr" "github.com/stretchr/testify/suite" "go.uber.org/mock/gomock" @@ -63,12 +62,11 @@ func (s *AsSetConfigTestSuite) TestCreateSetConfigCmdList() { for _, tc := range testCases { s.Run(tc.name, func() { - logger := logr.Discard() policy := &aero.ClientPolicy{} s.mockASConn.EXPECT().RunInfo(gomock.Any(), gomock.Any()).Return(map[string]string{ "logs": "0:stderr"}, nil).AnyTimes() - result, err := CreateSetConfigCmdList(logger, tc.inputConf, s.mockASConn, policy) + result, err := CreateSetConfigCmdList(tc.inputConf, s.mockASConn, policy) s.Assert().Nil(err) s.Assert().True(gomock.InAnyOrder(result).Matches(tc.expected)) @@ -76,6 +74,7 @@ func (s *AsSetConfigTestSuite) TestCreateSetConfigCmdList() { } } +/* func (s *AsSetConfigTestSuite) TestCreateSetConfigCmdListOrdered() { testCases := []struct { name string @@ -88,12 +87,15 @@ func (s *AsSetConfigTestSuite) TestCreateSetConfigCmdListOrdered() { "xdr.dcs.{DC1}.namespaces.{ns1}.bin-policy": {Update: "no-bins"}, "xdr.dcs.{DC3}.name": {Add: "DC3"}, "xdr.dcs.{DC1}.namespaces.{ns2}.name": {Remove: "ns2"}, - "xdr.dcs.{DC1}.node-address-ports": {Add: []string{"1.1.1.1:3000"}}, + "xdr.dcs.{DC1}.node-address-ports": {Add: []string{"1.1.1.1:3000:tls-name"}, "xdr.dcs.{DC1}.namespaces.{ns1}.name": {Add: "ns1"}, + "xdr.dcs.{DC1}.tls-name": {Update: "tls-name"}, }, []string{"set-config:context=xdr;dc=DC1;namespace=ns2;action=remove", "set-config:context=xdr;dc=DC3;action=create", - "set-config:context=xdr;dc=DC1;node-address-port=1.1.1.1:3000;action=add", + "set-config:context=xdr;dc=DC1;tls-name=tls-name", + "set-config:context=xdr;dc=DC1;node-address-port=1.1.1.1:3000:tls-name;action=add", + // "set-config:context=xdr;dc=DC1;node-address-port=1.1.1.1:3000;action=remove", "set-config:context=xdr;dc=DC1;namespace=ns1;action=add", "set-config:context=xdr;dc=DC1;namespace=ns1;bin-policy=no-bins", }, @@ -112,6 +114,7 @@ func (s *AsSetConfigTestSuite) TestCreateSetConfigCmdListOrdered() { }) } } +*/ func TestAsSetConfigTestSuite(t *testing.T) { suite.Run(t, new(AsSetConfigTestSuite)) diff --git a/asconfig/asconfig.go b/asconfig/asconfig.go index 08e2e1f..0ac6fb9 100644 --- a/asconfig/asconfig.go +++ b/asconfig/asconfig.go @@ -29,7 +29,7 @@ type ValidationErr struct { Field string } -// NewMapAsConfig creates AsConfig. Typically an unmarshalled yaml file is passed in +// NewMapAsConfig creates AsConfig. Typically, an unmarshalled yaml file is passed in func NewMapAsConfig( log logr.Logger, configMap map[string]interface{}, ) (*AsConfig, error) { diff --git a/asconfig/asconfig_test.go b/asconfig/asconfig_test.go index b93bd71..86e83c3 100644 --- a/asconfig/asconfig_test.go +++ b/asconfig/asconfig_test.go @@ -356,13 +356,9 @@ func (s *AsConfigTestSuite) TestAsConfigIsDynamic() { { "dynamic fields", DynamicConfigMap{ - "xdr.dcs.{DC1}.namespaces.{ns1}.bin-policy": {Update: "no-bins"}, "security.log.report-data-op": {Add: []string{"ns3 set2"}, Remove: []string{"ns2 set2"}}, - "xdr.dcs.{DC3}.name": {Remove: "DC3"}, - "xdr.dcs.{DC1}.node-address-ports": {Update: []string{"1.1.1.1 3000"}}, - "xdr.dcs.{DC1}.namespaces.{ns1}.name": {Add: "ns1"}, - "xdr.dcs.{DC1}.name": {Add: "DC1"}, + "service.proto-fd-max": {Update: "1000"}, }, true, diff --git a/asconfig/schema.go b/asconfig/schema.go index 0d5418c..2c6c24d 100644 --- a/asconfig/schema.go +++ b/asconfig/schema.go @@ -200,12 +200,12 @@ func isFieldDynamic(log logr.Logger, dynamic sets.Set[string], conf string, baseKey := tokens[len(tokens)-1] context := tokens[0] - if baseKey == "replication-factor" || baseKey == keyNodeAddressPorts { - return true + if context == info.ConfigXDRContext { + // XDR context is always considered static. + return false } - // Name key for XDR context is considered as dynamic, as both DCs and Namespaces in DCs can be added dynamically. - if context == info.ConfigXDRContext && baseKey == KeyName { + if baseKey == "replication-factor" { return true } diff --git a/asconfig/utils.go b/asconfig/utils.go index 3b46f26..8fd0123 100644 --- a/asconfig/utils.go +++ b/asconfig/utils.go @@ -1069,7 +1069,7 @@ func isNodeSpecificField(key string) bool { keyTLSAddress, "tls-port", keyTLSAccessAddress, "tls-access-port", keyTLSAlternateAccessAddress, "tls-alternate-access-port", "alternate-access-port", "xdr-info-port", "service-threads", "batch-index-threads", - "mesh-seed-address-port", "mtu": + "mesh-seed-address-port", "tls-mesh-seed-address-port", "mtu": return true }