From 56e8c47cf1ae44f75b8ff54085f4ebdf83c1624a Mon Sep 17 00:00:00 2001 From: balanza Date: Wed, 22 Jan 2025 21:14:16 +0100 Subject: [PATCH 01/13] ascsers --- .../factsengine/gatherers/ascsers_cluster.go | 2 +- .../gatherers/ascsers_cluster_test.go | 32 ++++++++++++++++--- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/internal/factsengine/gatherers/ascsers_cluster.go b/internal/factsengine/gatherers/ascsers_cluster.go index 1964d26c..1be81707 100644 --- a/internal/factsengine/gatherers/ascsers_cluster.go +++ b/internal/factsengine/gatherers/ascsers_cluster.go @@ -86,7 +86,7 @@ func (g *AscsErsClusterGatherer) Gather( content, err := factscache.GetOrUpdate( g.cache, CibAdminGathererCache, - memoizeCibAdmin, + makeMemoizeCibAdmin(ctx), g.executor, ) diff --git a/internal/factsengine/gatherers/ascsers_cluster_test.go b/internal/factsengine/gatherers/ascsers_cluster_test.go index 25963cf8..58f5c219 100644 --- a/internal/factsengine/gatherers/ascsers_cluster_test.go +++ b/internal/factsengine/gatherers/ascsers_cluster_test.go @@ -8,6 +8,7 @@ import ( "os" "testing" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" sapcontrol "github.com/trento-project/agent/internal/core/sapsystem/sapcontrolapi" sapControlMocks "github.com/trento-project/agent/internal/core/sapsystem/sapcontrolapi/mocks" @@ -36,7 +37,7 @@ func (suite *AscsErsClusterTestSuite) SetupTest() { } func (suite *AscsErsClusterTestSuite) TestAscsErsClusterGatherCmdNotFound() { - suite.mockExecutor.On("Exec", "cibadmin", "--query", "--local").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "cibadmin", "--query", "--local").Return( []byte{}, errors.New("cibadmin not found")) p := gatherers.NewAscsErsClusterGatherer(suite.mockExecutor, suite.webService, nil) @@ -84,7 +85,7 @@ func (suite *AscsErsClusterTestSuite) TestAscsErsClusterGatherInvalidInstanceNam lFile, _ := os.Open(helpers.GetFixturePath("gatherers/cibadmin_multisid_invalid.xml")) content, _ := io.ReadAll(lFile) - suite.mockExecutor.On("Exec", "cibadmin", "--query", "--local").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "cibadmin", "--query", "--local").Return( content, nil) p := gatherers.NewAscsErsClusterGatherer(suite.mockExecutor, suite.webService, nil) @@ -109,7 +110,7 @@ func (suite *AscsErsClusterTestSuite) TestAscsErsClusterGatherInvalidInstanceNum helpers.GetFixturePath("gatherers/cibadmin_multisid_invalid_instance_number.xml")) content, _ := io.ReadAll(lFile) - suite.mockExecutor.On("Exec", "cibadmin", "--query", "--local").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "cibadmin", "--query", "--local").Return( content, nil) p := gatherers.NewAscsErsClusterGatherer(suite.mockExecutor, suite.webService, nil) @@ -135,7 +136,7 @@ func (suite *AscsErsClusterTestSuite) TestAscsErsClusterGather() { lFile, _ := os.Open(helpers.GetFixturePath("gatherers/cibadmin_multisid.xml")) content, _ := io.ReadAll(lFile) - suite.mockExecutor.On("Exec", "cibadmin", "--query", "--local").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "cibadmin", "--query", "--local").Return( content, nil) mockWebServicePRDASCS00 := new(sapControlMocks.WebService) @@ -286,3 +287,26 @@ func (suite *AscsErsClusterTestSuite) TestAscsErsClusterGather() { } suite.ElementsMatch(expectedEntries, entries) } + +func (suite *AscsErsClusterTestSuite) TestAscsErskGathererContextCancelled() { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + suite.mockExecutor. + On("ExecContext", mock.Anything, "cibadmin", "--query", "--local"). + Return(nil, ctx.Err()) + + c := gatherers.NewAscsErsClusterGatherer(suite.mockExecutor, suite.webService, nil) + factRequests := []entities.FactRequest{ + { + Name: "ascsers", + Gatherer: "ascsers_cluster", + Argument: "", + CheckID: "check1", + }, + } + factResults, err := c.Gather(ctx, factRequests) + + suite.Error(err) + suite.Empty(factResults) +} From b90948e1df3136e96567b5919d5f170053792c1d Mon Sep 17 00:00:00 2001 From: balanza Date: Wed, 22 Jan 2025 21:14:26 +0100 Subject: [PATCH 02/13] cibadmin --- internal/factsengine/gatherers/cibadmin.go | 17 ++++++---- .../factsengine/gatherers/cibadmin_test.go | 34 ++++++++++++++++--- 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/internal/factsengine/gatherers/cibadmin.go b/internal/factsengine/gatherers/cibadmin.go index cb871d8a..cca0dfdd 100644 --- a/internal/factsengine/gatherers/cibadmin.go +++ b/internal/factsengine/gatherers/cibadmin.go @@ -47,18 +47,21 @@ func (g *CibAdminGatherer) SetCache(cache *factscache.FactsCache) { g.cache = cache } -func memoizeCibAdmin(args ...interface{}) (interface{}, error) { - executor, ok := args[0].(utils.CommandExecutor) - if !ok { - return nil, ImplementationError.Wrap("error using memoizeCibAdmin. executor must be 1st argument") +func makeMemoizeCibAdmin(ctx context.Context) func(...interface{}) (interface{}, error) { + return func(args ...interface{}) (interface{}, error) { + executor, ok := args[0].(utils.CommandExecutor) + if !ok { + return nil, ImplementationError.Wrap("error using memoizeCibAdmin. executor must be 1st argument") + } + return executor.ExecContext(ctx, "cibadmin", "--query", "--local") } - return executor.Exec("cibadmin", "--query", "--local") + } -func (g *CibAdminGatherer) Gather(_ context.Context, factsRequests []entities.FactRequest) ([]entities.Fact, error) { +func (g *CibAdminGatherer) Gather(ctx context.Context, factsRequests []entities.FactRequest) ([]entities.Fact, error) { log.Infof("Starting %s facts gathering process", CibAdminGathererName) - content, err := factscache.GetOrUpdate(g.cache, CibAdminGathererCache, memoizeCibAdmin, g.executor) + content, err := factscache.GetOrUpdate(g.cache, CibAdminGathererCache, makeMemoizeCibAdmin(ctx), g.executor) if err != nil { return nil, CibAdminCommandError.Wrap(err.Error()) diff --git a/internal/factsengine/gatherers/cibadmin_test.go b/internal/factsengine/gatherers/cibadmin_test.go index 6b934e2c..3b0d49fc 100644 --- a/internal/factsengine/gatherers/cibadmin_test.go +++ b/internal/factsengine/gatherers/cibadmin_test.go @@ -7,6 +7,7 @@ import ( "os" "testing" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" "github.com/trento-project/agent/internal/factsengine/factscache" "github.com/trento-project/agent/internal/factsengine/gatherers" @@ -37,7 +38,7 @@ func (suite *CibAdminTestSuite) SetupTest() { } func (suite *CibAdminTestSuite) TestCibAdminGatherCmdNotFound() { - suite.mockExecutor.On("Exec", "cibadmin", "--query", "--local").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "cibadmin", "--query", "--local").Return( suite.cibAdminOutput, errors.New("cibadmin not found")) p := gatherers.NewCibAdminGatherer(suite.mockExecutor, nil) @@ -58,7 +59,7 @@ func (suite *CibAdminTestSuite) TestCibAdminGatherCmdNotFound() { } func (suite *CibAdminTestSuite) TestCibAdminInvalidXML() { - suite.mockExecutor.On("Exec", "cibadmin", "--query", "--local").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "cibadmin", "--query", "--local").Return( []byte("invalid"), nil) p := gatherers.NewCibAdminGatherer(suite.mockExecutor, nil) @@ -79,7 +80,7 @@ func (suite *CibAdminTestSuite) TestCibAdminInvalidXML() { } func (suite *CibAdminTestSuite) TestCibAdminGather() { - suite.mockExecutor.On("Exec", "cibadmin", "--query", "--local").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "cibadmin", "--query", "--local").Return( suite.cibAdminOutput, nil) p := gatherers.NewCibAdminGatherer(suite.mockExecutor, nil) @@ -210,7 +211,7 @@ func (suite *CibAdminTestSuite) TestCibAdminGather() { } func (suite *CibAdminTestSuite) TestCibAdminGatherWithCache() { - suite.mockExecutor.On("Exec", "cibadmin", "--query", "--local"). + suite.mockExecutor.On("ExecContext", mock.Anything, "cibadmin", "--query", "--local"). Return(suite.cibAdminOutput, nil). Once() @@ -269,3 +270,28 @@ func (suite *CibAdminTestSuite) TestCibAdminGatherCacheCastingError() { suite.EqualError(err, "fact gathering error: cibadmin-decoding-error - "+ "error decoding cibadmin output: error casting the command output") } + +func (suite *CibAdminTestSuite) TestCibAdminGatherWithContextCancelled() { + + // Create a cancelled context + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + suite.mockExecutor.On("ExecContext", mock.Anything, "cibadmin", "--query", "--local"). + Return(nil, ctx.Err()) + + p := gatherers.NewCibAdminGatherer(suite.mockExecutor, nil) + factRequests := []entities.FactRequest{ + { + Name: "cib", + Gatherer: "cibadmin", + Argument: "cib", + CheckID: "check1", + }, + } + + factResults, err := p.Gather(ctx, factRequests) + + suite.Error(err) + suite.Empty(factResults) +} From 34cf342194f16980df54efa55a31ab18085f8265 Mon Sep 17 00:00:00 2001 From: balanza Date: Wed, 22 Jan 2025 21:14:34 +0100 Subject: [PATCH 03/13] corosync --- .../gatherers/corosynccmapctl_test.go | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/internal/factsengine/gatherers/corosynccmapctl_test.go b/internal/factsengine/gatherers/corosynccmapctl_test.go index ec90a3c2..92c8b89e 100644 --- a/internal/factsengine/gatherers/corosynccmapctl_test.go +++ b/internal/factsengine/gatherers/corosynccmapctl_test.go @@ -209,3 +209,25 @@ func (suite *CorosyncCmapctlTestSuite) TestCorosyncCmapctlGatherer() { suite.NoError(err) suite.ElementsMatch(expectedResults, factResults) } + +func (suite *CorosyncCmapctlTestSuite) TestCorosyncCmapctlGathererContextCancelled() { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + suite.mockExecutor. + On("ExecContext", mock.Anything, "corosync-cmapctl", "-b"). + Return(nil, ctx.Err()) + + c := gatherers.NewCorosyncCmapctlGatherer(suite.mockExecutor) + factRequests := []entities.FactRequest{ + { + Name: "madeup_fact", + Gatherer: "corosync-cmapctl", + Argument: "madeup.fact", + }, + } + factResults, err := c.Gather(ctx, factRequests) + + suite.Error(err) + suite.Empty(factResults) +} From 551061596e95f7cf4e8b7c004ecca7dfbd25ca12 Mon Sep 17 00:00:00 2001 From: balanza Date: Wed, 22 Jan 2025 21:14:44 +0100 Subject: [PATCH 04/13] dispwork --- internal/factsengine/gatherers/dispwork.go | 9 ++++-- .../factsengine/gatherers/dispwork_test.go | 31 ++++++++++++++++--- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/internal/factsengine/gatherers/dispwork.go b/internal/factsengine/gatherers/dispwork.go index 97b93851..714f423e 100644 --- a/internal/factsengine/gatherers/dispwork.go +++ b/internal/factsengine/gatherers/dispwork.go @@ -67,7 +67,7 @@ func NewDispWorkGatherer(fs afero.Fs, executor utils.CommandExecutor) *DispWorkG } } -func (g *DispWorkGatherer) Gather(_ context.Context, factsRequests []entities.FactRequest) ([]entities.Fact, error) { +func (g *DispWorkGatherer) Gather(ctx context.Context, factsRequests []entities.FactRequest) ([]entities.Fact, error) { facts := []entities.Fact{} log.Infof("Starting %s facts gathering process", DispWorkGathererName) @@ -82,8 +82,11 @@ func (g *DispWorkGatherer) Gather(_ context.Context, factsRequests []entities.Fa sid := filepath.Base(systemPath) sapUser := fmt.Sprintf("%sadm", strings.ToLower(sid)) - dispWorkOutput, err := g.executor.Exec("su", "-", sapUser, "-c", "\"disp+work\"") - if err != nil { + dispWorkOutput, err := g.executor.ExecContext(ctx, "su", "-", sapUser, "-c", "\"disp+work\"") + switch { + case ctx.Err() != nil: + return nil, ctx.Err() + case err != nil: gatheringError := DispWorkCommandError.Wrap(err.Error()) log.Error(gatheringError) dispWorkMap[sid] = dispWorkData{} // fill with empty data diff --git a/internal/factsengine/gatherers/dispwork_test.go b/internal/factsengine/gatherers/dispwork_test.go index fe8110a8..0b9624be 100644 --- a/internal/factsengine/gatherers/dispwork_test.go +++ b/internal/factsengine/gatherers/dispwork_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/spf13/afero" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" "github.com/trento-project/agent/internal/factsengine/gatherers" "github.com/trento-project/agent/pkg/factsengine/entities" @@ -48,13 +49,13 @@ func (suite *DispWorkGathererTestSuite) TestDispWorkGatheringSuccess() { unsortedOutputFile, _ := os.Open(helpers.GetFixturePath("gatherers/dispwork-unsorted.output")) unsortedOutput, _ := io.ReadAll(unsortedOutputFile) suite.mockExecutor. - On("Exec", "su", "-", "prdadm", "-c", "\"disp+work\""). + On("ExecContext", mock.Anything, "su", "-", "prdadm", "-c", "\"disp+work\""). Return(validOutput, nil). - On("Exec", "su", "-", "qasadm", "-c", "\"disp+work\""). + On("ExecContext", mock.Anything, "su", "-", "qasadm", "-c", "\"disp+work\""). Return(partialOutput, nil). - On("Exec", "su", "-", "qa2adm", "-c", "\"disp+work\""). + On("ExecContext", mock.Anything, "su", "-", "qa2adm", "-c", "\"disp+work\""). Return(unsortedOutput, nil). - On("Exec", "su", "-", "devadm", "-c", "\"disp+work\""). + On("ExecContext", mock.Anything, "su", "-", "devadm", "-c", "\"disp+work\""). Return(nil, errors.New("some error")) g := gatherers.NewDispWorkGatherer(suite.fs, suite.mockExecutor) @@ -132,3 +133,25 @@ func (suite *DispWorkGathererTestSuite) TestDispWorkGatheringEmptyFileSystem() { suite.NoError(err) suite.EqualValues(expectedResults, result) } + +func (suite *DispWorkGathererTestSuite) TestDispWorkGathererContextCancelled() { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + suite.mockExecutor. + On("ExecContext", mock.Anything, "su", "-", mock.AnythingOfType("string"), "-c", "\"disp+work\""). + Return(nil, ctx.Err()) + + c := gatherers.NewDispWorkGatherer(suite.fs, suite.mockExecutor) + factRequests := []entities.FactRequest{ + { + Name: "dispwork", + CheckID: "check1", + Gatherer: "disp+work", + }, + } + factResults, err := c.Gather(ctx, factRequests) + + suite.Error(err) + suite.Empty(factResults) +} From 9e976d65da82407d1c93e4fa1aa0e1fe3efaf95b Mon Sep 17 00:00:00 2001 From: balanza Date: Wed, 22 Jan 2025 21:14:57 +0100 Subject: [PATCH 05/13] mountinfo --- internal/factsengine/gatherers/mountinfo.go | 5 ++-- .../factsengine/gatherers/mountinfo_test.go | 28 +++++++++++++++++-- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/internal/factsengine/gatherers/mountinfo.go b/internal/factsengine/gatherers/mountinfo.go index 6d319e24..51545f1b 100644 --- a/internal/factsengine/gatherers/mountinfo.go +++ b/internal/factsengine/gatherers/mountinfo.go @@ -66,7 +66,7 @@ func NewMountInfoGatherer(mInfo MountParserInterface, executor utils.CommandExec return &MountInfoGatherer{mInfo: mInfo, executor: executor} } -func (g *MountInfoGatherer) Gather(_ context.Context, factsRequests []entities.FactRequest) ([]entities.Fact, error) { +func (g *MountInfoGatherer) Gather(ctx context.Context, factsRequests []entities.FactRequest) ([]entities.Fact, error) { facts := []entities.Fact{} log.Infof("Starting %s facts gathering process", MountInfoGathererName) mounts, err := g.mInfo.GetMounts(nil) @@ -92,7 +92,8 @@ func (g *MountInfoGatherer) Gather(_ context.Context, factsRequests []entities.F Options: mount.Options, } - if blkidOuptut, err := g.executor.Exec("blkid", foundMountInfoResult.Source, "-o", "export"); err != nil { + blkidOuptut, err := g.executor.ExecContext(ctx, "blkid", foundMountInfoResult.Source, "-o", "export") + if err != nil { log.Warnf("blkid command failed for source %s: %s", foundMountInfoResult.Source, err) } else if fields, err := envparse.Parse(strings.NewReader(string(blkidOuptut))); err != nil { log.Warnf("error parsing the blkid output: %s", err) diff --git a/internal/factsengine/gatherers/mountinfo_test.go b/internal/factsengine/gatherers/mountinfo_test.go index dea271e3..23b6ff28 100644 --- a/internal/factsengine/gatherers/mountinfo_test.go +++ b/internal/factsengine/gatherers/mountinfo_test.go @@ -66,11 +66,11 @@ TYPE=xfs `) suite.mockExecutor. - On("Exec", "blkid", "10.1.1.10:/sapmnt", "-o", "export"). + On("ExecContext", mock.Anything, "blkid", "10.1.1.10:/sapmnt", "-o", "export"). Return(nil, fmt.Errorf("blkid error")). - On("Exec", "blkid", "/dev/mapper/vg_hana-lv_data", "-o", "export"). + On("ExecContext", mock.Anything, "blkid", "/dev/mapper/vg_hana-lv_data", "-o", "export"). Return(blkidOutput, nil). - On("Exec", "blkid", "/dev/mapper/vg_hana-lv_log", "-o", "export"). + On("ExecContext", mock.Anything, "blkid", "/dev/mapper/vg_hana-lv_log", "-o", "export"). Return(blkidOutputNoUUID, nil) requestedFacts := []entities.FactRequest{ @@ -211,3 +211,25 @@ func (suite *MountInfoTestSuite) TestMountInfoParsingError() { suite.EqualError(err, "fact gathering error: mount-info-parsing-error - "+ "error parsing mount information: some error") } + +func (suite *MountInfoTestSuite) TestMountInfoParsingGathererContextCancelled() { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + suite.mockMountParser. + On("ExecContext", mock.Anything, "blkid", "10.1.1.10:/sapmnt", "-o", "export"). + Return(nil, ctx.Err()) + + c := gatherers.NewSapHostCtrlGatherer(suite.mockExecutor) + factRequests := []entities.FactRequest{ + {Name: "shared", + Gatherer: "mount_info", + CheckID: "check1", + Argument: "/sapmnt", + }, + } + factResults, err := c.Gather(ctx, factRequests) + + suite.Error(err) + suite.Empty(factResults) +} From e070438f23677d7012ece55cb1a7504ed5cf2c7c Mon Sep 17 00:00:00 2001 From: balanza Date: Wed, 22 Jan 2025 21:15:07 +0100 Subject: [PATCH 06/13] packageversion --- .../factsengine/gatherers/packageversion.go | 14 +++-- .../gatherers/packageversion_test.go | 52 ++++++++++++++----- 2 files changed, 47 insertions(+), 19 deletions(-) diff --git a/internal/factsengine/gatherers/packageversion.go b/internal/factsengine/gatherers/packageversion.go index 4d7397e1..1b946a8b 100644 --- a/internal/factsengine/gatherers/packageversion.go +++ b/internal/factsengine/gatherers/packageversion.go @@ -58,7 +58,7 @@ func NewPackageVersionGatherer(executor utils.CommandExecutor) *PackageVersionGa } func (g *PackageVersionGatherer) Gather( - _ context.Context, + ctx context.Context, factsRequests []entities.FactRequest, ) ([]entities.Fact, error) { facts := []entities.Fact{} @@ -81,7 +81,7 @@ func (g *PackageVersionGatherer) Gather( requestedVersion = arguments[1] } - installedVersions, err := executeRpmVersionRetrieveCommand(g.executor, packageName) + installedVersions, err := executeRpmVersionRetrieveCommand(ctx, g.executor, packageName) if err != nil { fact = entities.NewFactGatheredWithError(factReq, err) facts = append(facts, fact) @@ -89,7 +89,8 @@ func (g *PackageVersionGatherer) Gather( } if requestedVersion != "" { - comparisonResult, err := executeZypperVersionCmpCommand(g.executor, installedVersions[0].Version, requestedVersion) + comparisonResult, err := executeZypperVersionCmpCommand(ctx, g.executor, + installedVersions[0].Version, requestedVersion) if err != nil { fact = entities.NewFactGatheredWithError(factReq, err) facts = append(facts, fact) @@ -109,11 +110,13 @@ func (g *PackageVersionGatherer) Gather( } func executeZypperVersionCmpCommand( + ctx context.Context, executor utils.CommandExecutor, installedVersion string, comparedVersion string, ) (int, *entities.FactGatheringError) { - zypperOutput, err := executor.Exec("/usr/bin/zypper", "--terse", "versioncmp", comparedVersion, installedVersion) + zypperOutput, err := executor.ExecContext(ctx, + "/usr/bin/zypper", "--terse", "versioncmp", comparedVersion, installedVersion) if err != nil { gatheringError := PackageVersionZypperCommandError.Wrap(err.Error()) log.Error(gatheringError) @@ -135,10 +138,11 @@ func executeZypperVersionCmpCommand( } func executeRpmVersionRetrieveCommand( + ctx context.Context, executor utils.CommandExecutor, packageName string, ) ([]packageVersion, *entities.FactGatheringError) { - rpmOutputBytes, err := executor.Exec("/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, packageName) + rpmOutputBytes, err := executor.ExecContext(ctx, "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, packageName) rpmOutput := string(rpmOutputBytes) diff --git a/internal/factsengine/gatherers/packageversion_test.go b/internal/factsengine/gatherers/packageversion_test.go index 967e9b0f..d9482484 100644 --- a/internal/factsengine/gatherers/packageversion_test.go +++ b/internal/factsengine/gatherers/packageversion_test.go @@ -7,6 +7,7 @@ import ( "os" "testing" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" "github.com/trento-project/agent/internal/factsengine/gatherers" "github.com/trento-project/agent/pkg/factsengine/entities" @@ -30,7 +31,7 @@ func (suite *PackageVersionTestSuite) SetupTest() { } func (suite *PackageVersionTestSuite) TestPackageVersionGathererNoArgumentProvided() { - suite.mockExecutor.On("Exec", "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, "").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, "").Return( []byte("rpm: no arguments given for query"), nil) p := gatherers.NewPackageVersionGatherer(suite.mockExecutor) @@ -79,37 +80,37 @@ func (suite *PackageVersionTestSuite) TestPackageVersionGathererNoArgumentProvid func (suite *PackageVersionTestSuite) TestPackageVersionGather() { corosyncMockOutputFile, _ := os.Open(helpers.GetFixturePath("gatherers/rpm-query.corosync.output")) corosyncVersionMockOutput, _ := io.ReadAll(corosyncMockOutputFile) - suite.mockExecutor.On("Exec", "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, "corosync"). + suite.mockExecutor.On("ExecContext", mock.Anything, "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, "corosync"). Return(corosyncVersionMockOutput, nil) pacemakerMockOutputFile, _ := os.Open(helpers.GetFixturePath("gatherers/rpm-query.pacemaker.output")) pacemakerVersionMockOutput, _ := io.ReadAll(pacemakerMockOutputFile) - suite.mockExecutor.On("Exec", "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, "pacemaker"). + suite.mockExecutor.On("ExecContext", mock.Anything, "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, "pacemaker"). Return(pacemakerVersionMockOutput, nil) multiversionsMockOutputFile, _ := os.Open(helpers.GetFixturePath("gatherers/rpm-query-multi-versions.variant-1.output")) multiversionsVersionMockOutput, _ := io.ReadAll(multiversionsMockOutputFile) - suite.mockExecutor.On("Exec", "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, "sbd"). + suite.mockExecutor.On("ExecContext", mock.Anything, "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, "sbd"). Return(multiversionsVersionMockOutput, nil) multiversionsVariantMockOutputFile, _ := os.Open(helpers.GetFixturePath("gatherers/rpm-query-multi-versions.variant-2.output")) multiversionsVariantVersionMockOutput, _ := io.ReadAll(multiversionsVariantMockOutputFile) - suite.mockExecutor.On("Exec", "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, "awk"). + suite.mockExecutor.On("ExecContext", mock.Anything, "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, "awk"). Return(multiversionsVariantVersionMockOutput, nil) - suite.mockExecutor.On("Exec", "/usr/bin/zypper", "--terse", "versioncmp", "2.4.4", "2.4.5").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "/usr/bin/zypper", "--terse", "versioncmp", "2.4.4", "2.4.5").Return( []byte("-1\n"), nil) - suite.mockExecutor.On("Exec", "/usr/bin/zypper", "--terse", "versioncmp", "2.4.5", "2.4.5").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "/usr/bin/zypper", "--terse", "versioncmp", "2.4.5", "2.4.5").Return( []byte("0\n"), nil) - suite.mockExecutor.On("Exec", "/usr/bin/zypper", "--terse", "versioncmp", "2.4.6", "2.4.5").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "/usr/bin/zypper", "--terse", "versioncmp", "2.4.6", "2.4.5").Return( []byte("1\n"), nil) versionComparisonOutputWithWarningFile, _ := os.Open(helpers.GetFixturePath("gatherers/versioncmp-with-warning.output")) versionComparisonOutputWithWarning, _ := io.ReadAll(versionComparisonOutputWithWarningFile) - suite.mockExecutor.On("Exec", "/usr/bin/zypper", "--terse", "versioncmp", "1.5.2", "1.5.2").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "/usr/bin/zypper", "--terse", "versioncmp", "1.5.2", "1.5.2").Return( versionComparisonOutputWithWarning, nil) p := gatherers.NewPackageVersionGatherer(suite.mockExecutor) @@ -257,22 +258,22 @@ func (suite *PackageVersionTestSuite) TestPackageVersionGather() { } func (suite *PackageVersionTestSuite) TestPackageVersionGatherErrors() { - suite.mockExecutor.On("Exec", "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, "sbd"). + suite.mockExecutor.On("ExecContext", mock.Anything, "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, "sbd"). Return([]byte("package sbd is not installed"), errors.New("")) - suite.mockExecutor.On("Exec", "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, "pacemake"). + suite.mockExecutor.On("ExecContext", mock.Anything, "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, "pacemake"). Return([]byte("package pacemake is not installed"), errors.New("")) mockOutputFile, _ := os.Open(helpers.GetFixturePath("gatherers/rpm-query.corosync.output")) mockOutput, _ := io.ReadAll(mockOutputFile) - suite.mockExecutor.On("Exec", "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, "corosync"). + suite.mockExecutor.On("ExecContext", mock.Anything, "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, "corosync"). Return(mockOutput, nil) invalidDateMockOutputFile, _ := os.Open(helpers.GetFixturePath("gatherers/rpm-query-invalid-date.output")) invalidDateMockOutput, _ := io.ReadAll(invalidDateMockOutputFile) - suite.mockExecutor.On("Exec", "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, "another_package"). + suite.mockExecutor.On("ExecContext", mock.Anything, "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, "another_package"). Return(invalidDateMockOutput, nil) - suite.mockExecutor.On("Exec", "/usr/bin/zypper", "--terse", "versioncmp", "1.2.3", "2.4.5").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "/usr/bin/zypper", "--terse", "versioncmp", "1.2.3", "2.4.5").Return( []byte(""), errors.New("zypper: command not found")) p := gatherers.NewPackageVersionGatherer(suite.mockExecutor) @@ -348,3 +349,26 @@ func (suite *PackageVersionTestSuite) TestPackageVersionGatherErrors() { suite.NoError(err) suite.ElementsMatch(expectedResults, factResults) } + +func (suite *DispWorkGathererTestSuite) TestPackageVersionGathererContextCancelled() { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + suite.mockExecutor. + On("ExecContext", mock.Anything, "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, ""). + Return(nil, ctx.Err()) + + c := gatherers.NewSapHostCtrlGatherer(suite.mockExecutor) + factRequests := []entities.FactRequest{ + { + Name: "corosync", + Gatherer: "package_version", + Argument: "corosync", + CheckID: "check1", + }, + } + factResults, err := c.Gather(ctx, factRequests) + + suite.Error(err) + suite.Empty(factResults) +} From ff5486de05232bf8538ffebbbf9081117fe5f236 Mon Sep 17 00:00:00 2001 From: balanza Date: Wed, 22 Jan 2025 21:15:23 +0100 Subject: [PATCH 07/13] saphostctrl --- internal/factsengine/gatherers/saphostctrl.go | 21 ++++++++--- .../factsengine/gatherers/saphostctrl_test.go | 36 +++++++++++++++---- 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/internal/factsengine/gatherers/saphostctrl.go b/internal/factsengine/gatherers/saphostctrl.go index 4847bd99..b5ebab2e 100644 --- a/internal/factsengine/gatherers/saphostctrl.go +++ b/internal/factsengine/gatherers/saphostctrl.go @@ -65,7 +65,10 @@ func NewSapHostCtrlGatherer(executor utils.CommandExecutor) *SapHostCtrlGatherer } } -func (g *SapHostCtrlGatherer) Gather(_ context.Context, factsRequests []entities.FactRequest) ([]entities.Fact, error) { +func (g *SapHostCtrlGatherer) Gather( + ctx context.Context, + factsRequests []entities.FactRequest, +) ([]entities.Fact, error) { facts := []entities.Fact{} log.Infof("Starting saphostctrl facts gathering process") @@ -74,7 +77,7 @@ func (g *SapHostCtrlGatherer) Gather(_ context.Context, factsRequests []entities if len(factReq.Argument) == 0 { log.Error(SapHostCtrlMissingArgument.Message) fact = entities.NewFactGatheredWithError(factReq, &SapHostCtrlMissingArgument) - } else if factValue, err := handleWebmethod(g.executor, factReq.Argument); err != nil { + } else if factValue, err := handleWebmethod(ctx, g.executor, factReq.Argument); err != nil { fact = entities.NewFactGatheredWithError(factReq, err) } else { fact = entities.NewFactGatheredWithRequest(factReq, factValue) @@ -84,10 +87,14 @@ func (g *SapHostCtrlGatherer) Gather(_ context.Context, factsRequests []entities } log.Infof("Requested %s facts gathered", SapHostCtrlGathererName) + if ctx.Err() != nil { + return nil, ctx.Err() + } return facts, nil } func handleWebmethod( + ctx context.Context, executor utils.CommandExecutor, webMethod string, ) (entities.FactValue, *entities.FactGatheringError) { @@ -99,7 +106,7 @@ func handleWebmethod( return nil, gatheringError } - saphostctlOutput, commandError := executeSapHostCtrlCommand(executor, webMethod) + saphostctlOutput, commandError := executeSapHostCtrlCommand(ctx, executor, webMethod) if commandError != nil { return nil, commandError } @@ -107,8 +114,12 @@ func handleWebmethod( return webMethodHandler(saphostctlOutput) } -func executeSapHostCtrlCommand(executor utils.CommandExecutor, command string) (string, *entities.FactGatheringError) { - saphostctlOutput, err := executor.Exec("/usr/sap/hostctrl/exe/saphostctrl", "-function", command) +func executeSapHostCtrlCommand( + ctx context.Context, + executor utils.CommandExecutor, + command string, +) (string, *entities.FactGatheringError) { + saphostctlOutput, err := executor.ExecContext(ctx, "/usr/sap/hostctrl/exe/saphostctrl", "-function", command) if err != nil { gatheringError := SapHostCtrlCommandError.Wrap(err.Error()) log.Error(gatheringError) diff --git a/internal/factsengine/gatherers/saphostctrl_test.go b/internal/factsengine/gatherers/saphostctrl_test.go index e25b4da2..82fd1095 100644 --- a/internal/factsengine/gatherers/saphostctrl_test.go +++ b/internal/factsengine/gatherers/saphostctrl_test.go @@ -5,6 +5,7 @@ import ( "errors" "testing" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" "github.com/trento-project/agent/internal/factsengine/gatherers" "github.com/trento-project/agent/pkg/factsengine/entities" @@ -25,7 +26,7 @@ func (suite *SapHostCtrlTestSuite) SetupTest() { } func (suite *SapHostCtrlTestSuite) TestSapHostCtrlGathererNoArgumentProvided() { - suite.mockExecutor.On("Exec", "/usr/sap/hostctrl/exe/saphostctrl", "-function", "Ping").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "/usr/sap/hostctrl/exe/saphostctrl", "-function", "Ping").Return( []byte("SUCCESS ( 543341 usec)\n"), nil) c := gatherers.NewSapHostCtrlGatherer(suite.mockExecutor) @@ -68,7 +69,7 @@ func (suite *SapHostCtrlTestSuite) TestSapHostCtrlGathererNoArgumentProvided() { } func (suite *SapHostCtrlTestSuite) TestSapHostCtrlGatherListInstances() { - suite.mockExecutor.On("Exec", "/usr/sap/hostctrl/exe/saphostctrl", "-function", "ListInstances").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "/usr/sap/hostctrl/exe/saphostctrl", "-function", "ListInstances").Return( []byte(" Inst Info : S41 - 41 - s41app - 785, patch 50, changelist 2091708\n"+ " Inst Info : S41 - 40 - s41app - 785, patch 50, changelist 2091708\n"+ " Inst Info : HS1 - 11 - s41db - 753, patch 819, changelist 2069355\n"), nil) @@ -124,7 +125,7 @@ func (suite *SapHostCtrlTestSuite) TestSapHostCtrlGatherListInstances() { } func (suite *SapHostCtrlTestSuite) TestSapHostCtrlGatherPingSuccess() { - suite.mockExecutor.On("Exec", "/usr/sap/hostctrl/exe/saphostctrl", "-function", "Ping").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "/usr/sap/hostctrl/exe/saphostctrl", "-function", "Ping").Return( []byte("SUCCESS ( 543341 usec)\n"), nil) p := gatherers.NewSapHostCtrlGatherer(suite.mockExecutor) @@ -158,7 +159,7 @@ func (suite *SapHostCtrlTestSuite) TestSapHostCtrlGatherPingSuccess() { } func (suite *SapHostCtrlTestSuite) TestSapHostCtrlGatherPingFailed() { - suite.mockExecutor.On("Exec", "/usr/sap/hostctrl/exe/saphostctrl", "-function", "Ping").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "/usr/sap/hostctrl/exe/saphostctrl", "-function", "Ping").Return( []byte("FAILED ( 497 usec)\n"), nil) p := gatherers.NewSapHostCtrlGatherer(suite.mockExecutor) @@ -192,9 +193,9 @@ func (suite *SapHostCtrlTestSuite) TestSapHostCtrlGatherPingFailed() { } func (suite *SapHostCtrlTestSuite) TestSapHostCtrlGatherError() { - suite.mockExecutor.On("Exec", "/usr/sap/hostctrl/exe/saphostctrl", "-function", "Ping").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "/usr/sap/hostctrl/exe/saphostctrl", "-function", "Ping").Return( []byte("Unexpected output\n"), nil) - suite.mockExecutor.On("Exec", "/usr/sap/hostctrl/exe/saphostctrl", "-function", "ListInstances").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "/usr/sap/hostctrl/exe/saphostctrl", "-function", "ListInstances").Return( nil, errors.New("some error")) p := gatherers.NewSapHostCtrlGatherer(suite.mockExecutor) @@ -255,3 +256,26 @@ func (suite *SapHostCtrlTestSuite) TestSapHostCtrlGatherError() { suite.NoError(err) suite.ElementsMatch(expectedResults, factResults) } + +func (suite *DispWorkGathererTestSuite) TestSapHostCtrlGathererContextCancelled() { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + suite.mockExecutor. + On("ExecContext", mock.Anything, "/usr/sap/hostctrl/exe/saphostctrl", "-function", "Ping"). + Return(nil, ctx.Err()) + + c := gatherers.NewSapHostCtrlGatherer(suite.mockExecutor) + factRequests := []entities.FactRequest{ + { + Name: "ping", + Gatherer: "saphostctrl", + Argument: "Ping", + CheckID: "check1", + }, + } + factResults, err := c.Gather(ctx, factRequests) + + suite.Error(err) + suite.Empty(factResults) +} From 5e30bab3fe4e022b929a38ff32858c30059a3084 Mon Sep 17 00:00:00 2001 From: balanza Date: Wed, 22 Jan 2025 21:15:35 +0100 Subject: [PATCH 08/13] sbddump --- internal/factsengine/gatherers/sbddump.go | 19 ++++++++--- .../factsengine/gatherers/sbddump_test.go | 33 ++++++++++++++++--- 2 files changed, 43 insertions(+), 9 deletions(-) diff --git a/internal/factsengine/gatherers/sbddump.go b/internal/factsengine/gatherers/sbddump.go index eff97afd..71ccd3b0 100644 --- a/internal/factsengine/gatherers/sbddump.go +++ b/internal/factsengine/gatherers/sbddump.go @@ -49,7 +49,7 @@ func NewSBDDumpGatherer(executor utils.CommandExecutor, sbdConfigFile string) *S } func (gatherer *SBDDumpGatherer) Gather( - _ context.Context, + ctx context.Context, factsRequests []entities.FactRequest, ) ([]entities.Fact, error) { facts := []entities.Fact{} @@ -64,7 +64,7 @@ func (gatherer *SBDDumpGatherer) Gather( for _, factRequest := range factsRequests { var fact entities.Fact - if devicesDumps, err := getSBDDevicesDumps(gatherer.executor, configuredDevices); err == nil { + if devicesDumps, err := getSBDDevicesDumps(ctx, gatherer.executor, configuredDevices); err == nil { fact = entities.NewFactGatheredWithRequest(factRequest, devicesDumps) } else { gatheringError := SBDDumpCommandError.Wrap(err.Error()) @@ -75,6 +75,11 @@ func (gatherer *SBDDumpGatherer) Gather( } log.Infof("Requested %s facts gathered", SBDDumpGathererName) + + if ctx.Err() != nil { + return nil, ctx.Err() + } + return facts, nil } @@ -95,12 +100,13 @@ func loadDevices(sbdConfigFile string) ([]string, error) { } func getSBDDevicesDumps( + ctx context.Context, executor utils.CommandExecutor, configuredDevices []string) (*entities.FactValueMap, error) { var devicesDumps = make(map[string]entities.FactValue) for _, device := range configuredDevices { - SBDDumpMap, err := getSBDDumpFactValueMap(executor, device) + SBDDumpMap, err := getSBDDumpFactValueMap(ctx, executor, device) if err != nil { log.Errorf("Error getting sbd dump for device: %s", device) @@ -113,8 +119,11 @@ func getSBDDevicesDumps( return &entities.FactValueMap{Value: devicesDumps}, nil } -func getSBDDumpFactValueMap(executor utils.CommandExecutor, device string) (*entities.FactValueMap, error) { - SBDDump, err := executor.Exec("sbd", "-d", device, "dump") +func getSBDDumpFactValueMap( + ctx context.Context, + executor utils.CommandExecutor, + device string) (*entities.FactValueMap, error) { + SBDDump, err := executor.ExecContext(ctx, "sbd", "-d", device, "dump") if err != nil { return nil, fmt.Errorf("Error while dumping information for device %s: %w", device, err) } diff --git a/internal/factsengine/gatherers/sbddump_test.go b/internal/factsengine/gatherers/sbddump_test.go index 997cc8e9..ac27d3ec 100644 --- a/internal/factsengine/gatherers/sbddump_test.go +++ b/internal/factsengine/gatherers/sbddump_test.go @@ -7,6 +7,7 @@ import ( "os" "testing" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" "github.com/trento-project/agent/internal/factsengine/gatherers" "github.com/trento-project/agent/pkg/factsengine/entities" @@ -51,8 +52,8 @@ func (suite *SBDDumpTestSuite) TestSBDDumpUnableToDumpDevice() { mockOutputFile, _ := os.Open(helpers.GetFixturePath("gatherers/dev.vdc.sbddump.output")) mockOutput, _ := io.ReadAll(mockOutputFile) - mockExecutor.On("Exec", "sbd", "-d", "/dev/vdb", "dump").Return(nil, errors.New("a failure")) - mockExecutor.On("Exec", "sbd", "-d", "/dev/vdc", "dump").Return(mockOutput, nil) + mockExecutor.On("ExecContext", mock.Anything, "sbd", "-d", "/dev/vdb", "dump").Return(nil, errors.New("a failure")) + mockExecutor.On("ExecContext", mock.Anything, "sbd", "-d", "/dev/vdc", "dump").Return(mockOutput, nil) sbdDumpGatherer := gatherers.NewSBDDumpGatherer( mockExecutor, @@ -105,8 +106,8 @@ func (suite *SBDDumpTestSuite) TestSBDDumpGatherer() { deviceVDCMockOutputFile, _ := os.Open(helpers.GetFixturePath("gatherers/dev.vdc.sbddump.output")) deviceVDCMockOutput, _ := io.ReadAll(deviceVDCMockOutputFile) - mockExecutor.On("Exec", "sbd", "-d", "/dev/vdb", "dump").Return(deviceVDBMockOutput, nil) - mockExecutor.On("Exec", "sbd", "-d", "/dev/vdc", "dump").Return(deviceVDCMockOutput, nil) + mockExecutor.On("ExecContext", mock.Anything, "sbd", "-d", "/dev/vdb", "dump").Return(deviceVDBMockOutput, nil) + mockExecutor.On("ExecContext", mock.Anything, "sbd", "-d", "/dev/vdc", "dump").Return(deviceVDCMockOutput, nil) sbdDumpGatherer := gatherers.NewSBDDumpGatherer( mockExecutor, @@ -169,3 +170,27 @@ func (suite *SBDDumpTestSuite) TestSBDDumpGatherer() { suite.NoError(err) suite.ElementsMatch(expectedResults, factResults) } + +func (suite *SBDDumpTestSuite) TestSBDDumpCancelledContext() { + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + mockExecutor := new(utilsMocks.CommandExecutor) + mockExecutor.On("ExecContext", mock.Anything, "sbd", "-d", "/dev/vdc", "dump").Return(nil, ctx.Err()) + sbdDumpGatherer := gatherers.NewSBDDumpGatherer( + mockExecutor, + helpers.GetFixturePath("discovery/cluster/sbd/sbd_config"), + ) + factRequests := []entities.FactRequest{ + { + Name: "sbd_devices_dump", + Gatherer: "sbd_dump", + }, + } + + gatheredFacts, err := sbdDumpGatherer.Gather(ctx, factRequests) + + suite.Error(err) + suite.Empty(gatheredFacts) +} From e662064414b478a05c510b58948443bc4f6a2ec2 Mon Sep 17 00:00:00 2001 From: balanza Date: Wed, 22 Jan 2025 21:15:44 +0100 Subject: [PATCH 09/13] sysctl --- internal/factsengine/gatherers/sysctl.go | 4 +-- internal/factsengine/gatherers/sysctl_test.go | 32 +++++++++++++++---- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/internal/factsengine/gatherers/sysctl.go b/internal/factsengine/gatherers/sysctl.go index aea5d478..fd44db1e 100644 --- a/internal/factsengine/gatherers/sysctl.go +++ b/internal/factsengine/gatherers/sysctl.go @@ -45,11 +45,11 @@ func NewSysctlGatherer(executor utils.CommandExecutor) *SysctlGatherer { } } -func (s *SysctlGatherer) Gather(_ context.Context, factsRequests []entities.FactRequest) ([]entities.Fact, error) { +func (s *SysctlGatherer) Gather(ctx context.Context, factsRequests []entities.FactRequest) ([]entities.Fact, error) { facts := []entities.Fact{} log.Infof("Starting %s facts gathering process", SysctlGathererName) - output, err := s.executor.Exec("sysctl", "-a") + output, err := s.executor.ExecContext(ctx, "sysctl", "-a") if err != nil { return nil, SysctlCommandError.Wrap(err.Error()) } diff --git a/internal/factsengine/gatherers/sysctl_test.go b/internal/factsengine/gatherers/sysctl_test.go index 4f810ba3..bdd1d386 100644 --- a/internal/factsengine/gatherers/sysctl_test.go +++ b/internal/factsengine/gatherers/sysctl_test.go @@ -7,6 +7,7 @@ import ( "os/exec" "testing" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" "github.com/trento-project/agent/internal/factsengine/gatherers" "github.com/trento-project/agent/pkg/factsengine/entities" @@ -31,7 +32,7 @@ func (suite *SysctlTestSuite) SetupTest() { func (suite *SysctlTestSuite) TestSysctlGathererNoArgumentProvided() { mockOutputFile, _ := os.Open(helpers.GetFixturePath("gatherers/sysctl.output")) mockOutput, _ := io.ReadAll(mockOutputFile) - suite.mockExecutor.On("Exec", "sysctl", "-a").Return(mockOutput, nil) + suite.mockExecutor.On("ExecContext", mock.Anything, "sysctl", "-a").Return(mockOutput, nil) c := gatherers.NewSysctlGatherer(suite.mockExecutor) @@ -75,7 +76,7 @@ func (suite *SysctlTestSuite) TestSysctlGathererNoArgumentProvided() { func (suite *SysctlTestSuite) TestSysctlGathererNonExistingKey() { mockOutputFile, _ := os.Open(helpers.GetFixturePath("gatherers/sysctl.output")) mockOutput, _ := io.ReadAll(mockOutputFile) - suite.mockExecutor.On("Exec", "sysctl", "-a").Return(mockOutput, nil) + suite.mockExecutor.On("ExecContext", mock.Anything, "sysctl", "-a").Return(mockOutput, nil) c := gatherers.NewSysctlGatherer(suite.mockExecutor) @@ -105,7 +106,7 @@ func (suite *SysctlTestSuite) TestSysctlGathererNonExistingKey() { } func (suite *SysctlTestSuite) TestSysctlCommandNotFound() { - suite.mockExecutor.On("Exec", "sysctl", "-a").Return(nil, exec.ErrNotFound) + suite.mockExecutor.On("ExecContext", mock.Anything, "sysctl", "-a").Return(nil, exec.ErrNotFound) c := gatherers.NewSysctlGatherer(suite.mockExecutor) @@ -132,7 +133,7 @@ func (suite *SysctlTestSuite) TestSysctlCommandNotFound() { func (suite *SysctlTestSuite) TestSysctlGatherer() { mockOutputFile, _ := os.Open(helpers.GetFixturePath("gatherers/sysctl.output")) mockOutput, _ := io.ReadAll(mockOutputFile) - suite.mockExecutor.On("Exec", "sysctl", "-a").Return(mockOutput, nil) + suite.mockExecutor.On("ExecContext", mock.Anything, "sysctl", "-a").Return(mockOutput, nil) c := gatherers.NewSysctlGatherer(suite.mockExecutor) @@ -160,7 +161,7 @@ func (suite *SysctlTestSuite) TestSysctlGatherer() { func (suite *SysctlTestSuite) TestSysctlGathererPartialKey() { mockOutputFile, _ := os.Open(helpers.GetFixturePath("gatherers/sysctl.output")) mockOutput, _ := io.ReadAll(mockOutputFile) - suite.mockExecutor.On("Exec", "sysctl", "-a").Return(mockOutput, nil) + suite.mockExecutor.On("ExecContext", mock.Anything, "sysctl", "-a").Return(mockOutput, nil) c := gatherers.NewSysctlGatherer(suite.mockExecutor) @@ -193,7 +194,7 @@ func (suite *SysctlTestSuite) TestSysctlGathererPartialKey() { func (suite *SysctlTestSuite) TestSysctlGathererEmptyValue() { mockOutputFile, _ := os.Open(helpers.GetFixturePath("gatherers/sysctl.output")) mockOutput, _ := io.ReadAll(mockOutputFile) - suite.mockExecutor.On("Exec", "sysctl", "-a").Return(mockOutput, nil) + suite.mockExecutor.On("ExecContext", mock.Anything, "sysctl", "-a").Return(mockOutput, nil) c := gatherers.NewSysctlGatherer(suite.mockExecutor) @@ -217,3 +218,22 @@ func (suite *SysctlTestSuite) TestSysctlGathererEmptyValue() { suite.NoError(err) suite.ElementsMatch(expectedResults, factResults) } + +func (suite *SysctlTestSuite) TestSysctlGathererContextCancelled() { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + suite.mockExecutor.On("ExecContext", mock.Anything, "sysctl", "-a").Return(nil, ctx.Err()) + + c := gatherers.NewSysctlGatherer(suite.mockExecutor) + factRequests := []entities.FactRequest{ + { + Name: "context_cancelled_fact", + Gatherer: "sysctl", + }, + } + factResults, err := c.Gather(ctx, factRequests) + + suite.Error(err) + suite.Empty(factResults) +} From d32aee04646787a5fe804015a1f5b86463d8ab92 Mon Sep 17 00:00:00 2001 From: balanza Date: Wed, 22 Jan 2025 21:15:54 +0100 Subject: [PATCH 10/13] verifypassword --- .../factsengine/gatherers/verifypassword.go | 13 ++++-- .../gatherers/verifypassword_test.go | 41 +++++++++++++++---- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/internal/factsengine/gatherers/verifypassword.go b/internal/factsengine/gatherers/verifypassword.go index 04de4337..373b8588 100644 --- a/internal/factsengine/gatherers/verifypassword.go +++ b/internal/factsengine/gatherers/verifypassword.go @@ -70,7 +70,7 @@ func NewVerifyPasswordGatherer(executor utils.CommandExecutor) *VerifyPasswordGa This gatherer expects only the username for which the password will be verified */ func (g *VerifyPasswordGatherer) Gather( - _ context.Context, + ctx context.Context, factsRequests []entities.FactRequest, ) ([]entities.Fact, error) { facts := []entities.Fact{} @@ -85,9 +85,14 @@ func (g *VerifyPasswordGatherer) Gather( } username := factReq.Argument - hash, err := g.getHash(username) + hash, err := g.getHash(ctx, username) switch { + case ctx.Err() != nil: + { + log.Warn("Context cancelled") + return nil, ctx.Err() + } case err != nil: { gatheringError := VerifyPasswordShadowError.Wrap(err.Error()) @@ -147,8 +152,8 @@ func (g *VerifyPasswordGatherer) Gather( return facts, nil } -func (g *VerifyPasswordGatherer) getHash(user string) (string, error) { - shadow, err := g.executor.Exec("getent", "shadow", user) +func (g *VerifyPasswordGatherer) getHash(ctx context.Context, user string) (string, error) { + shadow, err := g.executor.ExecContext(ctx, "getent", "shadow", user) if err != nil { return "", errors.Wrap(err, "Error getting hash") } diff --git a/internal/factsengine/gatherers/verifypassword_test.go b/internal/factsengine/gatherers/verifypassword_test.go index 6b9f2329..3da5dd25 100644 --- a/internal/factsengine/gatherers/verifypassword_test.go +++ b/internal/factsengine/gatherers/verifypassword_test.go @@ -4,6 +4,7 @@ import ( "context" "testing" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" "github.com/trento-project/agent/internal/factsengine/gatherers" "github.com/trento-project/agent/pkg/factsengine/entities" @@ -28,7 +29,7 @@ func (suite *PasswordTestSuite) TestPasswordGatherEqual() { "cP8uJJpHzJ7ufTPjYuWoVF4s.3MUdOR9iwcO.6E3uCHX1waqypjey458NKGE9O7lnWpV/" + "qd2tg1:19029::::::") - suite.mockExecutor.On("Exec", "getent", "shadow", "hacluster").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "getent", "shadow", "hacluster").Return( shadow, nil) verifyPasswordGatherer := gatherers.NewVerifyPasswordGatherer(suite.mockExecutor) @@ -61,7 +62,7 @@ func (suite *PasswordTestSuite) TestPasswordGatherNotEqual() { "cP8uJJpHzJ7ufTPjYuWoVF4s.3MUdOR9iwcO.6E3uCHX1waqypjey458NKGE9O7lnWpV" + "/qd2tg1:19029::::::") - suite.mockExecutor.On("Exec", "getent", "shadow", "hacluster").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "getent", "shadow", "hacluster").Return( shadow, nil) verifyPasswordGatherer := gatherers.NewVerifyPasswordGatherer(suite.mockExecutor) @@ -91,13 +92,13 @@ func (suite *PasswordTestSuite) TestPasswordGatherNotEqual() { func (suite *PasswordTestSuite) TestPasswordGatherBloquedPassword() { suite.mockExecutor. - On("Exec", "getent", "shadow", "hacluster"). + On("ExecContext", mock.Anything, "getent", "shadow", "hacluster"). Return([]byte("hacluster:!:19029::::::"), nil). Once(). - On("Exec", "getent", "shadow", "hacluster"). + On("ExecContext", mock.Anything, "getent", "shadow", "hacluster"). Return([]byte("hacluster:!$6$WFEgSAefduOyvLCN$MprO90E:19029::::::"), nil). Once(). - On("Exec", "getent", "shadow", "hacluster"). + On("ExecContext", mock.Anything, "getent", "shadow", "hacluster"). Return([]byte("hacluster:*:19029::::::"), nil) verifyPasswordGatherer := gatherers.NewVerifyPasswordGatherer(suite.mockExecutor) @@ -162,7 +163,7 @@ func (suite *PasswordTestSuite) TestPasswordGatherBloquedPassword() { func (suite *PasswordTestSuite) TestPasswordGatherNoPassword() { shadow := []byte("hacluster::19029::::::") - suite.mockExecutor.On("Exec", "getent", "shadow", "hacluster").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "getent", "shadow", "hacluster").Return( shadow, nil) verifyPasswordGatherer := gatherers.NewVerifyPasswordGatherer(suite.mockExecutor) @@ -199,7 +200,7 @@ func (suite *PasswordTestSuite) TestPasswordGatherDifferentEncType() { "cP8uJJpHzJ7ufTPjYuWoVF4s.3MUdOR9iwcO.6E3uCHX1waqypjey458NKGE9O7lnWpV/" + "qd2tg1:19029::::::") - suite.mockExecutor.On("Exec", "getent", "shadow", "hacluster").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "getent", "shadow", "hacluster").Return( shadow, nil) verifyPasswordGatherer := gatherers.NewVerifyPasswordGatherer(suite.mockExecutor) @@ -234,7 +235,7 @@ func (suite *PasswordTestSuite) TestPasswordGatherDifferentEncType() { func (suite *PasswordTestSuite) TestPasswordGatherInvalidShadowOutput() { shadow := []byte("hacluster:hash:") - suite.mockExecutor.On("Exec", "getent", "shadow", "hacluster").Return( + suite.mockExecutor.On("ExecContext", mock.Anything, "getent", "shadow", "hacluster").Return( shadow, nil) verifyPasswordGatherer := gatherers.NewVerifyPasswordGatherer(suite.mockExecutor) @@ -295,3 +296,27 @@ func (suite *PasswordTestSuite) TestPasswordGatherWrongArguments() { suite.NoError(err) suite.ElementsMatch(expectedResults, factResults) } + +func (suite *PasswordTestSuite) TestPasswordGatherContextCancelled() { + // Create a cancelled context + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + suite.mockExecutor.On("ExecContext", mock.Anything, "getent", "shadow", "hacluster"). + Return(nil, ctx.Err()) + + verifyPasswordGatherer := gatherers.NewVerifyPasswordGatherer(suite.mockExecutor) + factRequests := []entities.FactRequest{ + { + Name: "hacluster", + Gatherer: "verify_password", + Argument: "hacluster", + CheckID: "check1", + }, + } + + factResults, err := verifyPasswordGatherer.Gather(ctx, factRequests) + + suite.Error(err) + suite.Empty(factResults) +} From bb7d346f976c4c152f8008f6d74952f3c373afc7 Mon Sep 17 00:00:00 2001 From: balanza Date: Thu, 23 Jan 2025 10:08:07 +0100 Subject: [PATCH 11/13] fix context --- internal/factsengine/gatherers/ascsers_cluster.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/factsengine/gatherers/ascsers_cluster.go b/internal/factsengine/gatherers/ascsers_cluster.go index 1be81707..0a2a3bab 100644 --- a/internal/factsengine/gatherers/ascsers_cluster.go +++ b/internal/factsengine/gatherers/ascsers_cluster.go @@ -75,14 +75,12 @@ func (g *AscsErsClusterGatherer) SetCache(cache *factscache.FactsCache) { } func (g *AscsErsClusterGatherer) Gather( - _ context.Context, + ctx context.Context, factsRequests []entities.FactRequest, ) ([]entities.Fact, error) { log.Infof("Starting %s facts gathering process", AscsErsClusterGathererName) var cibdata cib.Root - ctx := context.Background() - content, err := factscache.GetOrUpdate( g.cache, CibAdminGathererCache, From 596152435a42cc5abe3aaf86f435937ca1e89013 Mon Sep 17 00:00:00 2001 From: balanza Date: Thu, 23 Jan 2025 10:16:12 +0100 Subject: [PATCH 12/13] refactor tests --- internal/factsengine/gatherers/ascsers_cluster_test.go | 2 +- internal/factsengine/gatherers/cibadmin_test.go | 2 +- internal/factsengine/gatherers/corosynccmapctl_test.go | 2 +- internal/factsengine/gatherers/dispwork_test.go | 2 +- internal/factsengine/gatherers/mountinfo_test.go | 2 +- internal/factsengine/gatherers/packageversion_test.go | 2 +- internal/factsengine/gatherers/saphostctrl_test.go | 2 +- internal/factsengine/gatherers/sysctl_test.go | 2 +- internal/factsengine/gatherers/verifypassword_test.go | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/internal/factsengine/gatherers/ascsers_cluster_test.go b/internal/factsengine/gatherers/ascsers_cluster_test.go index 58f5c219..580ed55c 100644 --- a/internal/factsengine/gatherers/ascsers_cluster_test.go +++ b/internal/factsengine/gatherers/ascsers_cluster_test.go @@ -293,7 +293,7 @@ func (suite *AscsErsClusterTestSuite) TestAscsErskGathererContextCancelled() { cancel() suite.mockExecutor. - On("ExecContext", mock.Anything, "cibadmin", "--query", "--local"). + On("ExecContext", ctx, "cibadmin", "--query", "--local"). Return(nil, ctx.Err()) c := gatherers.NewAscsErsClusterGatherer(suite.mockExecutor, suite.webService, nil) diff --git a/internal/factsengine/gatherers/cibadmin_test.go b/internal/factsengine/gatherers/cibadmin_test.go index 3b0d49fc..fad872ae 100644 --- a/internal/factsengine/gatherers/cibadmin_test.go +++ b/internal/factsengine/gatherers/cibadmin_test.go @@ -277,7 +277,7 @@ func (suite *CibAdminTestSuite) TestCibAdminGatherWithContextCancelled() { ctx, cancel := context.WithCancel(context.Background()) cancel() - suite.mockExecutor.On("ExecContext", mock.Anything, "cibadmin", "--query", "--local"). + suite.mockExecutor.On("ExecContext", ctx, "cibadmin", "--query", "--local"). Return(nil, ctx.Err()) p := gatherers.NewCibAdminGatherer(suite.mockExecutor, nil) diff --git a/internal/factsengine/gatherers/corosynccmapctl_test.go b/internal/factsengine/gatherers/corosynccmapctl_test.go index 92c8b89e..c3f51ee4 100644 --- a/internal/factsengine/gatherers/corosynccmapctl_test.go +++ b/internal/factsengine/gatherers/corosynccmapctl_test.go @@ -215,7 +215,7 @@ func (suite *CorosyncCmapctlTestSuite) TestCorosyncCmapctlGathererContextCancell cancel() suite.mockExecutor. - On("ExecContext", mock.Anything, "corosync-cmapctl", "-b"). + On("ExecContext", ctx, "corosync-cmapctl", "-b"). Return(nil, ctx.Err()) c := gatherers.NewCorosyncCmapctlGatherer(suite.mockExecutor) diff --git a/internal/factsengine/gatherers/dispwork_test.go b/internal/factsengine/gatherers/dispwork_test.go index 0b9624be..9d5b822d 100644 --- a/internal/factsengine/gatherers/dispwork_test.go +++ b/internal/factsengine/gatherers/dispwork_test.go @@ -139,7 +139,7 @@ func (suite *DispWorkGathererTestSuite) TestDispWorkGathererContextCancelled() { cancel() suite.mockExecutor. - On("ExecContext", mock.Anything, "su", "-", mock.AnythingOfType("string"), "-c", "\"disp+work\""). + On("ExecContext", ctx, "su", "-", mock.AnythingOfType("string"), "-c", "\"disp+work\""). Return(nil, ctx.Err()) c := gatherers.NewDispWorkGatherer(suite.fs, suite.mockExecutor) diff --git a/internal/factsengine/gatherers/mountinfo_test.go b/internal/factsengine/gatherers/mountinfo_test.go index 23b6ff28..c5c9dfd7 100644 --- a/internal/factsengine/gatherers/mountinfo_test.go +++ b/internal/factsengine/gatherers/mountinfo_test.go @@ -217,7 +217,7 @@ func (suite *MountInfoTestSuite) TestMountInfoParsingGathererContextCancelled() cancel() suite.mockMountParser. - On("ExecContext", mock.Anything, "blkid", "10.1.1.10:/sapmnt", "-o", "export"). + On("ExecContext", ctx, "blkid", "10.1.1.10:/sapmnt", "-o", "export"). Return(nil, ctx.Err()) c := gatherers.NewSapHostCtrlGatherer(suite.mockExecutor) diff --git a/internal/factsengine/gatherers/packageversion_test.go b/internal/factsengine/gatherers/packageversion_test.go index d9482484..beefa512 100644 --- a/internal/factsengine/gatherers/packageversion_test.go +++ b/internal/factsengine/gatherers/packageversion_test.go @@ -355,7 +355,7 @@ func (suite *DispWorkGathererTestSuite) TestPackageVersionGathererContextCancell cancel() suite.mockExecutor. - On("ExecContext", mock.Anything, "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, ""). + On("ExecContext", ctx, "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, ""). Return(nil, ctx.Err()) c := gatherers.NewSapHostCtrlGatherer(suite.mockExecutor) diff --git a/internal/factsengine/gatherers/saphostctrl_test.go b/internal/factsengine/gatherers/saphostctrl_test.go index 82fd1095..41f312c3 100644 --- a/internal/factsengine/gatherers/saphostctrl_test.go +++ b/internal/factsengine/gatherers/saphostctrl_test.go @@ -262,7 +262,7 @@ func (suite *DispWorkGathererTestSuite) TestSapHostCtrlGathererContextCancelled( cancel() suite.mockExecutor. - On("ExecContext", mock.Anything, "/usr/sap/hostctrl/exe/saphostctrl", "-function", "Ping"). + On("ExecContext", ctx, "/usr/sap/hostctrl/exe/saphostctrl", "-function", "Ping"). Return(nil, ctx.Err()) c := gatherers.NewSapHostCtrlGatherer(suite.mockExecutor) diff --git a/internal/factsengine/gatherers/sysctl_test.go b/internal/factsengine/gatherers/sysctl_test.go index bdd1d386..5886f143 100644 --- a/internal/factsengine/gatherers/sysctl_test.go +++ b/internal/factsengine/gatherers/sysctl_test.go @@ -223,7 +223,7 @@ func (suite *SysctlTestSuite) TestSysctlGathererContextCancelled() { ctx, cancel := context.WithCancel(context.Background()) cancel() - suite.mockExecutor.On("ExecContext", mock.Anything, "sysctl", "-a").Return(nil, ctx.Err()) + suite.mockExecutor.On("ExecContext", ctx, "sysctl", "-a").Return(nil, ctx.Err()) c := gatherers.NewSysctlGatherer(suite.mockExecutor) factRequests := []entities.FactRequest{ diff --git a/internal/factsengine/gatherers/verifypassword_test.go b/internal/factsengine/gatherers/verifypassword_test.go index 3da5dd25..b42237a4 100644 --- a/internal/factsengine/gatherers/verifypassword_test.go +++ b/internal/factsengine/gatherers/verifypassword_test.go @@ -302,7 +302,7 @@ func (suite *PasswordTestSuite) TestPasswordGatherContextCancelled() { ctx, cancel := context.WithCancel(context.Background()) cancel() - suite.mockExecutor.On("ExecContext", mock.Anything, "getent", "shadow", "hacluster"). + suite.mockExecutor.On("ExecContext", ctx, "getent", "shadow", "hacluster"). Return(nil, ctx.Err()) verifyPasswordGatherer := gatherers.NewVerifyPasswordGatherer(suite.mockExecutor) From b064d323f9d672b12a09452156047bfee4f328bf Mon Sep 17 00:00:00 2001 From: balanza Date: Thu, 23 Jan 2025 10:33:04 +0100 Subject: [PATCH 13/13] refactor tests --- internal/factsengine/gatherers/ascsers_cluster_test.go | 9 +++------ internal/factsengine/gatherers/cibadmin_test.go | 6 ++---- internal/factsengine/gatherers/corosynccmapctl_test.go | 7 ++----- internal/factsengine/gatherers/dispwork_test.go | 7 ++----- internal/factsengine/gatherers/mountinfo_test.go | 7 ++----- internal/factsengine/gatherers/packageversion_test.go | 7 ++----- internal/factsengine/gatherers/saphostctrl_test.go | 7 ++----- internal/factsengine/gatherers/sbddump_test.go | 5 ++--- internal/factsengine/gatherers/sysctl_test.go | 5 ++--- internal/factsengine/gatherers/verifypassword_test.go | 6 ++---- 10 files changed, 21 insertions(+), 45 deletions(-) diff --git a/internal/factsengine/gatherers/ascsers_cluster_test.go b/internal/factsengine/gatherers/ascsers_cluster_test.go index 580ed55c..1a99fb70 100644 --- a/internal/factsengine/gatherers/ascsers_cluster_test.go +++ b/internal/factsengine/gatherers/ascsers_cluster_test.go @@ -15,6 +15,7 @@ import ( "github.com/trento-project/agent/internal/factsengine/factscache" "github.com/trento-project/agent/internal/factsengine/gatherers" "github.com/trento-project/agent/pkg/factsengine/entities" + "github.com/trento-project/agent/pkg/utils" utilsMocks "github.com/trento-project/agent/pkg/utils/mocks" "github.com/trento-project/agent/test/helpers" ) @@ -288,15 +289,11 @@ func (suite *AscsErsClusterTestSuite) TestAscsErsClusterGather() { suite.ElementsMatch(expectedEntries, entries) } -func (suite *AscsErsClusterTestSuite) TestAscsErskGathererContextCancelled() { +func (suite *AscsErsClusterTestSuite) TestAscsErsGathererContextCancelled() { ctx, cancel := context.WithCancel(context.Background()) cancel() - suite.mockExecutor. - On("ExecContext", ctx, "cibadmin", "--query", "--local"). - Return(nil, ctx.Err()) - - c := gatherers.NewAscsErsClusterGatherer(suite.mockExecutor, suite.webService, nil) + c := gatherers.NewAscsErsClusterGatherer(utils.Executor{}, suite.webService, nil) factRequests := []entities.FactRequest{ { Name: "ascsers", diff --git a/internal/factsengine/gatherers/cibadmin_test.go b/internal/factsengine/gatherers/cibadmin_test.go index fad872ae..cdb9ef5e 100644 --- a/internal/factsengine/gatherers/cibadmin_test.go +++ b/internal/factsengine/gatherers/cibadmin_test.go @@ -12,6 +12,7 @@ import ( "github.com/trento-project/agent/internal/factsengine/factscache" "github.com/trento-project/agent/internal/factsengine/gatherers" "github.com/trento-project/agent/pkg/factsengine/entities" + "github.com/trento-project/agent/pkg/utils" utilsMocks "github.com/trento-project/agent/pkg/utils/mocks" "github.com/trento-project/agent/test/helpers" ) @@ -277,10 +278,7 @@ func (suite *CibAdminTestSuite) TestCibAdminGatherWithContextCancelled() { ctx, cancel := context.WithCancel(context.Background()) cancel() - suite.mockExecutor.On("ExecContext", ctx, "cibadmin", "--query", "--local"). - Return(nil, ctx.Err()) - - p := gatherers.NewCibAdminGatherer(suite.mockExecutor, nil) + p := gatherers.NewCibAdminGatherer(utils.Executor{}, nil) factRequests := []entities.FactRequest{ { Name: "cib", diff --git a/internal/factsengine/gatherers/corosynccmapctl_test.go b/internal/factsengine/gatherers/corosynccmapctl_test.go index c3f51ee4..ee07ce55 100644 --- a/internal/factsengine/gatherers/corosynccmapctl_test.go +++ b/internal/factsengine/gatherers/corosynccmapctl_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/suite" "github.com/trento-project/agent/internal/factsengine/gatherers" "github.com/trento-project/agent/pkg/factsengine/entities" + "github.com/trento-project/agent/pkg/utils" utilsMocks "github.com/trento-project/agent/pkg/utils/mocks" "github.com/trento-project/agent/test/helpers" ) @@ -214,11 +215,7 @@ func (suite *CorosyncCmapctlTestSuite) TestCorosyncCmapctlGathererContextCancell ctx, cancel := context.WithCancel(context.Background()) cancel() - suite.mockExecutor. - On("ExecContext", ctx, "corosync-cmapctl", "-b"). - Return(nil, ctx.Err()) - - c := gatherers.NewCorosyncCmapctlGatherer(suite.mockExecutor) + c := gatherers.NewCorosyncCmapctlGatherer(utils.Executor{}) factRequests := []entities.FactRequest{ { Name: "madeup_fact", diff --git a/internal/factsengine/gatherers/dispwork_test.go b/internal/factsengine/gatherers/dispwork_test.go index 9d5b822d..6acc145b 100644 --- a/internal/factsengine/gatherers/dispwork_test.go +++ b/internal/factsengine/gatherers/dispwork_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/suite" "github.com/trento-project/agent/internal/factsengine/gatherers" "github.com/trento-project/agent/pkg/factsengine/entities" + "github.com/trento-project/agent/pkg/utils" utilsMocks "github.com/trento-project/agent/pkg/utils/mocks" "github.com/trento-project/agent/test/helpers" ) @@ -138,11 +139,7 @@ func (suite *DispWorkGathererTestSuite) TestDispWorkGathererContextCancelled() { ctx, cancel := context.WithCancel(context.Background()) cancel() - suite.mockExecutor. - On("ExecContext", ctx, "su", "-", mock.AnythingOfType("string"), "-c", "\"disp+work\""). - Return(nil, ctx.Err()) - - c := gatherers.NewDispWorkGatherer(suite.fs, suite.mockExecutor) + c := gatherers.NewDispWorkGatherer(suite.fs, utils.Executor{}) factRequests := []entities.FactRequest{ { Name: "dispwork", diff --git a/internal/factsengine/gatherers/mountinfo_test.go b/internal/factsengine/gatherers/mountinfo_test.go index c5c9dfd7..7b7195f0 100644 --- a/internal/factsengine/gatherers/mountinfo_test.go +++ b/internal/factsengine/gatherers/mountinfo_test.go @@ -11,6 +11,7 @@ import ( "github.com/trento-project/agent/internal/factsengine/gatherers" "github.com/trento-project/agent/internal/factsengine/gatherers/mocks" "github.com/trento-project/agent/pkg/factsengine/entities" + "github.com/trento-project/agent/pkg/utils" utilsMocks "github.com/trento-project/agent/pkg/utils/mocks" ) @@ -216,11 +217,7 @@ func (suite *MountInfoTestSuite) TestMountInfoParsingGathererContextCancelled() ctx, cancel := context.WithCancel(context.Background()) cancel() - suite.mockMountParser. - On("ExecContext", ctx, "blkid", "10.1.1.10:/sapmnt", "-o", "export"). - Return(nil, ctx.Err()) - - c := gatherers.NewSapHostCtrlGatherer(suite.mockExecutor) + c := gatherers.NewSapHostCtrlGatherer(utils.Executor{}) factRequests := []entities.FactRequest{ {Name: "shared", Gatherer: "mount_info", diff --git a/internal/factsengine/gatherers/packageversion_test.go b/internal/factsengine/gatherers/packageversion_test.go index beefa512..3a434c45 100644 --- a/internal/factsengine/gatherers/packageversion_test.go +++ b/internal/factsengine/gatherers/packageversion_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/suite" "github.com/trento-project/agent/internal/factsengine/gatherers" "github.com/trento-project/agent/pkg/factsengine/entities" + "github.com/trento-project/agent/pkg/utils" utilsMocks "github.com/trento-project/agent/pkg/utils/mocks" "github.com/trento-project/agent/test/helpers" ) @@ -354,11 +355,7 @@ func (suite *DispWorkGathererTestSuite) TestPackageVersionGathererContextCancell ctx, cancel := context.WithCancel(context.Background()) cancel() - suite.mockExecutor. - On("ExecContext", ctx, "/usr/bin/rpm", "-q", "--qf", packageVersionQueryFormat, ""). - Return(nil, ctx.Err()) - - c := gatherers.NewSapHostCtrlGatherer(suite.mockExecutor) + c := gatherers.NewSapHostCtrlGatherer(utils.Executor{}) factRequests := []entities.FactRequest{ { Name: "corosync", diff --git a/internal/factsengine/gatherers/saphostctrl_test.go b/internal/factsengine/gatherers/saphostctrl_test.go index 41f312c3..937b4c75 100644 --- a/internal/factsengine/gatherers/saphostctrl_test.go +++ b/internal/factsengine/gatherers/saphostctrl_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/suite" "github.com/trento-project/agent/internal/factsengine/gatherers" "github.com/trento-project/agent/pkg/factsengine/entities" + "github.com/trento-project/agent/pkg/utils" utilsMocks "github.com/trento-project/agent/pkg/utils/mocks" ) @@ -261,11 +262,7 @@ func (suite *DispWorkGathererTestSuite) TestSapHostCtrlGathererContextCancelled( ctx, cancel := context.WithCancel(context.Background()) cancel() - suite.mockExecutor. - On("ExecContext", ctx, "/usr/sap/hostctrl/exe/saphostctrl", "-function", "Ping"). - Return(nil, ctx.Err()) - - c := gatherers.NewSapHostCtrlGatherer(suite.mockExecutor) + c := gatherers.NewSapHostCtrlGatherer(utils.Executor{}) factRequests := []entities.FactRequest{ { Name: "ping", diff --git a/internal/factsengine/gatherers/sbddump_test.go b/internal/factsengine/gatherers/sbddump_test.go index ac27d3ec..851e5ec2 100644 --- a/internal/factsengine/gatherers/sbddump_test.go +++ b/internal/factsengine/gatherers/sbddump_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/suite" "github.com/trento-project/agent/internal/factsengine/gatherers" "github.com/trento-project/agent/pkg/factsengine/entities" + "github.com/trento-project/agent/pkg/utils" utilsMocks "github.com/trento-project/agent/pkg/utils/mocks" "github.com/trento-project/agent/test/helpers" ) @@ -176,10 +177,8 @@ func (suite *SBDDumpTestSuite) TestSBDDumpCancelledContext() { ctx, cancel := context.WithCancel(context.Background()) cancel() - mockExecutor := new(utilsMocks.CommandExecutor) - mockExecutor.On("ExecContext", mock.Anything, "sbd", "-d", "/dev/vdc", "dump").Return(nil, ctx.Err()) sbdDumpGatherer := gatherers.NewSBDDumpGatherer( - mockExecutor, + utils.Executor{}, helpers.GetFixturePath("discovery/cluster/sbd/sbd_config"), ) factRequests := []entities.FactRequest{ diff --git a/internal/factsengine/gatherers/sysctl_test.go b/internal/factsengine/gatherers/sysctl_test.go index 5886f143..e42c8781 100644 --- a/internal/factsengine/gatherers/sysctl_test.go +++ b/internal/factsengine/gatherers/sysctl_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/suite" "github.com/trento-project/agent/internal/factsengine/gatherers" "github.com/trento-project/agent/pkg/factsengine/entities" + "github.com/trento-project/agent/pkg/utils" utilsMocks "github.com/trento-project/agent/pkg/utils/mocks" "github.com/trento-project/agent/test/helpers" ) @@ -223,9 +224,7 @@ func (suite *SysctlTestSuite) TestSysctlGathererContextCancelled() { ctx, cancel := context.WithCancel(context.Background()) cancel() - suite.mockExecutor.On("ExecContext", ctx, "sysctl", "-a").Return(nil, ctx.Err()) - - c := gatherers.NewSysctlGatherer(suite.mockExecutor) + c := gatherers.NewSysctlGatherer(utils.Executor{}) factRequests := []entities.FactRequest{ { Name: "context_cancelled_fact", diff --git a/internal/factsengine/gatherers/verifypassword_test.go b/internal/factsengine/gatherers/verifypassword_test.go index b42237a4..4980cfdb 100644 --- a/internal/factsengine/gatherers/verifypassword_test.go +++ b/internal/factsengine/gatherers/verifypassword_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/suite" "github.com/trento-project/agent/internal/factsengine/gatherers" "github.com/trento-project/agent/pkg/factsengine/entities" + "github.com/trento-project/agent/pkg/utils" utilsMocks "github.com/trento-project/agent/pkg/utils/mocks" ) @@ -302,10 +303,7 @@ func (suite *PasswordTestSuite) TestPasswordGatherContextCancelled() { ctx, cancel := context.WithCancel(context.Background()) cancel() - suite.mockExecutor.On("ExecContext", ctx, "getent", "shadow", "hacluster"). - Return(nil, ctx.Err()) - - verifyPasswordGatherer := gatherers.NewVerifyPasswordGatherer(suite.mockExecutor) + verifyPasswordGatherer := gatherers.NewVerifyPasswordGatherer(utils.Executor{}) factRequests := []entities.FactRequest{ { Name: "hacluster",