argo-cd icon indicating copy to clipboard operation
argo-cd copied to clipboard

Tansfer only application's manifests to cmp-server for plugin-based applications

Open jsolana opened this issue 10 months ago • 2 comments

Summary

Reduce the volume of files transferred to the cmp-server during GenerateManifests op to optimize the manifest generation process for plugin-based applications.

Motivation

In monorepos, excessive data transfer during GenerateManifests can create bottlenecks by sending more information than the plugin actually requires to generate the manifest for an application. This inefficiency significantly impacts performance, especially with repeated operations.

The full repository (excluding globs) is being sent in each gRPC request to the cmp-server. For each call to the cmp-server, the repo-server creates a tar.gz of the entire repo and sends it, then the cmp-server has to decompress it and create all the files on the disk, run the generate command, and delete it. Most of the time is spent waiting for command executions, creating and deleting directories and files.

image

Proposal

Transmit only the necessary information to the cmp-server.

As a quick win, we can use appPath instead of repoPath in generateManifestsCMP for plugin-based applications.

reposerver/repository/repository.go:

// generateManifestsCMP will send the appPath files to the cmp-server over a gRPC stream.
// The cmp-server will generate the manifests. Returns a response object with the generated
// manifests.
func generateManifestsCMP(ctx context.Context, appPath string, env []string, cmpClient pluginclient.ConfigManagementPluginServiceClient, tarDoneCh chan<- bool, tarExcludedGlobs []string) (*pluginclient.ManifestResponse, error) {
	generateManifestStream, err := cmpClient.GenerateManifest(ctx, grpc_retry.Disable())
	if err != nil {
		return nil, fmt.Errorf("error getting generateManifestStream: %w", err)
	}
	opts := []cmp.SenderOption{
		cmp.WithTarDoneChan(tarDoneCh),
	}

	// In this case, we using use appPath instead of repoPath to only send app resources
	err = cmp.SendRepoStream(generateManifestStream.Context(), appPath, appPath, generateManifestStream, env, tarExcludedGlobs, opts...)
	if err != nil {
		return nil, fmt.Errorf("error sending file to cmp-server: %s", err)
	}

	return generateManifestStream.CloseAndRecv()
}

The screenshot below represents time duration of GenerataeManifests op before and after apply this quick win:

image

Note: A more general solution could involve leveraging the value described in the annotation argocd.argoproj.io/manifest-generate-paths. This approach can be extended to other solutions as kustomize or helm (to discuss maybe in another issue).

jsolana avatar Apr 24 '24 08:04 jsolana

If it is ok, I can create the MR :)

jsolana avatar Apr 24 '24 09:04 jsolana

I'd use this. But even if argocd.argoproj.io/manifest-generate-paths is set, and we use that value to copy files we would want to have this behavior off by default as some people might "include/import" out-of-app-folder files during manifest rendering, even if they didn't configure ArgoCD to trigger sync on those file changes

Oded-B avatar Apr 25 '24 07:04 Oded-B

Moving the conversation from https://github.com/argoproj/argo-cd/pull/18054 for visibility

This value ( the value of the argocd.argoproj.io/manifest-generate-paths annotation) is not propagated right now .

At first glance we have two options

  • we can extend ApplicationSource to contains AnnotationManifetGeneratePaths:

types.go

