Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sd 854 parallelize fetching of argocd diff in telefonistka #43

Merged
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
/telefonistka
vendor/
internal/pkg/mocks/argocd_settings.go
internal/pkg/mocks/argocd_project.go
32 changes: 16 additions & 16 deletions internal/pkg/argocd/argocd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))

Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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)
hnnsgstfssn marked this conversation as resolved.
Show resolved Hide resolved
}(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
}
Expand All @@ -572,6 +573,5 @@ func GenerateDiffOfChangedComponents(ctx context.Context, componentsToDiff map[s
}
diffResults = append(diffResults, currentDiffResult)
}

return hasComponentDiff, hasComponentDiffErrors, diffResults, err
}
121 changes: 121 additions & 0 deletions internal/pkg/argocd/argocd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
ashvarts marked this conversation as resolved.
Show resolved Hide resolved
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)
hnnsgstfssn marked this conversation as resolved.
Show resolved Hide resolved
}
7 changes: 6 additions & 1 deletion internal/pkg/githubapi/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 4 additions & 0 deletions internal/pkg/mocks/mocks.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,7 @@ package mocks
// This package contains generated mocks

//go:generate go run github.com/golang/mock/[email protected] -destination=argocd_application.go -package=mocks github.com/argoproj/argo-cd/v2/pkg/apiclient/application ApplicationServiceClient

//go:generate go run github.com/golang/mock/[email protected] -destination=argocd_settings.go -package=mocks github.com/argoproj/argo-cd/v2/pkg/apiclient/settings SettingsServiceClient

//go:generate go run github.com/golang/mock/[email protected] -destination=argocd_project.go -package=mocks github.com/argoproj/argo-cd/v2/pkg/apiclient/project ProjectServiceClient
Loading