Skip to content

Commit

Permalink
fix(appset): Don't use revision cache when reconciling after webhook (#…
Browse files Browse the repository at this point in the history 
…16062) (#16241) (#16543)

* fix(appset): store sha from webhook to get latest change during reconcile (#16062)



* fix(appset): Don't use revision cache when reconciling after webhook(#16062)



---------

Signed-off-by: dhruvang1 <[email protected]>
Co-authored-by: Dhruvang Makadia <[email protected]>
  • Loading branch information
@alexmt @dhruvang1
alexmt and dhruvang1 authored Dec 5, 2023
1 parent b84b143 commit 40fb89f
Show file tree
Hide file tree
Showing 9 changed files with 276 additions and 181 deletions.
14 changes: 8 additions & 6 deletions applicationset/generators/git.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,14 @@ func (g *GitGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.Applic
return nil, EmptyAppSetGeneratorError
}

noRevisionCache := appSet.RefreshRequired()

var err error
var res []map[string]interface{}
if len(appSetGenerator.Git.Directories) != 0 {
res, err = g.generateParamsForGitDirectories(appSetGenerator, appSet.Spec.GoTemplate, appSet.Spec.GoTemplateOptions)
res, err = g.generateParamsForGitDirectories(appSetGenerator, noRevisionCache, appSet.Spec.GoTemplate, appSet.Spec.GoTemplateOptions)
} else if len(appSetGenerator.Git.Files) != 0 {
res, err = g.generateParamsForGitFiles(appSetGenerator, appSet.Spec.GoTemplate, appSet.Spec.GoTemplateOptions)
res, err = g.generateParamsForGitFiles(appSetGenerator, noRevisionCache, appSet.Spec.GoTemplate, appSet.Spec.GoTemplateOptions)
} else {
return nil, EmptyAppSetGeneratorError
}
Expand All @@ -72,10 +74,10 @@ func (g *GitGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.Applic
return res, nil
}

func (g *GitGenerator) generateParamsForGitDirectories(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator, useGoTemplate bool, goTemplateOptions []string) ([]map[string]interface{}, error) {
func (g *GitGenerator) generateParamsForGitDirectories(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator, noRevisionCache bool, useGoTemplate bool, goTemplateOptions []string) ([]map[string]interface{}, error) {

// Directories, not files
allPaths, err := g.repos.GetDirectories(context.TODO(), appSetGenerator.Git.RepoURL, appSetGenerator.Git.Revision)
allPaths, err := g.repos.GetDirectories(context.TODO(), appSetGenerator.Git.RepoURL, appSetGenerator.Git.Revision, noRevisionCache)
if err != nil {
return nil, err
}
Expand All @@ -98,12 +100,12 @@ func (g *GitGenerator) generateParamsForGitDirectories(appSetGenerator *argoproj
return res, nil
}

func (g *GitGenerator) generateParamsForGitFiles(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator, useGoTemplate bool, goTemplateOptions []string) ([]map[string]interface{}, error) {
func (g *GitGenerator) generateParamsForGitFiles(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator, noRevisionCache bool, useGoTemplate bool, goTemplateOptions []string) ([]map[string]interface{}, error) {

// Get all files that match the requested path string, removing duplicates
allFiles := make(map[string][]byte)
for _, requestedPath := range appSetGenerator.Git.Files {
files, err := g.repos.GetFiles(context.TODO(), appSetGenerator.Git.RepoURL, appSetGenerator.Git.Revision, requestedPath.Path)
files, err := g.repos.GetFiles(context.TODO(), appSetGenerator.Git.RepoURL, appSetGenerator.Git.Revision, requestedPath.Path, noRevisionCache)
if err != nil {
return nil, err
}
Expand Down
8 changes: 4 additions & 4 deletions applicationset/generators/git_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ func TestGitGenerateParamsFromDirectories(t *testing.T) {

argoCDServiceMock := mocks.Repos{}

argoCDServiceMock.On("GetDirectories", mock.Anything, mock.Anything, mock.Anything).Return(testCaseCopy.repoApps, testCaseCopy.repoError)
argoCDServiceMock.On("GetDirectories", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(testCaseCopy.repoApps, testCaseCopy.repoError)

var gitGenerator = NewGitGenerator(&argoCDServiceMock)
applicationSetInfo := argoprojiov1alpha1.ApplicationSet{
Expand Down Expand Up @@ -559,7 +559,7 @@ func TestGitGenerateParamsFromDirectoriesGoTemplate(t *testing.T) {

argoCDServiceMock := mocks.Repos{}

argoCDServiceMock.On("GetDirectories", mock.Anything, mock.Anything, mock.Anything).Return(testCaseCopy.repoApps, testCaseCopy.repoError)
argoCDServiceMock.On("GetDirectories", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(testCaseCopy.repoApps, testCaseCopy.repoError)

var gitGenerator = NewGitGenerator(&argoCDServiceMock)
applicationSetInfo := argoprojiov1alpha1.ApplicationSet{
Expand Down Expand Up @@ -918,7 +918,7 @@ cluster:
t.Parallel()

argoCDServiceMock := mocks.Repos{}
argoCDServiceMock.On("GetFiles", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
argoCDServiceMock.On("GetFiles", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return(testCaseCopy.repoFileContents, testCaseCopy.repoPathsError)

var gitGenerator = NewGitGenerator(&argoCDServiceMock)
Expand Down Expand Up @@ -1268,7 +1268,7 @@ cluster:
t.Parallel()

argoCDServiceMock := mocks.Repos{}
argoCDServiceMock.On("GetFiles", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
argoCDServiceMock.On("GetFiles", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return(testCaseCopy.repoFileContents, testCaseCopy.repoPathsError)

var gitGenerator = NewGitGenerator(&argoCDServiceMock)
Expand Down
2 changes: 1 addition & 1 deletion applicationset/generators/matrix_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1054,7 +1054,7 @@ func TestGitGenerator_GenerateParams_list_x_git_matrix_generator(t *testing.T) {
}

repoServiceMock := &mocks.Repos{}
repoServiceMock.On("GetFiles", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(map[string][]byte{
repoServiceMock.On("GetFiles", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(map[string][]byte{
"some/path.json": []byte("test: content"),
}, nil)
gitGenerator := NewGitGenerator(repoServiceMock)
Expand Down
36 changes: 18 additions & 18 deletions applicationset/services/mocks/Repos.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 10 additions & 4 deletions applicationset/services/repo_service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"github.com/argoproj/argo-cd/v2/util/io"
)

//go:generate go run github.com/vektra/mockery/[email protected] --name=RepositoryDB

// RepositoryDB Is a lean facade for ArgoDB,
// Using a lean interface makes it easier to test the functionality of the git generator
type RepositoryDB interface {
Expand All @@ -25,13 +27,15 @@ type argoCDService struct {
newFileGlobbingEnabled bool
}

//go:generate go run github.com/vektra/mockery/[email protected] --name=Repos

type Repos interface {

// GetFiles returns content of files (not directories) within the target repo
GetFiles(ctx context.Context, repoURL string, revision string, pattern string) (map[string][]byte, error)
GetFiles(ctx context.Context, repoURL string, revision string, pattern string, noRevisionCache bool) (map[string][]byte, error)

// GetDirectories returns a list of directories (not files) within the target repo
GetDirectories(ctx context.Context, repoURL string, revision string) ([]string, error)
GetDirectories(ctx context.Context, repoURL string, revision string, noRevisionCache bool) ([]string, error)
}

func NewArgoCDService(db db.ArgoDB, submoduleEnabled bool, repoClientset apiclient.Clientset, newFileGlobbingEnabled bool) (Repos, error) {
Expand All @@ -43,7 +47,7 @@ func NewArgoCDService(db db.ArgoDB, submoduleEnabled bool, repoClientset apiclie
}, nil
}

func (a *argoCDService) GetFiles(ctx context.Context, repoURL string, revision string, pattern string) (map[string][]byte, error) {
func (a *argoCDService) GetFiles(ctx context.Context, repoURL string, revision string, pattern string, noRevisionCache bool) (map[string][]byte, error) {
repo, err := a.repositoriesDB.GetRepository(ctx, repoURL)
if err != nil {
return nil, fmt.Errorf("error in GetRepository: %w", err)
Expand All @@ -55,6 +59,7 @@ func (a *argoCDService) GetFiles(ctx context.Context, repoURL string, revision s
Revision: revision,
Path: pattern,
NewGitFileGlobbingEnabled: a.newFileGlobbingEnabled,
NoRevisionCache: noRevisionCache,
}
closer, client, err := a.repoServerClientSet.NewRepoServerClient()
if err != nil {
Expand All @@ -69,7 +74,7 @@ func (a *argoCDService) GetFiles(ctx context.Context, repoURL string, revision s
return fileResponse.GetMap(), nil
}

func (a *argoCDService) GetDirectories(ctx context.Context, repoURL string, revision string) ([]string, error) {
func (a *argoCDService) GetDirectories(ctx context.Context, repoURL string, revision string, noRevisionCache bool) ([]string, error) {
repo, err := a.repositoriesDB.GetRepository(ctx, repoURL)
if err != nil {
return nil, fmt.Errorf("error in GetRepository: %w", err)
Expand All @@ -79,6 +84,7 @@ func (a *argoCDService) GetDirectories(ctx context.Context, repoURL string, revi
Repo: repo,
SubmoduleEnabled: a.submoduleEnabled,
Revision: revision,
NoRevisionCache: noRevisionCache,
}

closer, client, err := a.repoServerClientSet.NewRepoServerClient()
Expand Down
28 changes: 15 additions & 13 deletions applicationset/services/repo_service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ func TestGetDirectories(t *testing.T) {
repoServerClientFuncs []func(*repo_mocks.RepoServerServiceClient)
}
type args struct {
ctx context.Context
repoURL string
revision string
ctx context.Context
repoURL string
revision string
noRevisionCache bool
}
tests := []struct {
name string
Expand Down Expand Up @@ -88,11 +89,11 @@ func TestGetDirectories(t *testing.T) {
submoduleEnabled: tt.fields.submoduleEnabled,
repoServerClientSet: &repo_mocks.Clientset{RepoServerServiceClient: mockRepoClient},
}
got, err := a.GetDirectories(tt.args.ctx, tt.args.repoURL, tt.args.revision)
if !tt.wantErr(t, err, fmt.Sprintf("GetDirectories(%v, %v, %v)", tt.args.ctx, tt.args.repoURL, tt.args.revision)) {
got, err := a.GetDirectories(tt.args.ctx, tt.args.repoURL, tt.args.revision, tt.args.noRevisionCache)
if !tt.wantErr(t, err, fmt.Sprintf("GetDirectories(%v, %v, %v, %v)", tt.args.ctx, tt.args.repoURL, tt.args.revision, tt.args.noRevisionCache)) {
return
}
assert.Equalf(t, tt.want, got, "GetDirectories(%v, %v, %v)", tt.args.ctx, tt.args.repoURL, tt.args.revision)
assert.Equalf(t, tt.want, got, "GetDirectories(%v, %v, %v, %v)", tt.args.ctx, tt.args.repoURL, tt.args.revision, tt.args.noRevisionCache)
})
}
}
Expand All @@ -105,10 +106,11 @@ func TestGetFiles(t *testing.T) {
repoServerClientFuncs []func(*repo_mocks.RepoServerServiceClient)
}
type args struct {
ctx context.Context
repoURL string
revision string
pattern string
ctx context.Context
repoURL string
revision string
pattern string
noRevisionCache bool
}
tests := []struct {
name string
Expand Down Expand Up @@ -175,11 +177,11 @@ func TestGetFiles(t *testing.T) {
submoduleEnabled: tt.fields.submoduleEnabled,
repoServerClientSet: &repo_mocks.Clientset{RepoServerServiceClient: mockRepoClient},
}
got, err := a.GetFiles(tt.args.ctx, tt.args.repoURL, tt.args.revision, tt.args.pattern)
if !tt.wantErr(t, err, fmt.Sprintf("GetFiles(%v, %v, %v, %v)", tt.args.ctx, tt.args.repoURL, tt.args.revision, tt.args.pattern)) {
got, err := a.GetFiles(tt.args.ctx, tt.args.repoURL, tt.args.revision, tt.args.pattern, tt.args.noRevisionCache)
if !tt.wantErr(t, err, fmt.Sprintf("GetFiles(%v, %v, %v, %v, %v)", tt.args.ctx, tt.args.repoURL, tt.args.revision, tt.args.pattern, tt.args.noRevisionCache)) {
return
}
assert.Equalf(t, tt.want, got, "GetFiles(%v, %v, %v, %v)", tt.args.ctx, tt.args.repoURL, tt.args.revision, tt.args.pattern)
assert.Equalf(t, tt.want, got, "GetFiles(%v, %v, %v, %v, %v)", tt.args.ctx, tt.args.repoURL, tt.args.revision, tt.args.pattern, tt.args.noRevisionCache)
})
}
}
Expand Down
Loading

0 comments on commit 40fb89f

Please sign in to comment.