Skip to content

Commit

Permalink
fix(perf): filtering process in application-list api (argoproj#12985) (…
Browse files Browse the repository at this point in the history 
…argoproj#12999)

* perf: fix filtering process in application-list api (fixes: argoproj#12985)

Signed-off-by: tken2039 <[email protected]>

* fix function for filtering by name

Signed-off-by: tken2039 <[email protected]>

* add nil check in filtering by name

Signed-off-by: tken2039 <[email protected]>

* add benchmark test for application list func

Signed-off-by: tken2039 <[email protected]>

* add err check for benchmark

Signed-off-by: tken2039 <[email protected]>

* fix test func for source soundness

Signed-off-by: tken2039 <[email protected]>

---------

Signed-off-by: tken2039 <[email protected]>
Signed-off-by: tken2039 <[email protected]>
  • Loading branch information
@tken2039
tken2039 authored Mar 30, 2023
1 parent b44c301 commit 9f6e5f9
Show file tree
Hide file tree
Showing 4 changed files with 478 additions and 14 deletions.
28 changes: 14 additions & 14 deletions server/application/application.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,21 @@ func (s *Server) List(ctx context.Context, q *application.ApplicationQuery) (*ap
if err != nil {
return nil, fmt.Errorf("error listing apps with selectors: %w", err)
}

filteredApps := apps
// Filter applications by name
if q.Name != nil {
filteredApps = argoutil.FilterByNameP(filteredApps, *q.Name)
}

// Filter applications by projects
filteredApps = argoutil.FilterByProjectsP(filteredApps, getProjectsFromApplicationQuery(*q))

// Filter applications by source repo URL
filteredApps = argoutil.FilterByRepoP(filteredApps, q.GetRepo())

newItems := make([]appv1.Application, 0)
for _, a := range apps {
for _, a := range filteredApps {
// Skip any application that is neither in the control plane's namespace
// nor in the list of enabled namespaces.
if a.Namespace != s.ns && !glob.MatchStringInList(s.enabledNamespaces, a.Namespace, false) {
Expand All @@ -219,19 +232,6 @@ func (s *Server) List(ctx context.Context, q *application.ApplicationQuery) (*ap
}
}

if q.Name != nil {
newItems, err = argoutil.FilterByName(newItems, *q.Name)
if err != nil {
return nil, fmt.Errorf("error filtering applications by name: %w", err)
}
}

// Filter applications by projects
newItems = argoutil.FilterByProjects(newItems, getProjectsFromApplicationQuery(*q))

// Filter applications by source repo URL
newItems = argoutil.FilterByRepo(newItems, q.GetRepo())

// Sort found applications by name
sort.Slice(newItems, func(i, j int) bool {
return newItems[i].Name < newItems[j].Name
Expand Down
309 changes: 309 additions & 0 deletions server/application/application_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,186 @@ func newTestAppServerWithEnforcerConfigure(f func(*rbac.Enforcer), t *testing.T,
return server.(*Server)
}

// return an ApplicationServiceServer which returns fake data
func newTestAppServerWithBenchmark(b *testing.B, objects ...runtime.Object) *Server {
f := func(enf *rbac.Enforcer) {
_ = enf.SetBuiltinPolicy(assets.BuiltinPolicyCSV)
enf.SetDefaultRole("role:admin")
}
return newTestAppServerWithEnforcerConfigureWithBenchmark(f, b, objects...)
}

func newTestAppServerWithEnforcerConfigureWithBenchmark(f func(*rbac.Enforcer), b *testing.B, objects ...runtime.Object) *Server {
kubeclientset := fake.NewSimpleClientset(&v1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Namespace: testNamespace,
Name: "argocd-cm",
Labels: map[string]string{
"app.kubernetes.io/part-of": "argocd",
},
},
}, &v1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "argocd-secret",
Namespace: testNamespace,
},
Data: map[string][]byte{
"admin.password": []byte("test"),
"server.secretkey": []byte("test"),
},
})
ctx := context.Background()
db := db.NewDB(testNamespace, settings.NewSettingsManager(ctx, kubeclientset, testNamespace), kubeclientset)
_, err := db.CreateRepository(ctx, fakeRepo())
require.NoError(b, err)
_, err = db.CreateCluster(ctx, fakeCluster())
require.NoError(b, err)

mockRepoClient := &mocks.Clientset{RepoServerServiceClient: fakeRepoServerClient(false)}

defaultProj := &appsv1.AppProject{
ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "default"},
Spec: appsv1.AppProjectSpec{
SourceRepos: []string{"*"},
Destinations: []appsv1.ApplicationDestination{{Server: "*", Namespace: "*"}},
},
}
myProj := &appsv1.AppProject{
ObjectMeta: metav1.ObjectMeta{Name: "my-proj", Namespace: "default"},
Spec: appsv1.AppProjectSpec{
SourceRepos: []string{"*"},
Destinations: []appsv1.ApplicationDestination{{Server: "*", Namespace: "*"}},
},
}
projWithSyncWindows := &appsv1.AppProject{
ObjectMeta: metav1.ObjectMeta{Name: "proj-maint", Namespace: "default"},
Spec: appsv1.AppProjectSpec{
SourceRepos: []string{"*"},
Destinations: []appsv1.ApplicationDestination{{Server: "*", Namespace: "*"}},
SyncWindows: appsv1.SyncWindows{},
},
}
matchingWindow := &appsv1.SyncWindow{
Kind: "allow",
Schedule: "* * * * *",
Duration: "1h",
Applications: []string{"test-app"},
}
projWithSyncWindows.Spec.SyncWindows = append(projWithSyncWindows.Spec.SyncWindows, matchingWindow)

objects = append(objects, defaultProj, myProj, projWithSyncWindows)

fakeAppsClientset := apps.NewSimpleClientset(objects...)
factory := appinformer.NewSharedInformerFactoryWithOptions(fakeAppsClientset, 0, appinformer.WithNamespace(""), appinformer.WithTweakListOptions(func(options *metav1.ListOptions) {}))
fakeProjLister := factory.Argoproj().V1alpha1().AppProjects().Lister().AppProjects(testNamespace)

enforcer := rbac.NewEnforcer(kubeclientset, testNamespace, common.ArgoCDRBACConfigMapName, nil)
f(enforcer)
enforcer.SetClaimsEnforcerFunc(rbacpolicy.NewRBACPolicyEnforcer(enforcer, fakeProjLister).EnforceClaims)

settingsMgr := settings.NewSettingsManager(ctx, kubeclientset, testNamespace)

// populate the app informer with the fake objects
appInformer := factory.Argoproj().V1alpha1().Applications().Informer()

go appInformer.Run(ctx.Done())
if !k8scache.WaitForCacheSync(ctx.Done(), appInformer.HasSynced) {
panic("Timed out waiting for caches to sync")
}

projInformer := factory.Argoproj().V1alpha1().AppProjects().Informer()
go projInformer.Run(ctx.Done())
if !k8scache.WaitForCacheSync(ctx.Done(), projInformer.HasSynced) {
panic("Timed out waiting for caches to sync")
}

broadcaster := new(appmocks.Broadcaster)
broadcaster.On("Subscribe", mock.Anything, mock.Anything).Return(func() {}).Run(func(args mock.Arguments) {
// Simulate the broadcaster notifying the subscriber of an application update.
// The second parameter to Subscribe is filters. For the purposes of tests, we ignore the filters. Future tests
// might require implementing those.
go func() {
events := args.Get(0).(chan *appsv1.ApplicationWatchEvent)
for _, obj := range objects {
app, ok := obj.(*appsv1.Application)
if ok {
oldVersion, err := strconv.Atoi(app.ResourceVersion)
if err != nil {
oldVersion = 0
}
clonedApp := app.DeepCopy()
clonedApp.ResourceVersion = fmt.Sprintf("%d", oldVersion+1)
events <- &appsv1.ApplicationWatchEvent{Type: watch.Added, Application: *clonedApp}
}
}
}()
})
broadcaster.On("OnAdd", mock.Anything).Return()
broadcaster.On("OnUpdate", mock.Anything, mock.Anything).Return()
broadcaster.On("OnDelete", mock.Anything).Return()

appStateCache := appstate.NewCache(cache.NewCache(cache.NewInMemoryCache(time.Hour)), time.Hour)
// pre-populate the app cache
for _, obj := range objects {
app, ok := obj.(*appsv1.Application)
if ok {
err := appStateCache.SetAppManagedResources(app.Name, []*appsv1.ResourceDiff{})
require.NoError(b, err)

// Pre-populate the resource tree based on the app's resources.
nodes := make([]appsv1.ResourceNode, len(app.Status.Resources))
for i, res := range app.Status.Resources {
nodes[i] = appsv1.ResourceNode{
ResourceRef: appsv1.ResourceRef{
Group: res.Group,
Kind: res.Kind,
Version: res.Version,
Name: res.Name,
Namespace: res.Namespace,
UID: "fake",
},
}
}
err = appStateCache.SetAppResourcesTree(app.Name, &appsv1.ApplicationTree{
Nodes: nodes,
})
require.NoError(b, err)
}
}
appCache := servercache.NewCache(appStateCache, time.Hour, time.Hour, time.Hour)

kubectl := &kubetest.MockKubectlCmd{}
kubectl = kubectl.WithGetResourceFunc(func(_ context.Context, _ *rest.Config, gvk schema.GroupVersionKind, name string, namespace string) (*unstructured.Unstructured, error) {
for _, obj := range objects {
if obj.GetObjectKind().GroupVersionKind().GroupKind() == gvk.GroupKind() {
if obj, ok := obj.(*unstructured.Unstructured); ok && obj.GetName() == name && obj.GetNamespace() == namespace {
return obj, nil
}
}
}
return nil, nil
})

server, _ := NewServer(
testNamespace,
kubeclientset,
fakeAppsClientset,
factory.Argoproj().V1alpha1().Applications().Lister(),
appInformer,
broadcaster,
mockRepoClient,
appCache,
kubectl,
db,
enforcer,
sync.NewKeyLock(),
settingsMgr,
projInformer,
[]string{},
)
return server.(*Server)
}

const fakeApp = `
apiVersion: argoproj.io/v1alpha1
kind: Application
Expand Down Expand Up @@ -1009,6 +1189,135 @@ g, group-49, role:test3
assert.Equal(t, 300, len(names))
}

func generateTestApp(num int) []*appsv1.Application {
apps := []*appsv1.Application{}
for i := 0; i < num; i++ {
apps = append(apps, newTestApp(func(app *appsv1.Application) {
app.Name = fmt.Sprintf("test-app%.6d", i)
}))
}

return apps
}

func BenchmarkListMuchApps(b *testing.B) {
// 10000 apps
apps := generateTestApp(10000)
obj := make([]runtime.Object, len(apps))
for i, v := range apps {
obj[i] = v
}
appServer := newTestAppServerWithBenchmark(b, obj...)

b.ResetTimer()
for n := 0; n < b.N; n++ {
_, err := appServer.List(context.Background(), &application.ApplicationQuery{})
if err != nil {
break
}
}
}

func BenchmarkListSomeApps(b *testing.B) {
// 500 apps
apps := generateTestApp(500)
obj := make([]runtime.Object, len(apps))
for i, v := range apps {
obj[i] = v
}
appServer := newTestAppServerWithBenchmark(b, obj...)

b.ResetTimer()
for n := 0; n < b.N; n++ {
_, err := appServer.List(context.Background(), &application.ApplicationQuery{})
if err != nil {
break
}
}
}

func BenchmarkListFewApps(b *testing.B) {
// 10 apps
apps := generateTestApp(10)
obj := make([]runtime.Object, len(apps))
for i, v := range apps {
obj[i] = v
}
appServer := newTestAppServerWithBenchmark(b, obj...)

b.ResetTimer()
for n := 0; n < b.N; n++ {
_, err := appServer.List(context.Background(), &application.ApplicationQuery{})
if err != nil {
break
}
}
}

func strToPtr(v string) *string {
return &v
}

func BenchmarkListMuchAppsWithName(b *testing.B) {
// 10000 apps
appsMuch := generateTestApp(10000)
obj := make([]runtime.Object, len(appsMuch))
for i, v := range appsMuch {
obj[i] = v
}
appServer := newTestAppServerWithBenchmark(b, obj...)

b.ResetTimer()
for n := 0; n < b.N; n++ {
app := &application.ApplicationQuery{Name: strToPtr("test-app000099")}
_, err := appServer.List(context.Background(), app)
if err != nil {
break
}
}
}

func BenchmarkListMuchAppsWithProjects(b *testing.B) {
// 10000 apps
appsMuch := generateTestApp(10000)
appsMuch[999].Spec.Project = "test-project1"
appsMuch[1999].Spec.Project = "test-project2"
obj := make([]runtime.Object, len(appsMuch))
for i, v := range appsMuch {
obj[i] = v
}
appServer := newTestAppServerWithBenchmark(b, obj...)

b.ResetTimer()
for n := 0; n < b.N; n++ {
app := &application.ApplicationQuery{Project: []string{"test-project1", "test-project2"}}
_, err := appServer.List(context.Background(), app)
if err != nil {
break
}
}
}

func BenchmarkListMuchAppsWithRepo(b *testing.B) {
// 10000 apps
appsMuch := generateTestApp(10000)
appsMuch[999].Spec.Source.RepoURL = "https://some-fake-source"
obj := make([]runtime.Object, len(appsMuch))
for i, v := range appsMuch {
obj[i] = v
}
appServer := newTestAppServerWithBenchmark(b, obj...)

b.ResetTimer()
for n := 0; n < b.N; n++ {
app := &application.ApplicationQuery{Repo: strToPtr("https://some-fake-source")}
_, err := appServer.List(context.Background(), app)
if err != nil {
break
}
}
}

func TestCreateApp(t *testing.T) {
testApp := newTestApp()
appServer := newTestAppServer(t)
Expand Down
Loading

0 comments on commit 9f6e5f9

Please sign in to comment.