feat: implement 'argocd admin appset generate' to troubeshoot appsets #19518

alexmt · 2024-08-13T20:00:06Z

PR implements argocd appset generate that helps troubleshooting appset issues. Examples:

argocd appset generate ~/go/src/github.com/argoproj/argo-cd/applicationset/examples/list-generator/list-

output

- apiVersion: argoproj.io/v1alpha1
  kind: Application
  metadata:
    finalizers:
    - resources-finalizer.argocd.argoproj.io
    name: engineering-dev-guestbook
  spec:
    destination:
      namespace: guestbook
      server: https://kubernetes.default.svc
    project: default
    source:
      path: applicationset/examples/list-generator/guestbook/engineering-dev
      repoURL: https://github.com/argoproj/argo-cd.git
      targetRevision: HEAD
- apiVersion: argoproj.io/v1alpha1
  kind: Application
  metadata:
    finalizers:
    - resources-finalizer.argocd.argoproj.io
    name: engineering-prod-guestbook
  spec:
    destination:
      namespace: guestbook
      server: https://kubernetes.default.svc
    project: default
    source:
      path: applicationset/examples/list-generator/guestbook/engineering-prod
      repoURL: https://github.com/argoproj/argo-cd.git
      targetRevision: HEAD

argocd appset generate ~/go/src/github.com/argoproj/argo-cd/applicationset/examples/git-generator-directory/git-directories-example.yaml

output

- apiVersion: argoproj.io/v1alpha1
  kind: Application
  metadata:
    finalizers:
    - resources-finalizer.argocd.argoproj.io
    name: argo-workflows
  spec:
    destination:
      namespace: argo-workflows
      server: https://kubernetes.default.svc
    project: my-project
    source:
      path: applicationset/examples/git-generator-directory/cluster-addons/argo-workflows
      repoURL: https://github.com/argoproj/argo-cd.git
      targetRevision: HEAD
    syncPolicy:
      syncOptions:
      - CreateNamespace=true
- apiVersion: argoproj.io/v1alpha1
  kind: Application
  metadata:
    finalizers:
    - resources-finalizer.argocd.argoproj.io
    name: prometheus-operator
  spec:
    destination:
      namespace: prometheus-operator
      server: https://kubernetes.default.svc
    project: my-project
    source:
      path: applicationset/examples/git-generator-directory/cluster-addons/prometheus-operator
      repoURL: https://github.com/argoproj/argo-cd.git
      targetRevision: HEAD
    syncPolicy:
      syncOptions:
      - CreateNamespace=true

argocd appset generate ~/go/src/github.com/argoproj/argo-cd/applicationset/examples/pull-request-generator/pull-request-example.yaml

failed output

ERRO[0000] error generating params                       error="failed to select pull request service provider: error fetching Secret token: error fetching secret /github-token: an empty namespace may not be set when a resource name is provided" generator="&{0x1400128d170 0x104833710 { [] true 0x1400126b1d0}}"
ERRO[0000] error generating application from params      error="failed to select pull request service provider: error fetching Secret token: error fetching secret /github-token: an empty namespace may not be set when a resource name is provided" generator="{nil nil nil nil nil &PullRequestGenerator{Github:&PullRequestGeneratorGithub{Owner:myorg,Repo:myrepo,API:https://git.example.com/,TokenRef:&SecretRef{SecretName:github-token,Key:token,},AppSecretName:,Labels:[preview],},GitLab:nil,Gitea:nil,BitbucketServer:nil,Filters:[]PullRequestGeneratorFilter{},RequeueAfterSeconds:nil,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}"
Error: failed to select pull request service provider: error fetching Secret token: error fetching secret /github-token: an empty namespace may not be set when a resource name is provided

bunnyshell · 2024-08-13T20:00:12Z

❌ Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

🚀 /bns:deploy to deploy the environment

bunnyshell · 2024-08-13T20:00:13Z

❌ Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

🚀 /bns:deploy to deploy the environment

codecov · 2024-08-13T20:59:04Z

Codecov Report

Attention: Patch coverage is 3.79747% with 76 lines in your changes missing coverage. Please review.

Project coverage is 55.80%. Comparing base (9af0ff5) to head (49b7245).
Report is 492 commits behind head on master.

