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.

Sign up for GitHub

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

feat(appset): add configurable max reconciliations to appset #13381

Merged
merged 3 commits into from
May 5, 2023
Merged

feat(appset): add configurable max reconciliations to appset #13381

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f3cd7bd
feat(appset): add configurable max reconciliations to appset
gdsoumya May 1, 2023
51367b7
fix: lint errors
gdsoumya May 1, 2023
e6d046e
Merge branch 'master' into feat/appset_parallelization
gdsoumya May 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions applicationset/controllers/applicationset_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/handler"
Expand Down Expand Up @@ -518,7 +519,7 @@ func (r *ApplicationSetReconciler) generateApplications(applicationSetInfo argov
return res, applicationSetReason, firstError
}

func (r *ApplicationSetReconciler) SetupWithManager(mgr ctrl.Manager, enableProgressiveSyncs bool) error {
func (r *ApplicationSetReconciler) SetupWithManager(mgr ctrl.Manager, enableProgressiveSyncs bool, maxConcurrentReconciliations int) error {
if err := mgr.GetFieldIndexer().IndexField(context.TODO(), &argov1alpha1.Application{}, ".metadata.controller", func(rawObj client.Object) []string {
// grab the job object, extract the owner...
app := rawObj.(*argov1alpha1.Application)
Expand All @@ -539,8 +540,9 @@ func (r *ApplicationSetReconciler) SetupWithManager(mgr ctrl.Manager, enableProg

ownsHandler := getOwnsHandlerPredicates(enableProgressiveSyncs)

return ctrl.NewControllerManagedBy(mgr).
For(&argov1alpha1.ApplicationSet{}).
return ctrl.NewControllerManagedBy(mgr).WithOptions(controller.Options{
MaxConcurrentReconciles: maxConcurrentReconciliations,
}).For(&argov1alpha1.ApplicationSet{}).
Owns(&argov1alpha1.Application{}, builder.WithPredicates(ownsHandler)).
Watches(
&source.Kind{Type: &corev1.Secret{}},
Expand Down Expand Up @@ -1349,7 +1351,7 @@ func getOwnsHandlerPredicates(enableProgressiveSyncs bool) predicate.Funcs {
return false
}
requeue := shouldRequeueApplicationSet(appOld, appNew, enableProgressiveSyncs)
log.Debugf("requeue: %t caused by application %s", requeue, appNew.Name)
log.Debugf("requeue: %t caused by application %s\n", requeue, appNew.Name)
return requeue
},
GenericFunc: func(e event.GenericEvent) bool {
Expand Down
38 changes: 20 additions & 18 deletions cmd/argocd-applicationset-controller/commands/applicationset_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,20 +47,21 @@ func getSubmoduleEnabled() bool {

func NewCommand() *cobra.Command {
var (
clientConfig clientcmd.ClientConfig
metricsAddr string
probeBindAddr string
webhookAddr string
enableLeaderElection bool
namespace string
argocdRepoServer string
policy string
debugLog bool
dryRun bool
enableProgressiveSyncs bool
repoServerPlaintext bool
repoServerStrictTLS bool
repoServerTimeoutSeconds int
clientConfig clientcmd.ClientConfig
metricsAddr string
probeBindAddr string
webhookAddr string
enableLeaderElection bool
namespace string
argocdRepoServer string
policy string
debugLog bool
dryRun bool
enableProgressiveSyncs bool
repoServerPlaintext bool
repoServerStrictTLS bool
repoServerTimeoutSeconds int
maxConcurrentReconciliations int
)
scheme := runtime.NewScheme()
_ = clientgoscheme.AddToScheme(scheme)
Expand Down Expand Up @@ -194,7 +195,7 @@ func NewCommand() *cobra.Command {
KubeClientset: k8sClient,
ArgoDB: argoCDDB,
EnableProgressiveSyncs: enableProgressiveSyncs,
}).SetupWithManager(mgr, enableProgressiveSyncs); err != nil {
}).SetupWithManager(mgr, enableProgressiveSyncs, maxConcurrentReconciliations); err != nil {
log.Error(err, "unable to create controller", "controller", "ApplicationSet")
os.Exit(1)
}
Expand Down Expand Up @@ -223,9 +224,10 @@ func NewCommand() *cobra.Command {
command.Flags().StringVar(&cmdutil.LogLevel, "loglevel", env.StringFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_LOGLEVEL", "info"), "Set the logging level. One of: debug|info|warn|error")
command.Flags().BoolVar(&dryRun, "dry-run", env.ParseBoolFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_DRY_RUN", false), "Enable dry run mode")
command.Flags().BoolVar(&enableProgressiveSyncs, "enable-progressive-syncs", env.ParseBoolFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_PROGRESSIVE_SYNCS", false), "Enable use of the experimental progressive syncs feature.")
command.Flags().BoolVar(&repoServerPlaintext, "repo-server-plaintext", env.ParseBoolFromEnv("ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER_PLAINTEXT", false), "Disable TLS on connections to repo server")
command.Flags().BoolVar(&repoServerStrictTLS, "repo-server-strict-tls", env.ParseBoolFromEnv("ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER_STRICT_TLS", false), "Whether to use strict validation of the TLS cert presented by the repo server")
command.Flags().IntVar(&repoServerTimeoutSeconds, "repo-server-timeout-seconds", env.ParseNumFromEnv("ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER_TIMEOUT_SECONDS", 60, 0, math.MaxInt64), "Repo server RPC call timeout seconds.")
command.Flags().BoolVar(&repoServerPlaintext, "repo-server-plaintext", env.ParseBoolFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_REPO_SERVER_PLAINTEXT", false), "Disable TLS on connections to repo server")
command.Flags().BoolVar(&repoServerStrictTLS, "repo-server-strict-tls", env.ParseBoolFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_REPO_SERVER_STRICT_TLS", false), "Whether to use strict validation of the TLS cert presented by the repo server")
command.Flags().IntVar(&repoServerTimeoutSeconds, "repo-server-timeout-seconds", env.ParseNumFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_REPO_SERVER_TIMEOUT_SECONDS", 60, 0, math.MaxInt64), "Repo server RPC call timeout seconds.")
command.Flags().IntVar(&maxConcurrentReconciliations, "concurrent-reconciliations", env.ParseNumFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_CONCURRENT_RECONCILIATIONS", 10, 1, 100), "Max concurrent reconciliations limit for the controller")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set the default to 10 and max to 100 for now, let me know if that needs to be changed.

return &command
}

Expand Down
20 changes: 16 additions & 4 deletions manifests/base/applicationset-controller/argocd-applicationset-controller-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,17 +91,29 @@ spec:
key: applicationsetcontroller.enable.progressive.syncs
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER_PLAINTEXT
- name: ARGOCD_APPLICATIONSET_CONTROLLER_REPO_SERVER_PLAINTEXT
valueFrom:
configMapKeyRef:
name: argocd-cmd-params-cm
key: controller.repo.server.plaintext
key: applicationsetcontroller.repo.server.plaintext
Copy link

@vincentgna vincentgna Aug 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add docs for ppl upgrading from 2.7 to 2.8 running without in-cluster TLS (this seems to have broken applicationset controller with error "transport: authentication handshake failed: tls: first record does not look like a TLS handshake" )

didn't see a notice about this cmd param config change here
https://argo-cd.readthedocs.io/en/stable/operator-manual/upgrading/2.7-2.8/

and missing from the sample in docs
https://github.com/argoproj/argo-cd/blob/v2.8.1/docs/operator-manual/argocd-cmd-params-cm.yaml

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#15181

optional: true
- name: ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER_STRICT_TLS
- name: ARGOCD_APPLICATIONSET_CONTROLLER_REPO_SERVER_STRICT_TLS
valueFrom:
configMapKeyRef:
name: argocd-cmd-params-cm
key: controller.repo.server.strict.tls
key: applicationsetcontroller.repo.server.strict.tls
optional: true
- name: ARGOCD_APPLICATIONSET_CONTROLLER_REPO_SERVER_TIMEOUT_SECONDS
valueFrom:
configMapKeyRef:
name: argocd-cmd-params-cm
key: applicationsetcontroller.repo.server.timeout.seconds
optional: true
- name: ARGOCD_APPLICATIONSET_CONTROLLER_CONCURRENT_RECONCILIATIONS
valueFrom:
configMapKeyRef:
name: argocd-cmd-params-cm
key: applicationsetcontroller.concurrent.reconciliations.max
Comment on lines +94 to +116
Copy link
Member Author

@gdsoumya gdsoumya May 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated the env var names from app controller to appset controller and also made them point to appset specific config instead of re-using app controller config.

optional: true
volumeMounts:
- mountPath: /app/config/ssh
Expand Down
20 changes: 16 additions & 4 deletions manifests/core-install.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16700,16 +16700,28 @@ spec:
key: applicationsetcontroller.enable.progressive.syncs
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER_PLAINTEXT
- name: ARGOCD_APPLICATIONSET_CONTROLLER_REPO_SERVER_PLAINTEXT
valueFrom:
configMapKeyRef:
key: controller.repo.server.plaintext
key: applicationsetcontroller.repo.server.plaintext
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER_STRICT_TLS
- name: ARGOCD_APPLICATIONSET_CONTROLLER_REPO_SERVER_STRICT_TLS
valueFrom:
configMapKeyRef:
key: controller.repo.server.strict.tls
key: applicationsetcontroller.repo.server.strict.tls
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_APPLICATIONSET_CONTROLLER_REPO_SERVER_TIMEOUT_SECONDS
valueFrom:
configMapKeyRef:
key: applicationsetcontroller.repo.server.timeout.seconds
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_APPLICATIONSET_CONTROLLER_CONCURRENT_RECONCILIATIONS
valueFrom:
configMapKeyRef:
key: applicationsetcontroller.concurrent.reconciliations.max
name: argocd-cmd-params-cm
optional: true
image: quay.io/argoproj/argocd:latest
Expand Down
20 changes: 16 additions & 4 deletions manifests/ha/install.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17919,16 +17919,28 @@ spec:
key: applicationsetcontroller.enable.progressive.syncs
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER_PLAINTEXT
- name: ARGOCD_APPLICATIONSET_CONTROLLER_REPO_SERVER_PLAINTEXT
valueFrom:
configMapKeyRef:
key: controller.repo.server.plaintext
key: applicationsetcontroller.repo.server.plaintext
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER_STRICT_TLS
- name: ARGOCD_APPLICATIONSET_CONTROLLER_REPO_SERVER_STRICT_TLS
valueFrom:
configMapKeyRef:
key: controller.repo.server.strict.tls
key: applicationsetcontroller.repo.server.strict.tls
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_APPLICATIONSET_CONTROLLER_REPO_SERVER_TIMEOUT_SECONDS
valueFrom:
configMapKeyRef:
key: applicationsetcontroller.repo.server.timeout.seconds
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_APPLICATIONSET_CONTROLLER_CONCURRENT_RECONCILIATIONS
valueFrom:
configMapKeyRef:
key: applicationsetcontroller.concurrent.reconciliations.max
name: argocd-cmd-params-cm
optional: true
image: quay.io/argoproj/argocd:latest
Expand Down
20 changes: 16 additions & 4 deletions manifests/ha/namespace-install.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1579,16 +1579,28 @@ spec:
key: applicationsetcontroller.enable.progressive.syncs
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER_PLAINTEXT
- name: ARGOCD_APPLICATIONSET_CONTROLLER_REPO_SERVER_PLAINTEXT
valueFrom:
configMapKeyRef:
key: controller.repo.server.plaintext
key: applicationsetcontroller.repo.server.plaintext
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER_STRICT_TLS
- name: ARGOCD_APPLICATIONSET_CONTROLLER_REPO_SERVER_STRICT_TLS
valueFrom:
configMapKeyRef:
key: controller.repo.server.strict.tls
key: applicationsetcontroller.repo.server.strict.tls
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_APPLICATIONSET_CONTROLLER_REPO_SERVER_TIMEOUT_SECONDS
valueFrom:
configMapKeyRef:
key: applicationsetcontroller.repo.server.timeout.seconds
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_APPLICATIONSET_CONTROLLER_CONCURRENT_RECONCILIATIONS
valueFrom:
configMapKeyRef:
key: applicationsetcontroller.concurrent.reconciliations.max
name: argocd-cmd-params-cm
optional: true
image: quay.io/argoproj/argocd:latest
Expand Down
20 changes: 16 additions & 4 deletions manifests/install.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17038,16 +17038,28 @@ spec:
key: applicationsetcontroller.enable.progressive.syncs
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER_PLAINTEXT
- name: ARGOCD_APPLICATIONSET_CONTROLLER_REPO_SERVER_PLAINTEXT
valueFrom:
configMapKeyRef:
key: controller.repo.server.plaintext
key: applicationsetcontroller.repo.server.plaintext
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER_STRICT_TLS
- name: ARGOCD_APPLICATIONSET_CONTROLLER_REPO_SERVER_STRICT_TLS
valueFrom:
configMapKeyRef:
key: controller.repo.server.strict.tls
key: applicationsetcontroller.repo.server.strict.tls
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_APPLICATIONSET_CONTROLLER_REPO_SERVER_TIMEOUT_SECONDS
valueFrom:
configMapKeyRef:
key: applicationsetcontroller.repo.server.timeout.seconds
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_APPLICATIONSET_CONTROLLER_CONCURRENT_RECONCILIATIONS
valueFrom:
configMapKeyRef:
key: applicationsetcontroller.concurrent.reconciliations.max
name: argocd-cmd-params-cm
optional: true
image: quay.io/argoproj/argocd:latest
Expand Down
20 changes: 16 additions & 4 deletions manifests/namespace-install.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -698,16 +698,28 @@ spec:
key: applicationsetcontroller.enable.progressive.syncs
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER_PLAINTEXT
- name: ARGOCD_APPLICATIONSET_CONTROLLER_REPO_SERVER_PLAINTEXT
valueFrom:
configMapKeyRef:
key: controller.repo.server.plaintext
key: applicationsetcontroller.repo.server.plaintext
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER_STRICT_TLS
- name: ARGOCD_APPLICATIONSET_CONTROLLER_REPO_SERVER_STRICT_TLS
valueFrom:
configMapKeyRef:
key: controller.repo.server.strict.tls
key: applicationsetcontroller.repo.server.strict.tls
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_APPLICATIONSET_CONTROLLER_REPO_SERVER_TIMEOUT_SECONDS
valueFrom:
configMapKeyRef:
key: applicationsetcontroller.repo.server.timeout.seconds
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_APPLICATIONSET_CONTROLLER_CONCURRENT_RECONCILIATIONS
valueFrom:
configMapKeyRef:
key: applicationsetcontroller.concurrent.reconciliations.max
name: argocd-cmd-params-cm
optional: true
image: quay.io/argoproj/argocd:latest
Expand Down
14 changes: 7 additions & 7 deletions pkg/client/clientset/versioned/fake/register.go
View file
Open in desktop

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

14 changes: 7 additions & 7 deletions pkg/client/clientset/versioned/scheme/register.go
View file
Open in desktop

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

30 changes: 18 additions & 12 deletions reposerver/repository/repository.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2539,20 +2539,23 @@ func (s *Service) GetGitFiles(_ context.Context, request *apiclient.GitFilesRequ
return nil, status.Error(codes.InvalidArgument, "must pass a valid repo")
}

gitClient, revision, err := s.newClientResolveRevision(repo, revision, git.WithCache(s.cache, true))
if err != nil {
return nil, status.Errorf(codes.Internal, "unable to resolve git revision %s: %v", revision, err)
}

// check the cache and return the results if present
if cachedFiles, err := s.cache.GetGitFiles(repo.Repo, revision); err == nil {
log.Debugf("cache hit for repo: %s revision: %s", repo.Repo, revision)
log.Debugf("get git files cache hit for repo: %s revision: %s", repo.Repo, revision)
return &apiclient.GitFilesResponse{
Map: cachedFiles,
}, nil
}

// cache miss, generate the results
gitClient, revision, err := s.newClientResolveRevision(repo, revision, git.WithCache(s.cache, true))
if err != nil {
return nil, status.Errorf(codes.Internal, "unable to resolve git revision %s: %v", revision, err)
}

s.metricsServer.IncPendingRepoRequest(repo.Repo)
defer s.metricsServer.DecPendingRepoRequest(repo.Repo)

closer, err := s.repoLock.Lock(gitClient.Root(), revision, true, func() (goio.Closer, error) {
return s.checkoutRevision(gitClient, revision, request.GetSubmoduleEnabled())
})
Expand Down Expand Up @@ -2594,20 +2597,23 @@ func (s *Service) GetGitDirectories(_ context.Context, request *apiclient.GitDir
return nil, status.Error(codes.InvalidArgument, "must pass a valid repo")
}

gitClient, revision, err := s.newClientResolveRevision(repo, revision, git.WithCache(s.cache, true))
if err != nil {
return nil, status.Errorf(codes.Internal, "unable to resolve git revision %s: %v", revision, err)
}

// check the cache and return the results if present
if cachedPaths, err := s.cache.GetGitDirectories(repo.Repo, revision); err == nil {
log.Debugf("cache hit for repo: %s revision: %s", repo.Repo, revision)
log.Debugf("get git dir cache hit for repo: %s revision: %s", repo.Repo, revision)
return &apiclient.GitDirectoriesResponse{
Paths: cachedPaths,
}, nil
}

// cache miss, generate the results
gitClient, revision, err := s.newClientResolveRevision(repo, revision, git.WithCache(s.cache, true))
if err != nil {
return nil, status.Errorf(codes.Internal, "unable to resolve git revision %s: %v", revision, err)
}

s.metricsServer.IncPendingRepoRequest(repo.Repo)
defer s.metricsServer.DecPendingRepoRequest(repo.Repo)

closer, err := s.repoLock.Lock(gitClient.Root(), revision, true, func() (goio.Closer, error) {
return s.checkoutRevision(gitClient, revision, request.GetSubmoduleEnabled())
})
Expand Down