Skip to content

Commit

Permalink
fix: Print correct error, when non-root user run "rhc status"
Browse files Browse the repository at this point in the history
* Card ID: CCT-288
* When "rhc status" is run as non-root user, then status about
  insighst-client should not tell that insights-client is not
  registered.
* When "rhc status" is run by non-root user, then rhc prints
  decent error about insights client. It prints: "Insights
  client must be run as root."
* Also fixed the case, when status command is run with
  "--format json". When some error happens, then generated
  JSON contains real error message and not empty dictionary.
  • Loading branch information
jirihnidek authored and subpop committed Mar 4, 2024
1 parent 6d561b7 commit a1d1d4d
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 10 deletions.
18 changes: 16 additions & 2 deletions insights.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
package main

import (
"bytes"
"errors"
"fmt"
"os/exec"
"strings"
)

func registerInsights() error {
Expand All @@ -17,15 +21,25 @@ func unregisterInsights() error {
}

func insightsIsRegistered() (bool, error) {
var errBuffer bytes.Buffer
cmd := exec.Command("/usr/bin/insights-client", "--status")
cmd.Stderr = &errBuffer

err := cmd.Run()

if err != nil {
// When the error is ExitError, then we know that insights-client only returned
// some error code not equal to zero. We do not care about error number.
if _, ok := err.(*exec.ExitError); ok {
return false, nil
var exitError *exec.ExitError
if errors.As(err, &exitError) {
// When stderr is not empty, then we should return this as error
// to be able to print this error in rhc output
stdErr := errBuffer.String()
if len(stdErr) == 0 {
return false, nil
} else {
return false, fmt.Errorf("%s", strings.TrimSpace(stdErr))
}
} else {
return false, err
}
Expand Down
10 changes: 5 additions & 5 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -497,13 +497,13 @@ func canonicalFactAction(_ *cli.Context) error {
// like xml:"hostname"
type SystemStatus struct {
SystemHostname string `json:"hostname"`
HostnameError error `json:"hostname_error,omitempty"`
HostnameError string `json:"hostname_error,omitempty"`
RHSMConnected bool `json:"rhsm_connected"`
RHSMError error `json:"rhsm_error,omitempty"`
RHSMError string `json:"rhsm_error,omitempty"`
InsightsConnected bool `json:"insights_connected"`
InsightsError error `json:"insights_error,omitempty"`
InsightsError string `json:"insights_error,omitempty"`
YggdrasilRunning bool `json:"yggdrasil_running"`
YggdrasilError error `json:"yggdrasil_error,omitempty"`
YggdrasilError string `json:"yggdrasil_error,omitempty"`
}

// printJSONStatus tries to print the system status as JSON to stdout.
Expand Down Expand Up @@ -577,7 +577,7 @@ func statusAction(ctx *cli.Context) (err error) {
hostname, err := os.Hostname()
if err != nil {
if uiSettings.isMachineReadable {
systemStatus.HostnameError = err
systemStatus.HostnameError = err.Error()
} else {
return cli.Exit(err, 1)
}
Expand Down
6 changes: 3 additions & 3 deletions status.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func insightStatus(systemStatus *SystemStatus) {
} else {
if uiSettings.isMachineReadable {
systemStatus.InsightsConnected = false
systemStatus.InsightsError = err
systemStatus.InsightsError = err.Error()
} else {
fmt.Printf(uiSettings.iconError+" Cannot execute insights-client: %v\n", err)
}
Expand All @@ -76,15 +76,15 @@ func serviceStatus(systemStatus *SystemStatus) error {
conn, err := systemd.NewSystemConnectionContext(ctx)
if err != nil {
systemStatus.YggdrasilRunning = false
systemStatus.YggdrasilError = err
systemStatus.YggdrasilError = err.Error()
return fmt.Errorf("unable to connect to systemd: %s", err)
}
defer conn.Close()
unitName := ServiceName + ".service"
properties, err := conn.GetUnitPropertiesContext(ctx, unitName)
if err != nil {
systemStatus.YggdrasilRunning = false
systemStatus.YggdrasilError = err
systemStatus.YggdrasilError = err.Error()
return fmt.Errorf("unable to get properties of %s: %s", unitName, err)
}
activeState := properties["ActiveState"]
Expand Down

0 comments on commit a1d1d4d

Please sign in to comment.