Merge pull request #342 from replicatedhq/laverya/improve-systemctl-service-analyzer

expand systemctl service analyzer to also match service sub/load

Author: laverya

pkg/analyze/host_services.go

@@ -117,18 +117,51 @@ func compareHostServicesConditionalToActual(conditional string, services []colle
 		return false, fmt.Errorf("expected exactly 3 parts, got %d", len(parts))
 	}
 
+	matchParams := strings.Split(parts[2], ",")
+	activeMatch := matchParams[0]
+	subMatch := ""
+	loadMatch := ""
+	if len(matchParams) > 1 {
+		subMatch = matchParams[1]
+	}
+	if len(matchParams) > 2 {
+		loadMatch = matchParams[2]
+	}
+
 	switch parts[1] {
 	case "=", "==":
 		for _, service := range services {
 			if isServiceMatch(service.Unit, parts[0]) {
-				return service.Active == parts[2], nil
+				isMatch := true
+				if activeMatch != "" && activeMatch != "*" {
+					isMatch = isMatch && (activeMatch == service.Active)
+				}
+				if subMatch != "" && subMatch != "*" {
+					isMatch = isMatch && (subMatch == service.Sub)
+				}
+				if loadMatch != "" && loadMatch != "*" {
+					isMatch = isMatch && (loadMatch == service.Load)
+				}
+
+				return isMatch, nil
 			}
 		}
 		return false, nil
 	case "!=", "<>":
 		for _, service := range services {
 			if isServiceMatch(service.Unit, parts[0]) {
-				return service.Active != parts[2], nil
+				isMatch := false
+				if activeMatch != "" && activeMatch != "*" {
+					isMatch = isMatch || (activeMatch != service.Active)
+				}
+				if subMatch != "" && subMatch != "*" {
+					isMatch = isMatch || (subMatch != service.Sub)
+				}
+				if loadMatch != "" && loadMatch != "*" {
+					isMatch = isMatch || (loadMatch != service.Load)
+				}
+
+				return isMatch, nil
 			}
 		}
 		return false, nil

pkg/analyze/host_services_test.go

@@ -51,7 +51,7 @@ func TestAnalyzeHostServices(t *testing.T) {
 				},
 				{
 					Unit:   "b.service",
-					Active: "stopped",
+					Active: "inactive",
 				},
 			},
 			hostAnalyzer: &troubleshootv1beta2.HostServicesAnalyze{
@@ -119,6 +119,7 @@ func Test_compareHostServicesConditionalToActual(t *testing.T) {
 				{
 					Unit:   "abc.service",
 					Active: "active",
+					Sub:    "running",
 				},
 			},
 			wantRes: true,
@@ -139,7 +140,8 @@ func Test_compareHostServicesConditionalToActual(t *testing.T) {
 			services: []collect.ServiceInfo{
 				{
 					Unit:   "abc.service",
-					Active: "stopped",
+					Active: "inactive",
+					Sub:    "exited",
 				},
 			},
 			wantRes: false,
@@ -150,11 +152,50 @@ func Test_compareHostServicesConditionalToActual(t *testing.T) {
 			services: []collect.ServiceInfo{
 				{
 					Unit:   "abc.service",
-					Active: "stopped",
+					Active: "inactive",
+					Sub:    "exited",
 				},
 			},
 			wantErr: true,
 		},
+		{
+			name:        "item active matches but not sub",
+			conditional: "abc = active,running",
+			services: []collect.ServiceInfo{
+				{
+					Unit:   "abc.service",
+					Active: "active",
+					Sub:    "exited",
+				},
+			},
+			wantRes: false,
+		},
+		{
+			name:        "item active,sub,load matches",
+			conditional: "abc = active,*,loaded",
+			services: []collect.ServiceInfo{
+				{
+					Unit:   "abc.service",
+					Active: "active",
+					Sub:    "exited",
+					Load:   "loaded",
+				},
+			},
+			wantRes: true,
+		},
+		{
+			name:        "one item active,sub,load does not match with !=",
+			conditional: "abc != active,running,loaded",
+			services: []collect.ServiceInfo{
+				{
+					Unit:   "abc.service",
+					Active: "active",
+					Sub:    "exited",
+					Load:   "loaded",
+				},
+			},
+			wantRes: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {