diff --git a/pkg/analyze/node_resources.go b/pkg/analyze/node_resources.go index be39c039..f42c0843 100644 --- a/pkg/analyze/node_resources.go +++ b/pkg/analyze/node_resources.go @@ -94,9 +94,19 @@ func analyzeNodeResources(analyzer *troubleshootv1beta1.NodeResources, getCollec return result, nil } -func compareNodeResourceConditionalToActual(conditional string, matchingNodes []corev1.Node, totalNodeCount int) (bool, error) { +func compareNodeResourceConditionalToActual(conditional string, matchingNodes []corev1.Node, totalNodeCount int) (res bool, err error) { + res = false + err = nil + + defer func() { + if r := recover(); r != nil { + err = errors.Errorf("failed to evaluate %q: %v", conditional, r) + } + }() + if conditional == "" { - return true, nil + res = true + return } parts := strings.Fields(strings.TrimSpace(conditional)) @@ -106,7 +116,8 @@ func compareNodeResourceConditionalToActual(conditional string, matchingNodes [] } if len(parts) != 3 { - return false, errors.New("unable to parse nodeResources conditional") + err = errors.New("unable to parse nodeResources conditional") + return } operator := parts[1] @@ -117,6 +128,8 @@ func compareNodeResourceConditionalToActual(conditional string, matchingNodes [] parsedDesiredValue, err := strconv.Atoi(parts[2]) if err == nil { desiredValue = parsedDesiredValue + } else { + err = nil // try parsing as a resource } reg := regexp.MustCompile(`(?P.*)\((?P.*)\)`) @@ -128,7 +141,8 @@ func compareNodeResourceConditionalToActual(conditional string, matchingNodes [] } if match == nil || len(match) != 3 { - return false, errors.New("conditional does not match pattern of function(property?)") + err = errors.New("conditional does not match pattern of function(property?)") + return } function := match[1] @@ -151,70 +165,86 @@ func compareNodeResourceConditionalToActual(conditional string, matchingNodes [] case "=", "==", "===": if _, ok := actualValue.(int); ok { if _, ok := desiredValue.(int); ok { - return actualValue.(int) == desiredValue.(int), nil + res = actualValue.(int) == desiredValue.(int) + return } } if _, ok := desiredValue.(string); ok { - return actualValue.(*resource.Quantity).Cmp(resource.MustParse(desiredValue.(string))) == 0, nil + res = actualValue.(*resource.Quantity).Cmp(resource.MustParse(desiredValue.(string))) == 0 + return } - return actualValue.(*resource.Quantity).Cmp(resource.MustParse(strconv.Itoa(desiredValue.(int)))) == 0, nil + res = actualValue.(*resource.Quantity).Cmp(resource.MustParse(strconv.Itoa(desiredValue.(int)))) == 0 + return case "<": if _, ok := actualValue.(int); ok { if _, ok := desiredValue.(int); ok { - return actualValue.(int) < desiredValue.(int), nil + res = actualValue.(int) < desiredValue.(int) + return } } if _, ok := desiredValue.(string); ok { - return actualValue.(*resource.Quantity).Cmp(resource.MustParse(desiredValue.(string))) == -1, nil + res = actualValue.(*resource.Quantity).Cmp(resource.MustParse(desiredValue.(string))) == -1 + return } - return actualValue.(*resource.Quantity).Cmp(resource.MustParse(strconv.Itoa(desiredValue.(int)))) == -1, nil + res = actualValue.(*resource.Quantity).Cmp(resource.MustParse(strconv.Itoa(desiredValue.(int)))) == -1 + return case ">": if _, ok := actualValue.(int); ok { if _, ok := desiredValue.(int); ok { - return actualValue.(int) > desiredValue.(int), nil + res = actualValue.(int) > desiredValue.(int) + return } } if _, ok := desiredValue.(string); ok { - return actualValue.(*resource.Quantity).Cmp(resource.MustParse(desiredValue.(string))) == 1, nil + res = actualValue.(*resource.Quantity).Cmp(resource.MustParse(desiredValue.(string))) == 1 + return } - return actualValue.(*resource.Quantity).Cmp(resource.MustParse(strconv.Itoa(desiredValue.(int)))) == 1, nil + res = actualValue.(*resource.Quantity).Cmp(resource.MustParse(strconv.Itoa(desiredValue.(int)))) == 1 + return case "<=": if _, ok := actualValue.(int); ok { if _, ok := desiredValue.(int); ok { - return actualValue.(int) <= desiredValue.(int), nil + res = actualValue.(int) <= desiredValue.(int) + return } } if _, ok := desiredValue.(string); ok { - return actualValue.(*resource.Quantity).Cmp(resource.MustParse(desiredValue.(string))) == 0 || - actualValue.(*resource.Quantity).Cmp(resource.MustParse(desiredValue.(string))) == -1, nil + res = actualValue.(*resource.Quantity).Cmp(resource.MustParse(desiredValue.(string))) == 0 || + actualValue.(*resource.Quantity).Cmp(resource.MustParse(desiredValue.(string))) == -1 + return } - return actualValue.(*resource.Quantity).Cmp(resource.MustParse(strconv.Itoa(desiredValue.(int)))) == 0 || - actualValue.(*resource.Quantity).Cmp(resource.MustParse(strconv.Itoa(desiredValue.(int)))) == -1, nil + res = actualValue.(*resource.Quantity).Cmp(resource.MustParse(strconv.Itoa(desiredValue.(int)))) == 0 || + actualValue.(*resource.Quantity).Cmp(resource.MustParse(strconv.Itoa(desiredValue.(int)))) == -1 + return case ">=": if _, ok := actualValue.(int); ok { if _, ok := desiredValue.(int); ok { - return actualValue.(int) >= desiredValue.(int), nil + res = actualValue.(int) >= desiredValue.(int) + return } } if _, ok := desiredValue.(string); ok { - return actualValue.(*resource.Quantity).Cmp(resource.MustParse(desiredValue.(string))) == 0 || - actualValue.(*resource.Quantity).Cmp(resource.MustParse(desiredValue.(string))) == 1, nil + res = actualValue.(*resource.Quantity).Cmp(resource.MustParse(desiredValue.(string))) == 0 || + actualValue.(*resource.Quantity).Cmp(resource.MustParse(desiredValue.(string))) == 1 + return } - return actualValue.(*resource.Quantity).Cmp(resource.MustParse(strconv.Itoa(desiredValue.(int)))) == 0 || - actualValue.(*resource.Quantity).Cmp(resource.MustParse(strconv.Itoa(desiredValue.(int)))) == 1, nil + res = actualValue.(*resource.Quantity).Cmp(resource.MustParse(strconv.Itoa(desiredValue.(int)))) == 0 || + actualValue.(*resource.Quantity).Cmp(resource.MustParse(strconv.Itoa(desiredValue.(int)))) == 1 + return } - return false, errors.New("unexpected conditional in nodeResources") + err = errors.New("unexpected conditional in nodeResources") + return } func getQuantity(node corev1.Node, property string) *resource.Quantity { diff --git a/pkg/analyze/node_resources_test.go b/pkg/analyze/node_resources_test.go index 4ad6496b..529db51b 100644 --- a/pkg/analyze/node_resources_test.go +++ b/pkg/analyze/node_resources_test.go @@ -67,6 +67,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { totalNodeCount int matchingNodes []corev1.Node expected bool + isError bool }{ { name: "=", @@ -74,6 +75,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "count()", @@ -81,6 +83,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "<", @@ -88,6 +91,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "count() <", @@ -95,6 +99,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: ">", @@ -102,6 +107,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: false, + isError: false, }, { name: "count() >", @@ -109,6 +115,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "count() >= 1 (true)", @@ -116,6 +123,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "count() <= 2 (true)", @@ -123,6 +131,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "count() <= 1 (false)", @@ -130,6 +139,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: false, + isError: false, }, { name: "min(memoryCapacity) < 4Gi (true)", @@ -137,6 +147,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "min(memoryCapacity) >= 4Gi (false)", @@ -144,6 +155,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: false, + isError: false, }, { name: "min(memoryAllocatable) == 16Ki (true)", @@ -151,6 +163,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "min(cpuCapacity) == 2 (true)", @@ -158,6 +171,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "min(cpuAllocatable) == 1.5 (true)", @@ -165,6 +179,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "min(podCapacity) == 15 (true)", @@ -172,6 +187,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "min(podAllocatable) == 12 (true)", @@ -179,6 +195,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "min(ephemeralStorageCapacity) <= 20Gi (true)", @@ -186,6 +203,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "min(ephemeralStorageCapacity) > 20Gi (false)", @@ -193,6 +211,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: false, + isError: false, }, { name: "min(ephemeralStorageAllocatable) == 12316009748 (true)", @@ -200,6 +219,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "max(memoryCapacity) == 7951376Ki (true)", @@ -207,6 +227,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "max(memoryAllocatable) == 7848976Ki (true)", @@ -214,6 +235,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "max(cpuCapacity) == 12 (false)", @@ -221,6 +243,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: false, + isError: false, }, { name: "max(cpuCapacity) == 4 (true)", @@ -228,6 +251,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "max(cpuAllocatable) == 3 (true)", @@ -235,6 +259,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "max(podCapacity) == 29 (true)", @@ -242,6 +267,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "max(podAllocatable) == 14 (true)", @@ -249,6 +275,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "max(ephemeralStorageCapacity) == 20959212Ki (true)", @@ -256,6 +283,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "max(ephemeralStorageAllocatable) == 19316009748 (true)", @@ -263,6 +291,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "sum(memoryCapacity) > 7951376Ki (true)", @@ -270,6 +299,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "sum(memoryAllocatable) > 7848976Ki (true)", @@ -277,6 +307,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "sum(cpuCapacity) > 5 (true)", @@ -284,6 +315,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "sum(cpuAllocatable) == 4.5 (true)", @@ -291,6 +323,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "sum(podCapacity) == 44 (true)", @@ -298,6 +331,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "sum(podAllocatable) == 26 (true)", @@ -305,6 +339,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "sum(ephemeralStorageCapacity) > 20959212Ki (true)", @@ -312,6 +347,7 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, }, { name: "sum(ephemeralStorageAllocatable) > 19316009748 (true)", @@ -319,6 +355,15 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { matchingNodes: nodeData, totalNodeCount: len(nodeData), expected: true, + isError: false, + }, + { + name: "sum(ephemeralStorageAllocatable) > 19316009748 (error)", + conditional: "sum(ephemeralStorageAllocatable) > \"19316009748\"", + matchingNodes: nodeData, + totalNodeCount: len(nodeData), + expected: false, + isError: true, }, } @@ -327,7 +372,11 @@ func Test_compareNodeResourceConditionalToActual(t *testing.T) { req := require.New(t) actual, err := compareNodeResourceConditionalToActual(test.conditional, test.matchingNodes, test.totalNodeCount) - req.NoError(err) + if test.isError { + req.Error(err) + } else { + req.NoError(err) + } assert.Equal(t, test.expected, actual)