chore(super-agent): Code clean up and added tests

internal/install/bundle_installer.go

@@ -3,7 +3,7 @@ package install
 import (
 	"context"
 	"fmt"
-	"github.com/newrelic/newrelic-cli/internal/utils"
+	"strings"
 
 	log "github.com/sirupsen/logrus"
 
@@ -27,7 +27,6 @@ type BundleInstaller struct {
 }
 
 func NewBundleInstaller(ctx context.Context, manifest *types.DiscoveryManifest, recipeInstallerInterface RecipeInstaller, statusReporter StatusReporter) *BundleInstaller {
-
 	return &BundleInstaller{
 		ctx:              ctx,
 		manifest:         manifest,
@@ -43,7 +42,6 @@ func NewPrompter() *ux.PromptUIPrompter {
 }
 
 func (bi *BundleInstaller) InstallStopOnError(bundle *recipes.Bundle, assumeYes bool) error {
-
 	bi.reportBundleStatus(bundle)
 
 	installableBundleRecipes := bi.getInstallableBundleRecipes(bundle)
@@ -53,7 +51,6 @@ func (bi *BundleInstaller) InstallStopOnError(bundle *recipes.Bundle, assumeYes
 
 	for _, br := range installableBundleRecipes {
 		err := bi.InstallBundleRecipe(br, assumeYes)
-
 		if err != nil {
 			return err
 		}
@@ -81,7 +78,6 @@ func (bi *BundleInstaller) InstallContinueOnError(bundle *recipes.Bundle, assume
 		prompter := ux.NewPromptUIPrompter()
 		msg := "Continue installing? "
 		isConfirmed, err := prompter.PromptYesNo(msg)
-
 		if err != nil {
 			log.Debug(err)
 			isConfirmed = false
@@ -106,10 +102,6 @@ func (bi *BundleInstaller) InstallContinueOnError(bundle *recipes.Bundle, assume
 
 func (bi *BundleInstaller) reportBundleStatus(bundle *recipes.Bundle) {
 	for _, recipe := range bundle.BundleRecipes {
-		if utils.StringInSlice(types.SuperAgentRecipeName, recipe.Recipe.Dependencies) && recipe.HasStatus(execution.RecipeStatusTypes.INSTALLED) {
-			bi.installedRecipes[recipe.Recipe.Name] = true
-			continue
-		}
 		if bi.installedRecipes[recipe.Recipe.Name] {
 			continue
 		}
@@ -135,6 +127,9 @@ func (bi *BundleInstaller) InstallBundleRecipe(bundleRecipe *recipes.BundleRecip
 	}
 
 	recipeName := bundleRecipe.Recipe.Name
+	if strings.EqualFold(recipeName, types.SuperAgentRecipeName) && bundleRecipe.HasStatus(execution.RecipeStatusTypes.INSTALLED) {
+		return nil
+	}
 	if bi.installedRecipes[bundleRecipe.Recipe.Name] {
 		return nil
 	}
@@ -159,7 +154,7 @@ func (bi *BundleInstaller) getInstallableBundleRecipes(bundle *recipes.Bundle) [
 	var bundleRecipes []*recipes.BundleRecipe
 
 	for _, bundleRecipe := range bundle.BundleRecipes {
-		if !bundleRecipe.HasStatus(execution.RecipeStatusTypes.AVAILABLE) {
+		if !bundleRecipe.LastStatus(execution.RecipeStatusTypes.AVAILABLE) {
 			//Skip if not available
 			continue
 		}

internal/install/bundle_installer_test.go

@@ -51,6 +51,16 @@ func TestInstallContinueOnErrorReturnsImmediatelyWhenNoIsEntered(t *testing.T) {
 	test.mockStatusReporter.AssertNumberOfCalls(t, "ReportStatus", 1)
 }
 
+func TestInstallContinueOnErrorReturnsImmediatelyWhenSuperAgentIsInstalled(t *testing.T) {
+	test := createBundleInstallerTest().withPrompterYesNoVal(false).withRecipeInstallerError()
+	test.addRecipeToBundle("super-agent", execution.RecipeStatusTypes.INSTALLED)
+
+	test.BundleInstaller.InstallContinueOnError(test.bundle, false)
+
+	test.mockRecipeInstaller.AssertNumberOfCalls(t, "executeAndValidateWithProgress", 0)
+	test.mockStatusReporter.AssertNumberOfCalls(t, "ReportStatus", 1)
+}
+
 func TestInstallContinueOnErrorIgnoresUxPromptIfBundleIsAdditionalTargeted(t *testing.T) {
 	test := createBundleInstallerTest().withRecipeInstallerError()
 	test.addRecipeToBundle("recipe1", execution.RecipeStatusTypes.UNSUPPORTED)

internal/install/recipe_installer.go

@@ -24,9 +24,7 @@ import (
 	"github.com/newrelic/newrelic-client-go/v2/newrelic"
 )
 
-const (
-	validationTimeout = 5 * time.Minute
-)
+const validationTimeout = 5 * time.Minute
 
 var infraAgentEntityKey string
 
@@ -209,10 +207,6 @@ Our Data Privacy Notice: https://newrelic.com/termsandconditions/services-notice
 		return err
 	}
 
-	// Check if super-agent process is already running on host
-	superAgentProcessOnHost := i.processEvaluator.FindProcess(types.SuperAgentProcessName)
-	log.Debugf("super agent running: %t\n", superAgentProcessOnHost)
-
 	hostname, _ := os.Hostname()
 	if hostname == "" {
 		message := "This system is not supported for automatic installation, no host info. Please see our documentation for requirements."
@@ -412,40 +406,26 @@ func (i *RecipeInstall) isTargetInstallRecipe(recipeName string) bool {
 	return false
 }
 
+// installAdditionalBundle installs additional bundles for the given recipes.
+// It checks if the host has a super agent process running, and proceeds with the additional bundle.
+// If the list of recipes is provided, it creates a targeted bundle; otherwise, it creates a guided bundle.
+// If the host has super agent installed infra agent and logs agent would be UNSUPPORTED
+// It then installs the additional bundle and reports any unsupported recipes.
 func (i *RecipeInstall) installAdditionalBundle(bundler RecipeBundler, bundleInstaller RecipeBundleInstaller, repo *recipes.RecipeRepository) error {
 	var additionalBundle *recipes.Bundle
-	if i.processEvaluator.FindProcess(types.SuperAgentProcessName) {
+	if i.hostHasSuperAgentProcess() {
 		if bun, ok := bundler.(*recipes.Bundler); ok {
 			bun.HasSuperInstalled = true
 			log.Debugf("Super agent process found. Proceeding with additional bundle.")
 		}
 	} else {
 		log.Debugf("Super agent process not found. Proceeding with additional bundle.")
 	}
-	//if bundler.(*recipes.Bundler).HasSuperInstalled {
-	//	// recipe name should not be infra & logs
-	//	// && i.RecipeNames
-	//	for _, r := range i.RecipeNames {
-	//		if r == types.InfraAgentRecipeName || r == types.LoggingRecipeName {
-	//			fmt.Printf("Super agent process found on the host. Skipping installation.")
-	//			// todo:
-	//			return nil
-	//		}
-	//		if r == types.SuperAgentRecipeName {
-	//			// FIXME: Provide the logic for the super agent present case
-	//		}
-	//	}
-	//}
-	// In the above the bundle is not yet created for -n recipe-a, infra , log
-	// so instead create the bundle
-	// have a separate method to deal with the installed super agent
-	// if the list of the recipes provided in the bundle and super agent installed
-	// mark the status as unsupported || skipped
+
 	if i.RecipeNamesProvided() {
 		log.Debugf("bundling addtional bundle")
 		log.Debugf("recipes in list %d", len(i.RecipeNames))
 		additionalBundle = bundler.CreateAdditionalTargetedBundle(i.RecipeNames)
-		// if addtional bundle has super and
 		if bundler.(*recipes.Bundler).HasSuperInstalled {
 			for _, coreRecipe := range bundler.(*recipes.Bundler).GetCoreRecipeNames() {
 				if i, ok := additionalBundle.ContainsName(coreRecipe); ok {
@@ -478,7 +458,7 @@ func (i *RecipeInstall) installAdditionalBundle(bundler RecipeBundler, bundleIns
 }
 
 func (i *RecipeInstall) installCoreBundle(bundler RecipeBundler, bundleInstaller RecipeBundleInstaller) error {
-	if i.processEvaluator.FindProcess(types.SuperAgentProcessName) {
+	if i.hostHasSuperAgentProcess() {
 		if bun, ok := bundler.(*recipes.Bundler); ok {
 			bun.HasSuperInstalled = true
 			log.Debugf("Super agent process found")
@@ -795,3 +775,7 @@ func (i *RecipeInstall) finishHandlingFailure(recipeName string) {
 		i.progressIndicator.Success("Complete!")
 	}
 }
+
+func (i *RecipeInstall) hostHasSuperAgentProcess() bool {
+	return i.processEvaluator.FindProcess(types.SuperAgentProcessName)
+}

internal/install/recipe_installer_test.go

@@ -8,11 +8,10 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/newrelic/newrelic-cli/internal/config"
-
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 
+	"github.com/newrelic/newrelic-cli/internal/config"
 	"github.com/newrelic/newrelic-cli/internal/install/execution"
 	"github.com/newrelic/newrelic-cli/internal/install/recipes"
 	"github.com/newrelic/newrelic-cli/internal/install/types"
@@ -100,7 +99,6 @@ func TestInstallGuidedShouldSkipCoreInstallWhileSuperAgentIsInstalled(t *testing
 
 	err := recipeInstall.Install()
 
-	//fmt.Print(err)
 	assert.Equal(t, "no recipes were installed", err.Error(), "no recipe installed")
 	assert.Equal(t, 1, statusReporter.RecipeDetectedCallCount, "Detection Count")
 	assert.Equal(t, 0, statusReporter.RecipeAvailableCallCount, "Available Count")
@@ -337,6 +335,32 @@ func TestInstallTargetedInstallShouldInstallCoreIfCoreWasSkipped(t *testing.T) {
 	assert.Equal(t, 1, statusReporter.ReportInstalled[r.Recipe.Name], "Recipe1 Installed")
 }
 
+func TestInstallTargetedInstallShouldInstallCoreIfCoreWasSkippedWhileSuperAgentIsInstalled(t *testing.T) {
+	r := &recipes.RecipeDetectionResult{
+		Recipe: recipes.NewRecipeBuilder().Name(types.InfraAgentRecipeName).Build(),
+		Status: execution.RecipeStatusTypes.AVAILABLE,
+	}
+	statusReporter := execution.NewMockStatusReporter()
+	recipeInstall := NewRecipeInstallBuilder().WithRecipeDetectionResult(r).withShouldInstallCore(func() bool { return false }).
+		WithTargetRecipeName(types.InfraAgentRecipeName).WithStatusReporter(statusReporter).
+		WithRunningProcess("super-agent-process", types.SuperAgentProcessName).Build()
+	recipeInstall.AssumeYes = true
+
+	err := recipeInstall.Install()
+
+	assert.Equal(t, "no recipes were installed", err.Error())
+	assert.Equal(t, 1, statusReporter.RecipeDetectedCallCount, "Detection Count")
+	assert.Equal(t, 1, statusReporter.RecipeAvailableCallCount, "Available Count")
+	assert.Equal(t, 0, statusReporter.RecipeInstallingCallCount, "Installing Count")
+	assert.Equal(t, 0, statusReporter.RecipeFailedCallCount, "Failed Count")
+	assert.Equal(t, 1, statusReporter.RecipeUnsupportedCallCount, "Unsupported Count")
+	assert.Equal(t, 0, statusReporter.RecipeInstalledCallCount, "InstalledCount")
+	assert.Equal(t, 0, statusReporter.RecipeRecommendedCallCount, "Recommendation Count")
+	assert.Equal(t, 0, statusReporter.RecipeSkippedCallCount, "Skipped Count")
+	assert.Equal(t, 0, statusReporter.RecipeCanceledCallCount, "Cancelled Count")
+	assert.Equal(t, 0, statusReporter.ReportInstalled[r.Recipe.Name], "Recipe1 Installed")
+}
+
 func TestInstallTargetedInstallWithoutRecipeShouldNotInstall(t *testing.T) {
 	statusReporter := execution.NewMockStatusReporter()
 	recipeInstall := NewRecipeInstallBuilder().WithTargetRecipeName("Other").WithStatusReporter(statusReporter).Build()
@@ -775,7 +799,6 @@ func TestWhenSingleInstallRunningNoError(t *testing.T) {
 	recipeInstall := NewRecipeInstallBuilder().WithRunningProcess("env=123 newrelic install", "newrelic").Build()
 
 	err := recipeInstall.Install()
-
 	if err != nil {
 		assert.False(t, strings.Contains(err.Error(), "only 1 newrelic install command can run at one time"))
 	}
@@ -794,7 +817,6 @@ func TestWhenSingleInstallRunningNoErrorWindows(t *testing.T) {
 	recipeInstall := NewRecipeInstallBuilder().WithRunningProcess("env=123 C:\\path\\newrelic.exe install", "C:\\path\\newrelic.exe").Build()
 
 	err := recipeInstall.Install()
-
 	if err != nil {
 		assert.False(t, strings.Contains(err.Error(), "only 1 newrelic install command can run at one time"))
 	}

internal/install/recipes/bundle_recipe.go

@@ -36,6 +36,13 @@ func (br *BundleRecipe) HasStatus(status execution.RecipeStatusType) bool {
 	return false
 }
 
+func (br *BundleRecipe) LastStatus(status execution.RecipeStatusType) bool {
+	if len(br.DetectedStatuses) > 0 {
+		return br.DetectedStatuses[len(br.DetectedStatuses)-1].Status == status
+	}
+	return false
+}
+
 func (br *BundleRecipe) AreAllDependenciesAvailable() bool {
 	for _, depName := range br.Recipe.Dependencies {
 		if br.IsNameInDependencies(depName) {

internal/install/recipes/bundler.go

@@ -61,8 +61,8 @@ func (b *Bundler) GetCoreRecipeNames() []string {
 	return coreRecipeNames
 }
 
-// TODO: Add doc for the following function
-// dependency match - agent running
+// createBundle creates a new bundle based on the given recipes and bundle type
+// It iterates over the recipes, detects dependencies, and adds recipes to the bundle
 func (b *Bundler) createBundle(recipes []string, bType BundleType) *Bundle {
 	bundle := &Bundle{Type: bType}
 
@@ -83,14 +83,12 @@ func (b *Bundler) createBundle(recipes []string, bType BundleType) *Bundle {
 
 			if bundleRecipe != nil {
 				for _, recipe := range recipes {
-					// TODo: better to do it after making it as a bundle
 					for _, dependency := range bundleRecipe.Dependencies {
 						if dependency.Recipe.Name != recipe {
 							log.Debugf("Found dependency %s", dependency.Recipe.Name)
-							for _, brDeps := range bundleRecipe.Dependencies {
-								if brDeps.Recipe.Name == types.SuperAgentRecipeName {
-									brDeps.AddDetectionStatus(execution.RecipeStatusTypes.INSTALLED, 0)
-								}
+							if dep, ok := findRecipeDependency(dependency, types.SuperAgentRecipeName); ok {
+								log.Debugf("updating the dependency status for %s", dep)
+								dep.AddDetectionStatus(execution.RecipeStatusTypes.INSTALLED, 0)
 							}
 						}
 					}
@@ -105,10 +103,24 @@ func (b *Bundler) createBundle(recipes []string, bType BundleType) *Bundle {
 	return bundle
 }
 
-// getBundleRecipeWithDependencies retrieves a bundle recipe with its resolved dependencies.
-// Parameters: recipe (types.OpenInstallationRecipe): The OpenInstallationRecipe object representing the bundle recipe.
-// Returns: *BundleRecipe: A pointer to a BundleRecipe object containing the recipe and its resolved dependencies,
-// or nil if there are missing dependencies or errors.
+// findRecipeDependency recursively searches for a recipe dependency
+func findRecipeDependency(recipe *BundleRecipe, name string) (*BundleRecipe, bool) {
+	for _, dep := range recipe.Dependencies {
+		if strings.EqualFold(dep.Recipe.Name, name) {
+			return dep, true
+		}
+		found, _ := findRecipeDependency(dep, name)
+		if found != nil {
+			return found, true
+		}
+	}
+	return nil, false
+}
+
+// getBundleRecipeWithDependencies returns a BundleRecipe for the given recipe, including its dependencies.
+// If the recipe is already cached, it is returned immediately. Otherwise, the function recursively
+// fetches the dependencies and builds the BundleRecipe. If any dependencies are missing, the
+// bundle recipe is invalidated and nil is returned.
 func (b *Bundler) getBundleRecipeWithDependencies(recipe *types.OpenInstallationRecipe) *BundleRecipe {
 	if br, ok := b.cachedBundleRecipes[recipe.Name]; ok {
 		return br
@@ -146,6 +158,7 @@ func (b *Bundler) getBundleRecipeWithDependencies(recipe *types.OpenInstallation
 	}
 
 	b.cachedBundleRecipes[recipe.Name] = nil
+	log.Debugf("Returning nil for the recipe: %s", recipe.Name)
 	return nil
 }
 
@@ -172,7 +185,10 @@ func detectDependencies(deps []string) (string, bool) {
 // updateDependency updates a recipe's dependency with the first one of the form 'recipe-a || recipe-b' that is found in the targeted
 // recipes (e.g.: newrelic install -n recipe-a,recipe-c). If none of the recipe's dependency in the form 'recipe-a || recipe-b' are found
 // in the targeted recipes, the first one of those in that same form 'recipe-a || recipe-b' is used. The final result is that recipe's
-// dependency will change from the form 'recipe-a || recipe-b' to, for example, 'recipe-a' only.
+// dependency will change from the form 'recipe-a || recipe-b' to the first recipe in that list, for example, 'recipe-a' only.
+// If the dependency is a super dependency (i.e., '||' is present), and the super dependency is installed, the function will return the super dependency.
+// If the dependency is not a super dependency and is found in the targeted recipes, the function will return that dependency.
+// If the dependency is not a super dependency and is not found in the targeted recipes, the function will return the first part of the dependency.
 func (b *Bundler) updateDependency(dualDep string, recipes []string) []string {
 	var (
 		splitDeps   = strings.Split(dualDep, `||`)
@@ -182,7 +198,6 @@ func (b *Bundler) updateDependency(dualDep string, recipes []string) []string {
 	if len(splitDeps) <= 1 {
 		return nil
 	}
-	// TODO: Update the doc
 	for _, dep := range splitDeps {
 		dep = strings.TrimSpace(dep)
 		if dep == types.SuperAgentRecipeName {

internal/install/recipes/bundler_test.go

@@ -29,7 +29,6 @@ func TestCreateAdditionalTargetedBundleShouldNotSkipCoreRecipes(t *testing.T) {
 	goldenRecipe := NewRecipeBuilder().Name(types.GoldenRecipeName).Build()
 	mysqlRecipe := NewRecipeBuilder().Name("mysql").Build()
 	bundler := createTestBundler()
-	// Todo: fix me
 	bundler.HasSuperInstalled = true
 	withAvailableRecipe(bundler, types.InfraAgentRecipeName, execution.RecipeStatusTypes.AVAILABLE, infraRecipe)
 	withAvailableRecipe(bundler, types.LoggingRecipeName, execution.RecipeStatusTypes.AVAILABLE, loggingRecipe)