kubernetes
/
autoscaler
mirror of https://github.com/kubernetes/autoscaler.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

addressed review feedback items

This commit is contained in:
“gkazanci” 2024-10-24 16:46:48 +01:00
parent cf061f3194
commit 6396e63dcf
3 changed files with 74 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
cluster-autoscaler/cloudprovider/oci/common/oci_util.go
View File

@ -219,7 +219,10 @@ func HasNodeGroupTags(nodeGroupAutoDiscoveryList []string) (bool, bool, error) {
}
}
if instancePoolTagsFound == true && nodePoolTagsFound == true {
return instancePoolTagsFound, nodePoolTagsFound, fmt.Errorf("can not use both instancepoolTags and nodepoolTags")
return instancePoolTagsFound, nodePoolTagsFound, fmt.Errorf("can not use both instancepoolTags and nodepoolTags in node-group-auto-discovery")
}
if len(nodeGroupAutoDiscoveryList) > 0 && instancePoolTagsFound == false && nodePoolTagsFound == false {
return instancePoolTagsFound, nodePoolTagsFound, fmt.Errorf("either instancepoolTags or nodepoolTags should be provided in node-group-auto-discovery")
}
return instancePoolTagsFound, nodePoolTagsFound, nil
}

7
cluster-autoscaler/cloudprovider/oci/nodepools/oci_manager.go
View File

@ -272,8 +272,7 @@ func nodePoolFromArg(value string) (*nodePool, error) {
// nodeGroupFromArg parses a node group spec represented in the form of
// `clusterId:<clusterId>,compartmentId:<compartmentId>,nodepoolTags:<tagKey1>=<tagValue1>&<tagKey2>=<tagValue2>,min:<min>,max:<max>`
// and produces a node group auto discovery object,
// nodepoolTags are optional and CA will capture all nodes if no tags are provided.
// and produces a node group auto discovery object
func nodeGroupFromArg(value string) (*nodeGroupAutoDiscovery, error) {
// this regex will find the key-value pairs in any given order if separated with a colon
regexPattern := `(?:` + compartmentId + `:(?P<` + compartmentId + `>[^,]+)`
@ -333,8 +332,12 @@ func nodeGroupFromArg(value string) (*nodeGroupAutoDiscovery, error) {
parts := strings.Split(pair, "=")
if len(parts) == 2 {
spec.tags[parts[0]] = parts[1]
} else {
return nil, fmt.Errorf("nodepoolTags should be given in tagKey=tagValue format, this is not valid: %s", pair)
}
}
} else {
return nil, fmt.Errorf("failed to set %s, it is missing in node-group-auto-discovery parameter", nodepoolTags)
}
klog.Infof("node group auto discovery spec constructed: %+v", spec)

104
cluster-autoscaler/cloudprovider/oci/nodepools/oci_manager_test.go
View File

@ -390,16 +390,16 @@ func TestRemoveInstance(t *testing.T) {
}
func TestNodeGroupFromArg(t *testing.T) {
var nodeGroupArg = "clusterId:testClusterId,compartmentId:testCompartmentId,nodepoolTags:ca-managed=true&namespace.foo=bar,min:1,max:5"
var nodeGroupArg = "clusterId:ocid1.cluster.oc1.test-region.test,compartmentId:ocid1.compartment.oc1.test-region.test,nodepoolTags:ca-managed=true&namespace.foo=bar,min:1,max:5"
nodeGroupAutoDiscovery, err := nodeGroupFromArg(nodeGroupArg)
if err != nil {
t.Errorf("Error: #{err}")
}
if nodeGroupAutoDiscovery.clusterId != "testClusterId" {
t.Errorf("Error: clusterId should be testClusterId")
if nodeGroupAutoDiscovery.clusterId != "ocid1.cluster.oc1.test-region.test" {
t.Errorf("Error: clusterId should be ocid1.cluster.oc1.test-region.test")
}
if nodeGroupAutoDiscovery.compartmentId != "testCompartmentId" {
t.Errorf("Error: compartmentId should be testCompartmentId")
if nodeGroupAutoDiscovery.compartmentId != "ocid1.compartment.oc1.test-region.test" {
t.Errorf("Error: compartmentId should be ocid1.compartment.oc1.test-region.test")
}
if nodeGroupAutoDiscovery.minSize != 1 {
t.Errorf("Error: minSize should be 1")
@ -417,41 +417,67 @@ func TestNodeGroupFromArg(t *testing.T) {
func TestValidateNodePoolTags(t *testing.T) {
var nodeGroupTags map[string]string = nil
var nodePoolTags map[string]string = nil
var definedTags map[string]map[string]interface{} = nil
if validateNodepoolTags(nodeGroupTags, nodePoolTags, definedTags) == false {
t.Errorf("validateNodepoolTags shouldn't return false for empty tags map")
testCases := map[string]struct {
nodeGroupTags map[string]string
freeFormTags map[string]string
definedTags map[string]map[string]interface{}
expectedResult bool
}{
"no-tags": {
nodeGroupTags: nil,
freeFormTags: nil,
definedTags: nil,
expectedResult: true,
},
"node-group tags provided but no tags on nodepool": {
nodeGroupTags: map[string]string{
"testTag": "testTagValue",
},
freeFormTags: nil,
definedTags: nil,
expectedResult: false,
},
"node-group tags and free-form tags do not match": {
nodeGroupTags: map[string]string{
"testTag": "testTagValue",
},
freeFormTags: map[string]string{
"foo": "bar",
},
definedTags: nil,
expectedResult: false,
},
"free-form tags have required node-group tags": {
nodeGroupTags: map[string]string{
"testTag": "testTagValue",
},
freeFormTags: map[string]string{
"foo": "bar",
"testTag": "testTagValue",
},
definedTags: nil,
expectedResult: true,
},
"defined tags have required node-group tags": {
nodeGroupTags: map[string]string{
"ns.testTag": "testTagValue",
},
freeFormTags: nil,
definedTags: map[string]map[string]interface{}{
"ns": {
"testTag": "testTagValue",
},
},
expectedResult: true,
},
}
nodeGroupTags = make(map[string]string)
nodeGroupTags["test"] = "test"
if validateNodepoolTags(nodeGroupTags, nodePoolTags, definedTags) == true {
t.Errorf("validateNodepoolTags shouldn't return true for tags missing")
}
nodePoolTags = make(map[string]string)
nodePoolTags["foo"] = "bar"
if validateNodepoolTags(nodeGroupTags, nodePoolTags, definedTags) == true {
t.Errorf("validateNodepoolTags shouldn't return true for not matching tags")
}
nodePoolTags["test"] = "test"
if validateNodepoolTags(nodeGroupTags, nodePoolTags, definedTags) == false {
t.Errorf("validateNodepoolTags shouldn't return false for matching tags")
}
nodeGroupTags["ns.tag1"] = "tag2"
definedTagsMap := make(map[string]interface{})
definedTagsMap["tag1"] = "tag2"
definedTags = make(map[string]map[string]interface{})
definedTags["ns"] = definedTagsMap
if validateNodepoolTags(nodeGroupTags, nodePoolTags, definedTags) == false {
t.Errorf("validateNodepoolTags shouldn't return false for namespaced tags")
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
result := validateNodepoolTags(tc.nodeGroupTags, tc.freeFormTags, tc.definedTags)
if result != tc.expectedResult {
t.Errorf("Testcase '%s' failed: got %t ; expected %t", name, result, tc.expectedResult)
}
})
}
}