Files with missing lines	Patch %	Lines
cmd/argocd/commands/applicationset.go	0.00%	50 Missing ⚠️
server/applicationset/applicationset.go	12.00%	22 Missing ⚠️
cmd/argocd-server/commands/argocd_server.go	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #19518      +/-   ##
==========================================
- Coverage   55.87%   55.80%   -0.08%     
==========================================
  Files         316      320       +4     
  Lines       43794    43980     +186     
==========================================
+ Hits        24471    24541      +70     
- Misses      16775    16887     +112     
- Partials     2548     2552       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

todaywasawesome

@alexmt Part of the implementation here is with an embedded repo server, can you share how that works?

daftping · 2024-08-19T16:12:10Z

I wonder how this command differs from a recently merged similar PR.
#16781

pasha-codefresh

Amazing feature. Thank you Alex, just few small comments

cmd/argocd/commands/admin/appset.go

cmd/argocd/commands/headless/headless.go

agaudreault · 2024-08-22T17:45:39Z

cmd/argocd/commands/admin/appset.go

+	"sigs.k8s.io/yaml"
+)
+
+func NewAppsetCommand(opts *apiclient.ClientOptions) *cobra.Command {


I feel like this does not make a lot of sense under the admin commands. Maybe argocd appset template ./appset.yaml -o yaml would make more sense. Take a look at #16781, that has prior work to implement this feature.

Agree - moved command to argocd appset generate. I've choosen to name it generate , however don't have strong opinion . Please let me know what you think

cmd/argocd/commands/root.go

alexmt · 2024-08-26T21:01:16Z

@agaudreault, @daftping, Thanks for letting me know about argocd appset create --dry-run #16781! I did not know it was implemented.

My main goal is to enable customers to troubleshoot application sets. Pros of argocd admin appset generate:

Command prints all related logs if app generation fails - currently we often have to check applicationset controller logs to understand what went wrong. ApplicationSet status is not enough
Command prints YAML of generated applications - this enables user verify that appset is working as expected
Command allows using local git repos using file:///<path>/<to>/<repo> (embedded repo server)

Cons:

argocd admin appset generate works only for admins, while argocd admin appset generate works for everyone
the argocd admin appset generate is already there so it is confusing to introduce another command

I propose to improve argocd appset create --dry-run to return error logs (if any ) and the list of generate argocd apps. We won't be able to use local git repos but it is anyway nice to have feature. If no objections I would close this PR and work on argocd appset create --dry-run improvements in another PR.

agaudreault · 2024-08-26T22:26:08Z

@alexmt sounds good to me! Last time I looked at it, I chose not to return the full templated app so i wouldn't change the return type of the create. By default, it won't return the template when it is not a dry run because they are generated async. I didn't want to have the create returns false results.

I think it would be better to introduce a new template command that would return the appset and the templates of generated apps. And if a name is provided instead of a file, it returns the actual data.

That was my state of mind as far as i can recall, but I'll let you judge what's best when you get in the code!

kencochrane · 2024-08-27T19:06:09Z

@alexmt I think closing this one and adding improvement to argocd appset create --dry-run make sense. Thank you

alexmt · 2024-08-27T19:59:52Z

Updated the PR with new implementation (instead of closing and reopening). The PR introduces argocd appset generate command that uses new API to generate applications/get generation error.

PTAL @agaudreault , @pasha-codefresh

alexmt · 2024-08-27T20:10:08Z

cmd/argocd-server/commands/argocd_server.go

@@ -142,7 +144,11 @@ func NewCommand() *cobra.Command {

 			dynamicClient := dynamic.NewForConfigOrDie(config)

-			controllerClient, err := client.New(config, client.Options{})


Change is required if user is trying to generate appset with non default project (or create with --dry-run=true flag)

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

pasha-codefresh · 2024-08-28T08:59:29Z

server/applicationset/applicationset.go

+
+	apps, err := s.generateApplicationSetApps(ctx, logger.WithField("applicationset", appset.Name), *appset, namespace)
+	if err != nil {
+		res.ErrorLog = fmt.Sprintf("unable to generate Applications of ApplicationSet: %v\n", err)


why not return error and build error message on cli , base on this error ? In any case you will need process error on client part, to show reasonable error when request or permissions are incorrect, i think it will be more consistent to do it also when you cant generate applications

Makes sense. ErrorLog just duplicates error - removed it.

pasha-codefresh · 2024-08-28T09:06:48Z

LGTM, just left small comment

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

agaudreault

Nice! ❤️ Few small comments, nothing major :)

agaudreault · 2024-08-28T15:00:33Z

cmd/argocd/commands/applicationset.go

+	var output string
+	command := &cobra.Command{
+		Use:   "generate",
+		Short: "Generate apps of ApplicationSet",


Clarify that we are only generating the rendered templates and not the apps themselves

Suggested change

Short: "Generate apps of ApplicationSet",

Short: "Generate apps of ApplicationSet rendered templates",

thank you, applied!

agaudreault · 2024-08-28T15:01:42Z

cmd/argocd/commands/applicationset.go

+			}
+			appset := appsets[0]
+			if appset.Name == "" {
+				err := fmt.Errorf("Error generating aps for ApplicationSet %s. ApplicationSet does not have Name field set", appset)


Suggested change

err := fmt.Errorf("Error generating aps for ApplicationSet %s. ApplicationSet does not have Name field set", appset)

err := fmt.Errorf("Error generating apps for ApplicationSet %s. ApplicationSet does not have Name field set", appset)

accepted suggesion

agaudreault · 2024-08-28T15:06:45Z

cmd/argocd/commands/applicationset.go

+				for i := range appsList {
+					app := appsList[i]
+					app.APIVersion = arogappsetv1.ApplicationSchemaGroupVersionKind.GroupVersion().String()
+					app.Kind = arogappsetv1.ApplicationSchemaGroupVersionKind.Kind
+					resources = append(resources, app)
+				}


More of a question, but why are you overriding the version and kind? This should already be provided in the response object no? The client could be outdated locally and have a different ApiVersion. I think it would be better to use the value provided by the server.
Maybe adding a comment with the explanation if necessary would help future contributors.

This is due to a strange behavior of go k8s client. For some reason it drops apiVersion/kind . We have to do the same workaround in other places. Example:

argo-cd/cmd/argocd/commands/app_actions.go

Line 225 in 7cf5ed0

app.Kind = application.ApplicationKind

agaudreault · 2024-08-28T15:16:32Z

server/applicationset/applicationset.go

@@ -366,6 +365,40 @@ func (s *Server) ResourceTree(ctx context.Context, q *applicationset.Application
 	return s.buildApplicationSetTree(a)
 }

+func (s *Server) Generate(ctx context.Context, q *applicationset.ApplicationSetGenerateRequest) (*applicationset.ApplicationSetGenerateResponse, error) {


What about RBAC for this call? It may allow to discover cluster secrets, private repos, etc. Should we restrict this to people with the "create" rbac?

Agree - user must have "create" RBAC to see generated apps. I've added a check here:

argo-cd/server/applicationset/applicationset.go

Line 383 in 4cbb1a9

if err := s.checkCreatePermissions(ctx, appset, projectName); err != nil {

alexmt · 2024-08-28T15:48:40Z

I've addressed your comments @agaudreault, thank you! PTAL

agaudreault · 2024-08-28T16:45:45Z

@alexmt missing a run of codegen, then good to go 🚀

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

alexmt · 2024-08-28T22:38:36Z

@agaudreault done!

alexmt requested review from a team as code owners August 13, 2024 20:00

todaywasawesome reviewed Aug 14, 2024

View reviewed changes

pasha-codefresh requested changes Aug 21, 2024

View reviewed changes

cmd/argocd/commands/admin/appset.go Outdated Show resolved Hide resolved

cmd/argocd/commands/headless/headless.go Outdated Show resolved Hide resolved

cmd/argocd/commands/headless/headless.go Outdated Show resolved Hide resolved

agaudreault requested changes Aug 22, 2024

View reviewed changes

alexmt force-pushed the appset-debug-local branch 2 times, most recently from b9e21b6 to b008678 Compare August 27, 2024 19:55

alexmt commented Aug 27, 2024

View reviewed changes

alexmt force-pushed the appset-debug-local branch from b008678 to 170f238 Compare August 27, 2024 20:32

feat: implement 'argocd appset generate' to troubeshoot appsets

5bbcffd

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

alexmt force-pushed the appset-debug-local branch from 170f238 to 5bbcffd Compare August 27, 2024 20:44

pasha-codefresh requested changes Aug 28, 2024

View reviewed changes

remove unnecessary ErrorLog field

4cbb1a9

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

pasha-codefresh approved these changes Aug 28, 2024

View reviewed changes

agaudreault reviewed Aug 28, 2024

View reviewed changes

alexmt force-pushed the appset-debug-local branch from 355c3c0 to dc8cad8 Compare August 28, 2024 15:42

apply reviewer suggestions

49b7245

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

alexmt force-pushed the appset-debug-local branch from dc8cad8 to 49b7245 Compare August 28, 2024 22:38

alexmt enabled auto-merge (squash) August 28, 2024 22:38

agaudreault approved these changes Aug 28, 2024

View reviewed changes

alexmt merged commit 3a5b653 into argoproj:master Aug 28, 2024
27 of 28 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implement 'argocd admin appset generate' to troubeshoot appsets #19518

feat: implement 'argocd admin appset generate' to troubeshoot appsets #19518

alexmt commented Aug 13, 2024 •

edited

Loading

bunnyshell bot commented Aug 13, 2024 •

edited

Loading

bunnyshell bot commented Aug 13, 2024 •

edited

Loading

codecov bot commented Aug 13, 2024 •

edited

Loading

todaywasawesome left a comment

daftping commented Aug 19, 2024

pasha-codefresh left a comment

agaudreault Aug 22, 2024

alexmt Aug 27, 2024

alexmt commented Aug 26, 2024

agaudreault commented Aug 26, 2024

kencochrane commented Aug 27, 2024

alexmt commented Aug 27, 2024

alexmt Aug 27, 2024

pasha-codefresh Aug 28, 2024

alexmt Aug 28, 2024

pasha-codefresh commented Aug 28, 2024

agaudreault left a comment

agaudreault Aug 28, 2024

alexmt Aug 28, 2024

agaudreault Aug 28, 2024

alexmt Aug 28, 2024

agaudreault Aug 28, 2024

alexmt Aug 28, 2024

agaudreault Aug 28, 2024 •

edited

Loading

alexmt Aug 28, 2024

alexmt commented Aug 28, 2024

agaudreault commented Aug 28, 2024

alexmt commented Aug 28, 2024

		@@ -142,7 +144,11 @@ func NewCommand() *cobra.Command {

		dynamicClient := dynamic.NewForConfigOrDie(config)

		controllerClient, err := client.New(config, client.Options{})

	Short: "Generate apps of ApplicationSet",
	Short: "Generate apps of ApplicationSet rendered templates",

	err := fmt.Errorf("Error generating aps for ApplicationSet %s. ApplicationSet does not have Name field set", appset)
	err := fmt.Errorf("Error generating apps for ApplicationSet %s. ApplicationSet does not have Name field set", appset)

feat: implement 'argocd admin appset generate' to troubeshoot appsets #19518

feat: implement 'argocd admin appset generate' to troubeshoot appsets #19518

Conversation

alexmt commented Aug 13, 2024 • edited Loading

bunnyshell bot commented Aug 13, 2024 • edited Loading

❌ Preview Environment deleted from Bunnyshell

bunnyshell bot commented Aug 13, 2024 • edited Loading

❌ Preview Environment deleted from Bunnyshell

codecov bot commented Aug 13, 2024 • edited Loading

Codecov Report

todaywasawesome left a comment

Choose a reason for hiding this comment

daftping commented Aug 19, 2024

pasha-codefresh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexmt commented Aug 26, 2024

agaudreault commented Aug 26, 2024

kencochrane commented Aug 27, 2024

alexmt commented Aug 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pasha-codefresh commented Aug 28, 2024

agaudreault left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agaudreault Aug 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexmt commented Aug 28, 2024

agaudreault commented Aug 28, 2024

alexmt commented Aug 28, 2024

alexmt commented Aug 13, 2024 •

edited

Loading

bunnyshell bot commented Aug 13, 2024 •

edited

Loading

bunnyshell bot commented Aug 13, 2024 •

edited

Loading

codecov bot commented Aug 13, 2024 •

edited

Loading

agaudreault Aug 28, 2024 •

edited

Loading