sys-apps/policymanager: skip update-engine status check if service not found If update-engine.service is not installed on the system, skip checking its status and enforcing policy on it, rather than failing. BUG=b/541334427 TEST=go test ./... RELEASE_NOTE=None Change-Id: Idf6dd1d1a65a620f40b2f8a9587219e905ce9a42
diff --git a/pkg/policyenforcer/policy_enforcer.go b/pkg/policyenforcer/policy_enforcer.go index c876c68..8fd1a37 100644 --- a/pkg/policyenforcer/policy_enforcer.go +++ b/pkg/policyenforcer/policy_enforcer.go
@@ -72,7 +72,7 @@ updateStateErr := false // Start or stop the services to reach desired state. - if serviceStatus.GetUpdateEngine() != config.GetUpdateStrategy() { + if serviceStatus.UpdateEngine != nil && serviceStatus.GetUpdateEngine() != config.GetUpdateStrategy() { updateStrategyMode := false if config.GetUpdateStrategy() == updateEnabledStrategy { updateStrategyMode = true @@ -115,17 +115,26 @@ ServiceStatus := new(protos.ServiceStatus) statusErr := false - isRunning, err := client.systemdClient.IsUnitActiveRunning(serviceMonitor["updateService"]) + var isRunning bool + updateService := serviceMonitor["updateService"] + updateExists, err := client.systemdClient.ServiceExists(updateService) if err != nil { - glog.Error(err) + glog.Errorf("failed to check if %s exists: %v", updateService, err) statusErr = true - - } else { - if isRunning { - ServiceStatus.UpdateEngine = proto.String(updateEnabledStrategy) + } else if updateExists { + isRunning, err = client.systemdClient.IsUnitActiveRunning(updateService) + if err != nil { + glog.Error(err) + statusErr = true } else { - ServiceStatus.UpdateEngine = proto.String(updateDisabledStrategy) + if isRunning { + ServiceStatus.UpdateEngine = proto.String(updateEnabledStrategy) + } else { + ServiceStatus.UpdateEngine = proto.String(updateDisabledStrategy) + } } + } else { + glog.Infof("%s does not exist, skipping status check", updateService) } isRunning, err = client.systemdClient.IsUnitActiveRunning(serviceMonitor["loggingService"])
diff --git a/pkg/policyenforcer/policy_enforcer_test.go b/pkg/policyenforcer/policy_enforcer_test.go index 00d5c99..6ef8ed4 100644 --- a/pkg/policyenforcer/policy_enforcer_test.go +++ b/pkg/policyenforcer/policy_enforcer_test.go
@@ -55,6 +55,7 @@ func TestUpdateServiceState(t *testing.T) { tests := []struct { name string + updateNotExist bool onDiskConfig *protos.InstanceConfig getConfigErr error getStatusErr error @@ -159,6 +160,22 @@ expectChangeMonitoringErr: nil, expectErr: true, }, + { + name: "UpdateServiceNotExist", + updateNotExist: true, + onDiskConfig: &protos.InstanceConfig{ + UpdateStrategy: proto.String("update_enabled"), + }, + getConfigErr: nil, + getStatusErr: nil, + isUpdateEnabled: false, + isLogging: true, + isMonitoring: true, + expectChangeUpdateStrategyErr: nil, + expectChangeLoggingErr: nil, + expectChangeMonitoringErr: nil, + expectErr: false, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -172,10 +189,14 @@ sysapi.AtomicWriteFile(tmpCosDevicePolicyFile, expectedCosDevicePolicyBytes, cosDevicePolicyFilePerm) defer os.RemoveAll(tmpCosDevicePolicyFile) + updateServiceName := "update.service" + if test.updateNotExist { + updateServiceName = "update-not-exist.service" + } serviceMonitor := map[string]string{ "loggingService": fmt.Sprintf("%s,%t,%v", "logging.service", test.isLogging, test.expectChangeLoggingErr), "monitoringService": fmt.Sprintf("%s,%t,%v", "monitoring.service", test.isMonitoring, test.expectChangeMonitoringErr), - "updateService": fmt.Sprintf("%s,%t,%v", "update.service", test.isUpdateEnabled, test.expectChangeUpdateStrategyErr), + "updateService": fmt.Sprintf("%s,%t,%v", updateServiceName, test.isUpdateEnabled, test.expectChangeUpdateStrategyErr), } client := NewPolicyEnforcer(*fakeSystemdClient) @@ -200,6 +221,7 @@ func TestGetServiceStatus(t *testing.T) { tests := []struct { name string + updateNotExist bool isUpdateEnabled bool checkUpdateDisabledError error isLogging bool @@ -281,6 +303,21 @@ }, expectErr: true, }, + { + name: "UpdateServiceNotExist", + updateNotExist: true, + isUpdateEnabled: false, + checkUpdateDisabledError: nil, + isLogging: true, + checkLoggingErr: nil, + isMonitoring: true, + checkMonitoringErr: nil, + expectedStatus: &protos.ServiceStatus{ + Logging: proto.Bool(true), + Monitoring: proto.Bool(true), + }, + expectErr: false, + }, } for _, test := range tests { @@ -288,10 +325,14 @@ var err error fakeSystemdClient := systemd.NewSystemdClient(systemctlCmd) + updateServiceName := "update.service" + if test.updateNotExist { + updateServiceName = "update-not-exist.service" + } serviceMonitor := map[string]string{ "loggingService": fmt.Sprintf("%s,%t,%v", "logging.service", test.isLogging, test.checkLoggingErr), "monitoringService": fmt.Sprintf("%s,%t,%v", "monitoring.service", test.isMonitoring, test.checkMonitoringErr), - "updateService": fmt.Sprintf("%s,%t,%v", "update.service", test.isUpdateEnabled, test.checkUpdateDisabledError), + "updateService": fmt.Sprintf("%s,%t,%v", updateServiceName, test.isUpdateEnabled, test.checkUpdateDisabledError), } client := NewPolicyEnforcer(*fakeSystemdClient)
diff --git a/pkg/policyenforcer/testdata/systemctl.sh b/pkg/policyenforcer/testdata/systemctl.sh index fb4e351..6ce7823 100755 --- a/pkg/policyenforcer/testdata/systemctl.sh +++ b/pkg/policyenforcer/testdata/systemctl.sh
@@ -33,6 +33,15 @@ return 0 fi # perform the following action when the command is + # systemctl show <service.name> -p LoadState + elif [[ $1 == "show" ]]; then + if [[ $2 == *"not-exist"* ]]; then + echo "LoadState=not-found" + else + echo "LoadState=loaded" + fi + exit 0 + # perform the following action when the command is # systemctl start/stop <service.name> elif [[ $1 == "start" || $1 == "stop" ]]; then IFS=',' read -r -a array <<< $2
diff --git a/pkg/systemd/systemd_client.go b/pkg/systemd/systemd_client.go index 043ef00..c9b7819 100644 --- a/pkg/systemd/systemd_client.go +++ b/pkg/systemd/systemd_client.go
@@ -16,6 +16,7 @@ import ( "bytes" + "fmt" "os/exec" "policy-manager/pkg/sysapi" "strings" @@ -40,9 +41,16 @@ err := cmd.Run() output := strings.TrimSpace(string(stdout.Bytes())) - if err != nil && output != "inactive" { + if err != nil { + if exitError, ok := err.(*exec.ExitError); ok { + code := exitError.ExitCode() + if code == 3 || code == 4 { + return false, nil + } + } return false, err - } else if output == "active" { + } + if output == "active" { return true, nil } return false, nil @@ -57,3 +65,16 @@ _, _, err := sysapi.RunCommand(systemd.systemctlCmd, "stop", name) return err } + +// ServiceExists checks if a systemd service exists (is loaded). +func (systemd *SystemdClient) ServiceExists(name string) (bool, error) { + stdout, _, err := sysapi.RunCommand(systemd.systemctlCmd, "show", name, "-p", "LoadState") + if err != nil { + return false, err + } + output := strings.TrimSpace(string(stdout)) + if !strings.HasPrefix(output, "LoadState=") { + return false, fmt.Errorf("unexpected output from systemctl show: %q", output) + } + return !strings.Contains(output, "LoadState=not-found"), nil +}
diff --git a/pkg/systemd/systemd_client_test.go b/pkg/systemd/systemd_client_test.go index b333e74..1b5c308 100644 --- a/pkg/systemd/systemd_client_test.go +++ b/pkg/systemd/systemd_client_test.go
@@ -26,7 +26,7 @@ tests := []struct { name string unitName string - cmdStdout []byte + simulatedOutput string isActive bool cmdErr error expectActiveRunning bool @@ -35,7 +35,7 @@ { name: "UnitIsActiveRunning", unitName: "stackdriver-logging.service", - cmdStdout: []byte("ActiveState=active\nSubState=running\n"), + simulatedOutput: "active", isActive: true, cmdErr: nil, expectActiveRunning: true, @@ -44,7 +44,7 @@ { name: "UnitIsActiveNotRunning", unitName: "node-problem-detector.service", - cmdStdout: []byte("ActiveState=active\nSubState=starting\n"), + simulatedOutput: "active", isActive: true, cmdErr: nil, expectActiveRunning: true, @@ -53,7 +53,7 @@ { name: "UnitIsInactiveNotRunning", unitName: "node-problem-detector.service", - cmdStdout: []byte("ActiveState=inactive\nSubState=dead\n"), + simulatedOutput: "inactive", isActive: false, cmdErr: nil, expectActiveRunning: false, @@ -63,7 +63,7 @@ { name: "UnitIsFailed", unitName: "node-problem-detector.service", - cmdStdout: []byte("ActiveState=failed\nSubState=failed\n"), + simulatedOutput: "failed", isActive: false, cmdErr: nil, expectActiveRunning: false, @@ -72,7 +72,7 @@ { name: "UnitIsActivating", unitName: "node-problem-detector.service", - cmdStdout: []byte("ActiveState=activating\nSubState=starting\n"), + simulatedOutput: "activating", isActive: false, cmdErr: nil, expectActiveRunning: false, @@ -81,7 +81,7 @@ { name: "NotExistUnit", unitName: "thisisnotaunit.nonunit", - cmdStdout: []byte("ActiveState=inactive\nSubState=dead\n"), + simulatedOutput: "unknown", isActive: false, cmdErr: nil, expectActiveRunning: false, @@ -90,7 +90,7 @@ { name: "IgnoreOutputWhenCommandFailed", unitName: "node-problem-detector.service", - cmdStdout: []byte("ActiveState=active\nSubState=running\n"), + simulatedOutput: "active", isActive: true, cmdErr: errors.New("Some error."), expectActiveRunning: false, @@ -100,7 +100,7 @@ for _, test := range tests { t.Run(test.name, func(t *testing.T) { systemdClient := NewSystemdClient(systemctlCmd) - unitName := fmt.Sprintf("%s,%t,%v", test.unitName, test.isActive, test.cmdErr) + unitName := fmt.Sprintf("%s,%t,%v,%s", test.unitName, test.isActive, test.cmdErr, test.simulatedOutput) activeRunning, err := systemdClient.IsUnitActiveRunning(unitName) if activeRunning != test.expectActiveRunning { t.Errorf("got ActiveState %v, expect to have ActiveState %v",
diff --git a/pkg/systemd/testdata/systemctl.sh b/pkg/systemd/testdata/systemctl.sh index 600cbab..534a856 100755 --- a/pkg/systemd/testdata/systemctl.sh +++ b/pkg/systemd/testdata/systemctl.sh
@@ -24,14 +24,23 @@ # exit with status code 1 when expected error is not nil if [[ "${array[2]}" != "<nil>" ]]; then exit 1 - # if the expected state of systemd service is active, - # echo active - elif [[ "${array[1]}" == "true" ]]; then - echo "active" - # if no error is expected and status is not active, return success - else - return 0 fi + output="${array[3]}" + echo "$output" + if [[ "$output" == "active" ]]; then + exit 0 + else + exit 3 + fi + # perform the following action when the command is + # systemctl show <service.name> -p LoadState + elif [[ $1 == "show" ]]; then + if [[ $2 == *"not-exist"* ]]; then + echo "LoadState=not-found" + else + echo "LoadState=loaded" + fi + exit 0 # perform the following action when the command is # systemctl start/stop <service.name> elif [[ $1 == "start" || $1 == "stop" ]]; then