// ApplicationSource contains all required information about the source of an application
type ApplicationSource struct {
	// RepoURL is the URL to the repository (Git or Helm) that contains the application manifests
	RepoURL string `json:"repoURL" protobuf:"bytes,1,opt,name=repoURL"`
	// Path is a directory path within the Git repository, and is only valid for applications sourced from Git.
	Path string `json:"path,omitempty" protobuf:"bytes,2,opt,name=path"`
	// TargetRevision defines the revision of the source to sync the application to.
	// In case of Git, this can be commit, tag, or branch. If omitted, will equal to HEAD.
	// In case of Helm, this is a semver tag for the Chart's version.
	TargetRevision string `json:"targetRevision,omitempty" protobuf:"bytes,4,opt,name=targetRevision"`
	// Helm holds helm specific options
	Helm *ApplicationSourceHelm `json:"helm,omitempty" protobuf:"bytes,7,opt,name=helm"`
	// Kustomize holds kustomize specific options
	Kustomize *ApplicationSourceKustomize `json:"kustomize,omitempty" protobuf:"bytes,8,opt,name=kustomize"`
	// Directory holds path/directory specific options
	Directory *ApplicationSourceDirectory `json:"directory,omitempty" protobuf:"bytes,10,opt,name=directory"`
	// Plugin holds config management plugin specific options
	Plugin *ApplicationSourcePlugin `json:"plugin,omitempty" protobuf:"bytes,11,opt,name=plugin"`
	// Chart is a Helm chart name, and must be specified for applications sourced from a Helm repo.
	Chart string `json:"chart,omitempty" protobuf:"bytes,12,opt,name=chart"`
	// Ref is reference to another source within sources field. This field will not be used if used with a `source` tag.
	Ref string `json:"ref,omitempty" protobuf:"bytes,13,opt,name=ref"`
	// AnnotationManifestGeneratePaths holds argocd.argoproj.io/manifest-generate-paths annotation value of the Application.
	AnnotationManifestGeneratePaths string `json:"annotationManifestGeneratePaths" protobuf:"bytes,14,opt,name=annotationManifestGeneratePaths"`
}
  • Or because this annotation only affects manifests generation maybe make sense to modify ManifestRequest:

repository.proto

// ManifestRequest is a query for manifest generation.
message ManifestRequest {
    github.com.argoproj.argo_cd.v2.pkg.apis.application.v1alpha1.Repository repo = 1;
    // revision, potentially un-resolved
    string revision = 2;
    bool noCache = 3;
    string appLabelKey = 4;
    // Name of the application for which the request is triggered
    string appName = 5;
    string namespace = 8;
    github.com.argoproj.argo_cd.v2.pkg.apis.application.v1alpha1.ApplicationSource applicationSource = 10;
    repeated github.com.argoproj.argo_cd.v2.pkg.apis.application.v1alpha1.Repository repos = 11;
    // Deprecated: use sidecar plugins instead.
    repeated github.com.argoproj.argo_cd.v2.pkg.apis.application.v1alpha1.ConfigManagementPlugin plugins = 12;
    github.com.argoproj.argo_cd.v2.pkg.apis.application.v1alpha1.KustomizeOptions kustomizeOptions = 13;
    string kubeVersion = 14;
    repeated string apiVersions = 15;
    // Request to verify the signature when generating the manifests (only for Git repositories)
    bool verifySignature = 16;
    repeated github.com.argoproj.argo_cd.v2.pkg.apis.application.v1alpha1.RepoCreds helmRepoCreds = 17;
    bool noRevisionCache = 18;
    string trackingMethod = 19;
    map<string, bool> enabledSourceTypes = 20;
    github.com.argoproj.argo_cd.v2.pkg.apis.application.v1alpha1.HelmOptions helmOptions = 21;
    bool hasMultipleSources = 22;
    map<string, github.com.argoproj.argo_cd.v2.pkg.apis.application.v1alpha1.RefTarget> refSources = 23;
    // This is used to surface "source not permitted" errors for Helm repositories
    repeated string projectSourceRepos = 24;
    // This is used to surface "source not permitted" errors for Helm repositories
    string projectName = 25;
    // argocd.argoproj.io/manifest-generate-paths annotation value of the Application to allow optimize which resources propagated to cmpserver
    string AnnotationManifestGeneratePaths = 26;
}

IMHO I prefer the second one cause is more related to the manifest generation.

Wdyt?

jsolana avatar Jun 20 '24 13:06 jsolana