diff --git a/.gitignore b/.gitignore index 386d5d7d..eaa026f9 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,4 @@ /telefonistka vendor/ +internal/pkg/mocks/argocd_settings.go +internal/pkg/mocks/argocd_project.go diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go index 5038521b..074ce615 100644 --- a/internal/pkg/argocd/argocd.go +++ b/internal/pkg/argocd/argocd.go @@ -172,7 +172,7 @@ func getEnv(key, fallback string) string { return fallback } -func createArgoCdClients() (ac argoCdClients, err error) { +func CreateArgoCdClients() (ac argoCdClients, err error) { plaintext, _ := strconv.ParseBool(getEnv("ARGOCD_PLAINTEXT", "false")) insecure, _ := strconv.ParseBool(getEnv("ARGOCD_INSECURE", "false")) @@ -320,7 +320,7 @@ func findArgocdApp(ctx context.Context, componentPath string, repo string, appCl func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision string, repo string, useSHALabelForArgoDicovery bool) error { var foundApp *argoappv1.Application var err error - ac, err := createArgoCdClients() + ac, err := CreateArgoCdClients() if err != nil { return fmt.Errorf("Error creating ArgoCD clients: %w", err) } @@ -461,7 +461,7 @@ func generateDiffOfAComponent(ctx context.Context, commentDiff bool, componentPa componentDiffResult.AppWasTemporarilyCreated = true } } else { - componentDiffResult.DiffError = fmt.Errorf("No ArgoCD application found for component path %s(repo %s)", componentPath, repo) + componentDiffResult.DiffError = fmt.Errorf("no ArgoCD application found for component path %s(repo %s)", componentPath, repo) return } } else { @@ -472,7 +472,7 @@ func generateDiffOfAComponent(ctx context.Context, commentDiff bool, componentPa Name: &app.Name, // we expect only one app with this label and repo selectors Refresh: &refreshType, } - app, err := ac.app.Get(ctx, &appNameQuery) + app, err = ac.app.Get(ctx, &appNameQuery) if err != nil { componentDiffResult.DiffError = err log.Errorf("Error getting app(HardRefresh) %v: %v", appNameQuery.Name, err) @@ -544,26 +544,27 @@ func generateDiffOfAComponent(ctx context.Context, commentDiff bool, componentPa } // GenerateDiffOfChangedComponents generates diff of changed components -func GenerateDiffOfChangedComponents(ctx context.Context, componentsToDiff map[string]bool, prBranch string, repo string, useSHALabelForArgoDicovery bool, createTempAppObjectFromNewApps bool) (hasComponentDiff bool, hasComponentDiffErrors bool, diffResults []DiffResult, err error) { +func GenerateDiffOfChangedComponents(ctx context.Context, componentsToDiff map[string]bool, prBranch string, repo string, useSHALabelForArgoDicovery bool, createTempAppObjectFromNewApps bool, argoClients argoCdClients) (hasComponentDiff bool, hasComponentDiffErrors bool, diffResults []DiffResult, err error) { hasComponentDiff = false hasComponentDiffErrors = false - // env var should be centralized - ac, err := createArgoCdClients() - if err != nil { - log.Errorf("Error creating ArgoCD clients: %v", err) - return false, true, nil, err - } - argoSettings, err := ac.setting.Get(ctx, &settings.SettingsQuery{}) + argoSettings, err := argoClients.setting.Get(ctx, &settings.SettingsQuery{}) if err != nil { - log.Errorf("Error getting ArgoCD settings: %v", err) + log.Errorf("error getting ArgoCD settings: %v", err) return false, true, nil, err } + diffResult := make(chan DiffResult) for componentPath, shouldIDiff := range componentsToDiff { - currentDiffResult := generateDiffOfAComponent(ctx, shouldIDiff, componentPath, prBranch, repo, ac, argoSettings, useSHALabelForArgoDicovery, createTempAppObjectFromNewApps) + go func(componentPath string, shouldDiff bool) { + diffResult <- generateDiffOfAComponent(ctx, shouldIDiff, componentPath, prBranch, repo, argoClients, argoSettings, useSHALabelForArgoDicovery, createTempAppObjectFromNewApps) + }(componentPath, shouldIDiff) + } + + for range componentsToDiff { + currentDiffResult := <-diffResult if currentDiffResult.DiffError != nil { - log.Errorf("Error generating diff for component %s: %v", componentPath, currentDiffResult.DiffError) + log.Errorf("Error generating diff for component %s: %v", currentDiffResult.ComponentPath, currentDiffResult.DiffError) hasComponentDiffErrors = true err = currentDiffResult.DiffError } @@ -572,6 +573,5 @@ func GenerateDiffOfChangedComponents(ctx context.Context, componentsToDiff map[s } diffResults = append(diffResults, currentDiffResult) } - return hasComponentDiff, hasComponentDiffErrors, diffResults, err } diff --git a/internal/pkg/argocd/argocd_test.go b/internal/pkg/argocd/argocd_test.go index 80e094d9..a34766b6 100644 --- a/internal/pkg/argocd/argocd_test.go +++ b/internal/pkg/argocd/argocd_test.go @@ -3,14 +3,21 @@ package argocd import ( "bytes" "context" + "fmt" "log" "os" "strings" "testing" "text/template" + "time" + "github.com/argoproj/argo-cd/v2/pkg/apiclient/application" + "github.com/argoproj/argo-cd/v2/pkg/apiclient/project" + "github.com/argoproj/argo-cd/v2/pkg/apiclient/settings" argoappv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" + reposerverApiClient "github.com/argoproj/argo-cd/v2/reposerver/apiclient" "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" "github.com/wayfair-incubator/telefonistka/internal/pkg/mocks" "github.com/wayfair-incubator/telefonistka/internal/pkg/testutils" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -311,3 +318,117 @@ func TestFindArgocdAppByPathAnnotationNotFound(t *testing.T) { log.Fatal("expected the application to be nil") } } + +func TestFetchArgoDiffConcurrently(t *testing.T) { + t.Parallel() + // MockApplicationServiceClient + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + // mock the argoClients + mockAppServiceClient := mocks.NewMockApplicationServiceClient(mockCtrl) + mockSettingsServiceClient := mocks.NewMockSettingsServiceClient(mockCtrl) + mockProjectServiceClient := mocks.NewMockProjectServiceClient(mockCtrl) + // fake InitArgoClients + + argoClients := argoCdClients{ + app: mockAppServiceClient, + setting: mockSettingsServiceClient, + project: mockProjectServiceClient, + } + // slowReply simulates a slow reply from the server + slowReply := func(ctx context.Context, in any, opts ...any) { + time.Sleep(time.Second) + } + + // makeComponents for test + makeComponents := func(num int) map[string]bool { + components := make(map[string]bool, num) + for i := 0; i < num; i++ { + components[fmt.Sprintf("component/to/diff/%d", i)] = true + } + return components + } + + mockSettingsServiceClient.EXPECT(). + Get(gomock.Any(), gomock.Any()). + Return(&settings.Settings{ + URL: "https://test-argocd.test.test", + }, nil) + // mock the List method + mockAppServiceClient.EXPECT(). + List(gomock.Any(), gomock.Any(), gomock.Any()). + Return(&argoappv1.ApplicationList{ + Items: []argoappv1.Application{ + { + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{}, + Spec: argoappv1.ApplicationSpec{}, + Status: argoappv1.ApplicationStatus{}, + Operation: &argoappv1.Operation{}, + }, + }, + }, nil). + AnyTimes(). + Do(slowReply) // simulate slow reply + + // mock the Get method + mockAppServiceClient.EXPECT(). + Get(gomock.Any(), gomock.Any()). + Return(&argoappv1.Application{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-app", + }, + Spec: argoappv1.ApplicationSpec{ + Source: &argoappv1.ApplicationSource{ + TargetRevision: "test-revision", + }, + SyncPolicy: &argoappv1.SyncPolicy{ + Automated: &argoappv1.SyncPolicyAutomated{}, + }, + }, + Status: argoappv1.ApplicationStatus{}, + Operation: &argoappv1.Operation{}, + }, nil). + AnyTimes() + + // mock managedResource + mockAppServiceClient.EXPECT(). + ManagedResources(gomock.Any(), gomock.Any()). + Return(&application.ManagedResourcesResponse{}, nil). + AnyTimes() + + // mock the GetManifests method + mockAppServiceClient.EXPECT(). + GetManifests(gomock.Any(), gomock.Any()). + Return(&reposerverApiClient.ManifestResponse{}, nil). + AnyTimes() + + // mock the GetDetailedProject method + mockProjectServiceClient.EXPECT(). + GetDetailedProject(gomock.Any(), gomock.Any()). + Return(&project.DetailedProjectsResponse{}, nil). + AnyTimes() + + const numComponents = 5 + // start timer + start := time.Now() + + // TODO: Test all the return values, for now we will just ignore the linter. + _, _, diffResults, _ := GenerateDiffOfChangedComponents( //nolint:dogsled + context.TODO(), + makeComponents(numComponents), + "test-pr-branch", + "test-repo", + true, + false, + argoClients, + ) + + // stop timer + elapsed := time.Since(start) + assert.Equal(t, numComponents, len(diffResults)) + // assert that the entire run takes less than numComponents * 1 second + assert.Less(t, elapsed, time.Duration(numComponents)*time.Second) +} diff --git a/internal/pkg/githubapi/github.go b/internal/pkg/githubapi/github.go index f8372997..5f128c98 100644 --- a/internal/pkg/githubapi/github.go +++ b/internal/pkg/githubapi/github.go @@ -224,7 +224,12 @@ func handleChangedPREvent(ctx context.Context, mainGithubClientPair GhClientPair ghPrClientDetails.PrLogger.Debugf("ArgoCD diff disabled for %s\n", componentPath) } } - hasComponentDiff, hasComponentDiffErrors, diffOfChangedComponents, err := argocd.GenerateDiffOfChangedComponents(ctx, componentsToDiff, ghPrClientDetails.Ref, ghPrClientDetails.RepoURL, config.Argocd.UseSHALabelForAppDiscovery, config.Argocd.CreateTempAppObjectFroNewApps) + argoClients, err := argocd.CreateArgoCdClients() + if err != nil { + return fmt.Errorf("error creating ArgoCD clients: %w", err) + } + + hasComponentDiff, hasComponentDiffErrors, diffOfChangedComponents, err := argocd.GenerateDiffOfChangedComponents(ctx, componentsToDiff, ghPrClientDetails.Ref, ghPrClientDetails.RepoURL, config.Argocd.UseSHALabelForAppDiscovery, config.Argocd.CreateTempAppObjectFroNewApps, argoClients) if err != nil { return fmt.Errorf("getting diff information: %w", err) } diff --git a/internal/pkg/mocks/mocks.go b/internal/pkg/mocks/mocks.go index 0fbed2e1..42b1e1e6 100644 --- a/internal/pkg/mocks/mocks.go +++ b/internal/pkg/mocks/mocks.go @@ -3,3 +3,7 @@ package mocks // This package contains generated mocks //go:generate go run github.com/golang/mock/mockgen@v1.6.0 -destination=argocd_application.go -package=mocks github.com/argoproj/argo-cd/v2/pkg/apiclient/application ApplicationServiceClient + +//go:generate go run github.com/golang/mock/mockgen@v1.6.0 -destination=argocd_settings.go -package=mocks github.com/argoproj/argo-cd/v2/pkg/apiclient/settings SettingsServiceClient + +//go:generate go run github.com/golang/mock/mockgen@v1.6.0 -destination=argocd_project.go -package=mocks github.com/argoproj/argo-cd/v2/pkg/apiclient/project ProjectServiceClient