Address feedback

internal/agent/agent.go

@@ -47,7 +47,7 @@ func NewAgent(config *Config) (*Agent, error) {
 		discovery.NewCloudDiscovery(collectorClient, *config.DiscoveriesConfig),
 		discovery.NewSubscriptionDiscovery(collectorClient, config.InstanceName, *config.DiscoveriesConfig),
 		discovery.NewHostDiscovery(collectorClient, config.InstanceName, *config.DiscoveriesConfig),
-		discovery.NewSaptuneDiscovery(collectorClient, config.InstanceName, *config.DiscoveriesConfig),
+		discovery.NewSaptuneDiscovery(collectorClient, *config.DiscoveriesConfig),
 	}
 
 	agent := &Agent{

internal/core/saptune/saptune.go

@@ -1,8 +1,6 @@
 package saptune
 
 import (
-	"encoding/json"
-
 	"github.com/pkg/errors"
 	log "github.com/sirupsen/logrus"
 	"github.com/trento-project/agent/pkg/utils"
@@ -19,53 +17,8 @@ const (
 )
 
 type Saptune struct {
-	Version  *string  `json:"version"`
-	Commands []Output `json:"commands"`
-}
-type Output struct {
-	Schema      string    `json:"$schema"`
-	PublishTime string    `json:"publish time"`
-	Argv        string    `json:"argv"`
-	Pid         int       `json:"pid"`
-	Command     string    `json:"command"`
-	ExitCode    int       `json:"exit code"`
-	Result      Result    `json:"result"`
-	Messages    []Message `json:"messages"`
-}
-
-type Result struct {
-	Services                 Services `json:"services"`
-	SystemdSystemState       string   `json:"systemd system state"`
-	TuningState              string   `json:"tuning state"`
-	Virtualization           string   `json:"virtualization"`
-	ConfiguredVersion        string   `json:"configured version"`
-	PackageVersion           string   `json:"package version"`
-	SolutionEnabled          []string `json:"Solution enabled"`
-	NotesEnabledBySolution   []string `json:"Notes enabled by Solution"`
-	SolutionApplied          []string `json:"Solution applied"`
-	NotesAppliedBySolution   []string `json:"Notes applied by Solution"`
-	NotesEnabledAdditionally []string `json:"Notes enabled additionally"`
-	NotesEnabled             []string `json:"Notes enabled"`
-	NotesApplied             []string `json:"Notes applied"`
-	Staging                  Staging  `json:"staging"`
-	RememberMessage          string   `json:"remember message"`
-}
-
-type Services struct {
-	Saptune []string `json:"saptune"`
-	Sapconf []string `json:"sapconf"`
-	Tuned   []string `json:"tuned"`
-}
-
-type Staging struct {
-	StagingEnabled  bool     `json:"staging enabled"`
-	NotesStaged     []string `json:"Notes staged"`
-	SolutionsStaged []string `json:"Solutions staged"`
-}
-
-type Message struct {
-	Priority string `json:"priority"`
-	Message  string `json:"message"`
+	Version  string `json:"version"`
+	executor utils.CommandExecutor
 }
 
 func getSaptuneVersion(commandExecutor utils.CommandExecutor) (string, error) {
@@ -81,7 +34,6 @@ func getSaptuneVersion(commandExecutor utils.CommandExecutor) (string, error) {
 }
 
 func isSaptuneVersionSupported(version string) bool {
-
 	compareOutput := semver.Compare(MinimalSaptuneVersion, "v"+version)
 
 	return compareOutput != 1
@@ -90,31 +42,30 @@ func isSaptuneVersionSupported(version string) bool {
 func NewSaptune(commandExecutor utils.CommandExecutor) (Saptune, error) {
 	saptuneVersion, err := getSaptuneVersion(commandExecutor)
 	if err != nil {
-		return Saptune{Version: nil, Commands: []Output{}}, err
+		return Saptune{Version: ""}, err
 	}
 
-	if !isSaptuneVersionSupported(saptuneVersion) {
-		return Saptune{Version: &saptuneVersion, Commands: []Output{}}, ErrUnsupportedSaptuneVer
+	return Saptune{Version: saptuneVersion, executor: commandExecutor}, nil
+}
+
+func (s *Saptune) RunCommand(args ...string) ([]byte, error) {
+	if !isSaptuneVersionSupported(s.Version) {
+		return nil, ErrUnsupportedSaptuneVer
 	}
 
 	log.Info("Requesting Saptune status...")
-	output, err := commandExecutor.Exec("saptune", "--format", "json", "status")
+
+	log.Infof("Saptune status discovered")
+	log.Infof("Running saptune command: saptune %v", args)
+	output, err := s.executor.Exec("saptune", args...)
+	log.Debugf("saptune output: %s", string(output))
 	if err != nil {
-		return Saptune{Version: &saptuneVersion, Commands: []Output{}},
-			errors.Wrap(err, "unexpected error while calling saptune")
+		return nil, errors.Wrap(err, "unexpected error while calling saptune")
 	}
 
 	log.Debugf("saptune output: %s", string(output))
 
-	var saptuneOutput Output
-	err = json.Unmarshal(output, &saptuneOutput)
-	if err != nil {
-		return Saptune{Version: &saptuneVersion, Commands: []Output{}},
-			errors.Wrap(err, "unexpected error while parsing saptune output")
-	}
-	log.Infof("Saptune status discovered")
+	log.Infof("Saptune command executed")
 
-	return Saptune{Version: &saptuneVersion, Commands: []Output{
-		saptuneOutput,
-	}}, nil
+	return output, nil
 }

internal/core/saptune/saptune_test.go

@@ -23,62 +23,21 @@ func (suite *SaptuneTestSuite) TestNewSaptune() {
 		[]byte("3.1.0"), nil,
 	)
 
-	saptuneOutput := []byte(`{"$schema":"file:///usr/share/saptune/schemas/1.0/saptune_status.schema.json","publish time":"2023-09-12 16:28:11.718","argv":"saptune --format json status","pid":6675,"command":"status","exit code":1,"result":{"services":{"saptune":["disabled","inactive"],"sapconf":[],"tuned":[]},"systemd system state":"degraded","tuning state":"compliant","virtualization":"kvm","configured version":"3","package version":"3.1.0","Solution enabled":[],"Notes enabled by Solution":[],"Solution applied":[],"Notes applied by Solution":[],"Notes enabled additionally":["1410736"],"Notes enabled":["1410736"],"Notes applied":["1410736"],"staging":{"staging enabled":false,"Notes staged":[],"Solutions staged":[]},"remember message":"\nRemember: if you wish to automatically activate the note's and solution's tuning options after a reboot, you must enable and start saptune.service by running:\n 'saptune service enablestart'.\nThe systemd system state is NOT ok.\nPlease call 'saptune check' to get guidance to resolve the issues!\n\n"},"messages":[{"priority":"NOTICE","message":"actions.go:85: ATTENTION: You are running a test version (3.1.0 from 2023/08/03) of saptune which is not supported for production use\n"}]}`)
+	saptuneOutput := []byte("{\"some_json_key\": \"some_value\"}")
 
 	mockCommand.On("Exec", "saptune", "--format", "json", "status").Return(
 		saptuneOutput, nil,
 	)
 
-	expectedVersion := "3.1.0"
-	saptuneDetails, err := NewSaptune(mockCommand)
+	saptuneRetriever, err := NewSaptune(mockCommand)
+	statusOutput, err := saptuneRetriever.RunCommand("--format", "json", "status")
 
-	expectedDetails := Saptune{
-		Version: &expectedVersion,
-		Commands: []Output{
-			{
-				Schema:      "file:///usr/share/saptune/schemas/1.0/saptune_status.schema.json",
-				PublishTime: "2023-09-12 16:28:11.718",
-				Argv:        "saptune --format json status",
-				Pid:         6675,
-				Command:     "status",
-				ExitCode:    1,
-				Result: Result{
-					Services: Services{
-						Saptune: []string{"disabled", "inactive"},
-						Sapconf: []string{},
-						Tuned:   []string{},
-					},
-					SystemdSystemState:       "degraded",
-					TuningState:              "compliant",
-					Virtualization:           "kvm",
-					ConfiguredVersion:        "3",
-					PackageVersion:           "3.1.0",
-					SolutionEnabled:          []string{},
-					NotesEnabledBySolution:   []string{},
-					SolutionApplied:          []string{},
-					NotesAppliedBySolution:   []string{},
-					NotesEnabledAdditionally: []string{"1410736"},
-					NotesEnabled:             []string{"1410736"},
-					NotesApplied:             []string{"1410736"},
-					Staging: Staging{
-						StagingEnabled:  false,
-						NotesStaged:     []string{},
-						SolutionsStaged: []string{},
-					},
-					RememberMessage: "\nRemember: if you wish to automatically activate the note's and solution's tuning options after a reboot, you must enable and start saptune.service by running:\n 'saptune service enablestart'.\nThe systemd system state is NOT ok.\nPlease call 'saptune check' to get guidance to resolve the issues!\n\n",
-				},
-				Messages: []Message{
-					{
-						Priority: "NOTICE",
-						Message:  "actions.go:85: ATTENTION: You are running a test version (3.1.0 from 2023/08/03) of saptune which is not supported for production use\n",
-					},
-				},
-			},
-		},
-	}
+	expectedVersion := "3.1.0"
+	expectedOutput := []byte("{\"some_json_key\": \"some_value\"}")
 
 	suite.NoError(err)
-	suite.Equal(expectedDetails, saptuneDetails)
+	suite.Equal(expectedOutput, statusOutput)
+	suite.Equal(expectedVersion, *saptuneRetriever.Version)
 }
 
 func (suite *SaptuneTestSuite) TestNewSaptuneSaptuneVersionUnknownErr() {
@@ -88,14 +47,14 @@ func (suite *SaptuneTestSuite) TestNewSaptuneSaptuneVersionUnknownErr() {
 		nil, errors.New("Error: exec: \"rpm\": executable file not found in $PATH"),
 	)
 
-	saptuneDetails, err := NewSaptune(mockCommand)
+	saptuneRetriever, err := NewSaptune(mockCommand)
 
 	expectedDetails := Saptune{
-		Version:  nil,
-		Commands: []Output{},
+		Version: nil,
 	}
+
 	suite.EqualError(err, ErrSaptuneVersionUnknown.Error())
-	suite.Equal(expectedDetails, saptuneDetails)
+	suite.Equal(expectedDetails, saptuneRetriever)
 }
 
 func (suite *SaptuneTestSuite) TestNewSaptuneUnsupportedSaptuneVerErr() {
@@ -105,13 +64,16 @@ func (suite *SaptuneTestSuite) TestNewSaptuneUnsupportedSaptuneVerErr() {
 		[]byte("3.0.0"), nil,
 	)
 
-	saptuneDetails, err := NewSaptune(mockCommand)
+	saptuneRetriever, _ := NewSaptune(mockCommand)
+	statusOutput, err := saptuneRetriever.RunCommand("--format", "json", "status")
 
 	expectedVersion := "3.0.0"
 	expectedDetails := Saptune{
 		Version:  &expectedVersion,
-		Commands: []Output{},
+		executor: mockCommand,
 	}
+
 	suite.EqualError(err, ErrUnsupportedSaptuneVer.Error())
-	suite.Equal(expectedDetails, saptuneDetails)
+	suite.Equal(expectedDetails, saptuneRetriever)
+	suite.Equal([]byte(nil), statusOutput)
 }

internal/discovery/saptune.go

@@ -15,14 +15,18 @@ const SaptuneDiscoveryMinPeriod time.Duration = 1 * time.Second
 type SaptuneDiscovery struct {
 	id              string
 	collectorClient collector.Client
-	host            string
 	interval        time.Duration
 }
 
-func NewSaptuneDiscovery(collectorClient collector.Client, hostname string, config DiscoveriesConfig) Discovery {
+type SaptuneDiscoveryPayload struct {
+	PackageVersion   string `json:"package_version"`
+	SaptuneInstalled bool   `json:"saptune_installed"`
+	Status           string `json:"status"`
+}
+
+func NewSaptuneDiscovery(collectorClient collector.Client, config DiscoveriesConfig) Discovery {
 	return SaptuneDiscovery{
 		id:              SaptuneDiscoveryID,
-		host:            hostname,
 		collectorClient: collectorClient,
 		interval:        config.DiscoveriesPeriodsConfig.Saptune,
 	}
@@ -37,12 +41,20 @@ func (d SaptuneDiscovery) GetInterval() time.Duration {
 }
 
 func (d SaptuneDiscovery) Discover() (string, error) {
-	saptuneData, err := saptune.NewSaptune(utils.Executor{})
+	saptuneRetriever, err := saptune.NewSaptune(utils.Executor{})
 	if err != nil {
 		return "", err
 	}
 
-	err = d.collectorClient.Publish(d.id, saptuneData)
+	saptuneData, err := saptuneRetriever.RunCommand("--format", "json", "status")
+
+	var saptunePayload = SaptuneDiscoveryPayload{
+		PackageVersion:   saptuneRetriever.Version,
+		SaptuneInstalled: saptuneRetriever.Version != "",
+		Status:           string(saptuneData),
+	}
+
+	err = d.collectorClient.Publish(d.id, saptunePayload)
 	if err != nil {
 		log.Debugf("Error while sending saptune discovery to data collector: %s", err)
 		return "", err