Skip to content

Commit

Permalink
fix: cherry-pick argoproj#18761 (v2.12) (argoproj#19109)
Browse files Browse the repository at this point in the history
* fix(applicationset): use requeue after if generate app errors out (argoproj#18761)

The `GenerateApplications` can call to external resources like Github
API for instance which might be rate limited or fail. If those requests
somehow fail we should requeue them after some time like (same
reason as https://github.com/argoproj/argo-cd/blob/e98d3b2a877fb7140880dc1450703a99db4398b5/applicationset/controllers/applicationset_controller.go#L154).

For instance, in our environments we were rate limited by Github and the ArgoCD
applicationset controller was logging the following error about every
second or less for every application set using the pull request generator
that we have:
```
time="2024-06-21T14:17:15Z" level=error msg="error generating params" error="error listing repos: error listing pull requests for LedgerHQ/xxx: GET https://api.github.com/repos/LedgerHQ/xxx/pulls?per_page=100: 403 API rate limit exceeded for installation ID xxx. If you reach out to GitHub Support for help, please include the request ID xxx and timestamp 2024-06-xx xxx UTC. [rate reset in 8m18s]" generator="&{0xc000d652c0 0x289a100 {0xc00087bdd0}  [] true}"
time="2024-06-21T14:17:15Z" level=error msg="error generating application from params" applicationset=argocd/xxx error="error listing repos: error listing pull requests for LedgerHQ/xxxx: GET https://api.github.com/repos/LedgerHQ/xxx/pulls?per_page=100: 403 API rate limit exceeded for installation ID xxx. If you reach out to GitHub Support for help, please include the request ID xxx and timestamp 2024-06-xx xxx UTC. [rate reset in 8m18s]" generator="{nil nil nil nil nil &PullRequestGenerator{Github:&PullRequestGeneratorGithub{Owner:LedgerHQ,Repo:xxx,API:,TokenRef:nil,AppSecretName:xxxx,Labels:[argocd/preview],},GitLab:nil,Gitea:nil,BitbucketServer:nil,Filters:[]PullRequestGeneratorFilter{},RequeueAfterSeconds:*1800,Template:ApplicationSetTemplate{ApplicationSetTemplateMeta:ApplicationSetTemplateMeta{Name:,Namespace:,Labels:map[string]string{},Annotations:map[string]string{},Finalizers:[],},Spec:ApplicationSpec{Source:nil,Destination:ApplicationDestination{Server:,Namespace:,Name:,},Project:,SyncPolicy:nil,IgnoreDifferences:[]ResourceIgnoreDifferences{},Info:[]Info{},RevisionHistoryLimit:nil,Sources:[]ApplicationSource{},},},Bitbucket:nil,AzureDevOps:nil,} nil nil nil nil}"
```

Signed-off-by: Arthur Outhenin-Chalandre <[email protected]>

* test: cherry-pick fixes

Signed-off-by: Blake Pettersson <[email protected]>

* chore: please the linter

Signed-off-by: Blake Pettersson <[email protected]>

---------

Signed-off-by: Arthur Outhenin-Chalandre <[email protected]>
Signed-off-by: Blake Pettersson <[email protected]>
Co-authored-by: Arthur Outhenin-Chalandre <[email protected]>
  • Loading branch information
blakepettersson and MrFreezeex committed Jul 19, 2024
1 parent 0704aa6 commit cec8504
Show file tree
Hide file tree
Showing 4 changed files with 157 additions and 1 deletion.
2 changes: 1 addition & 1 deletion applicationset/controllers/applicationset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func (r *ApplicationSetReconciler) Reconcile(ctx context.Context, req ctrl.Reque
Status: argov1alpha1.ApplicationSetConditionStatusTrue,
}, parametersGenerated,
)
return ctrl.Result{}, err
return ctrl.Result{RequeueAfter: ReconcileRequeueOnValidationError}, err
}

parametersGenerated = true
Expand Down
54 changes: 54 additions & 0 deletions applicationset/controllers/applicationset_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"testing"
"time"

"github.com/argoproj/argo-cd/v2/applicationset/generators/mocks"

log "github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
Expand Down Expand Up @@ -2139,6 +2141,58 @@ func TestGetMinRequeueAfter(t *testing.T) {
assert.Equal(t, time.Duration(1)*time.Second, got)
}

func TestRequeueGeneratorFails(t *testing.T) {
scheme := runtime.NewScheme()
err := v1alpha1.AddToScheme(scheme)
require.NoError(t, err)
err = v1alpha1.AddToScheme(scheme)
require.NoError(t, err)

appSet := v1alpha1.ApplicationSet{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Namespace: "argocd",
},
Spec: v1alpha1.ApplicationSetSpec{
Generators: []v1alpha1.ApplicationSetGenerator{{
PullRequest: &v1alpha1.PullRequestGenerator{},
}},
},
}
client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&appSet).Build()

generator := v1alpha1.ApplicationSetGenerator{
PullRequest: &v1alpha1.PullRequestGenerator{},
}

generatorMock := mocks.Generator{}
generatorMock.On("GetTemplate", &generator).
Return(&v1alpha1.ApplicationSetTemplate{})
generatorMock.On("GenerateParams", &generator, mock.AnythingOfType("*v1alpha1.ApplicationSet"), mock.Anything).
Return([]map[string]interface{}{}, fmt.Errorf("Simulated error generating params that could be related to an external service/API call"))

r := ApplicationSetReconciler{
Client: client,
Scheme: scheme,
Recorder: record.NewFakeRecorder(0),
Cache: &fakeCache{},
Generators: map[string]generators.Generator{
"PullRequest": &generatorMock,
},
}

req := ctrl.Request{
NamespacedName: types.NamespacedName{
Namespace: "argocd",
Name: "name",
},
}

res, err := r.Reconcile(context.Background(), req)
require.Error(t, err)
assert.Equal(t, ReconcileRequeueOnValidationError, res.RequeueAfter)
}

func TestValidateGeneratedApplications(t *testing.T) {
scheme := runtime.NewScheme()
err := v1alpha1.AddToScheme(scheme)
Expand Down
2 changes: 2 additions & 0 deletions applicationset/generators/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
argoprojiov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
)

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

// Generator defines the interface implemented by all ApplicationSet generators.
type Generator interface {
// GenerateParams interprets the ApplicationSet and generates all relevant parameters for the application template.
Expand Down
100 changes: 100 additions & 0 deletions applicationset/generators/mocks/Generator.go

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

0 comments on commit cec8504

Please sign in to comment.