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.

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

fix(repo-server): excess git requests, short-circuit GenerateManifests ref only (Issue #14725) #1

Open
wants to merge 38 commits into
base: lsremote-part1
Choose a base branch
from

Conversation

nromriell
Copy link
Owner

Partially Fixes argoproj#14725

Second part of a split of the argoproj#16309 and follow-up to argoproj#16410. Continues the fixes required to remove the excess git requests when using multi-source and ref sources.

Issue:
When GenerateManifest is called with a ref only source manifest generation is still run, and since there are no manifests this is always considered a manifest cache miss causing an excess number of fetch requests to the git server

Changes:
Short-circuit calls to GenerateManifest when it is a ref only source, however as part of the short circuit still resolve the references if they are not already in cache, this can help prevent additional calls to ls-remote when GenerateManifest is called with a related manifest generating source

Impact:
This PR should greatly reduce the number of fetch calls made to git for multi-source applications with a ref only source. Some minor improvements to ls-remote should be observed as well since we skip unnecessary git util calls.

There is one additional item that will be included in a follow-up PR to finish up the reduction of ls-remote calls

Baseline v2.9.0 with 200 multi-source applications with a ref only source:
Screenshot from 2023-11-18 18-24-30

With the changes in the first PR under the same conditions:
Screenshot from 2023-11-21 00-43-54

With the changes from the first PR and this PR under the same conditions:
Screenshot from 2023-11-21 01-23-41

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR states what changed and the related issues number (used for the release note).
  • The title of the PR conforms to the Toolchain Guide
  • I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Does this PR require documentation updates?
  • I've updated documentation as required by this PR.
  • Optional. My organization is added to USERS.md.
  • I have signed off all my commits as required by DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My build is green (troubleshooting builds).
  • My new feature complies with the feature status guidelines.
  • I have added a brief description of why this PR is necessary and/or what this PR solves.

dependabot bot and others added 18 commits November 21, 2023 19:34
…rgoproj#16418)

Bumps [github.com/go-jose/go-jose/v3](https://github.com/go-jose/go-jose) from 3.0.0 to 3.0.1.
- [Release notes](https://github.com/go-jose/go-jose/releases)
- [Changelog](https://github.com/go-jose/go-jose/blob/v3/CHANGELOG.md)
- [Commits](go-jose/go-jose@v3.0.0...v3.0.1)

---
updated-dependencies:
- dependency-name: github.com/go-jose/go-jose/v3
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This actually wasn't that bad. It was just a few bumps in the right order.

I ran tests and built everything and it appears to work.

Signed-off-by: Dan Lorenc <[email protected]>
Add Salad Technologies

Signed-off-by: Kyle Dodson <[email protected]>
* fix: fixed cli admin dashboard cmd

Signed-off-by: Soumya Ghosh Dastidar <[email protected]>

* feat: update docs

Signed-off-by: Soumya Ghosh Dastidar <[email protected]>

---------

Signed-off-by: Soumya Ghosh Dastidar <[email protected]>
…rgoproj#15573)

* feat(opentelemetry): ✨ support for secured OTLP endpoint and headers

Signed-off-by: Prashant Shahi <[email protected]>

* docs(opentelemetry): 📝 include new otlp headers in docs

Signed-off-by: Prashant Shahi <[email protected]>

* docs(opentelemetry): 📝 update readme docs as per integration tests

Signed-off-by: Prashant Shahi <[email protected]>

* docs(opentelemetry): 📝 update readme docs as per integration tests

Signed-off-by: Prashant Shahi <[email protected]>

* chore: resolve indentation issues

Signed-off-by: Prashant Shahi <[email protected]>

* chore: fix indentation issues

Signed-off-by: Prashant Shahi <[email protected]>

* chore: include OTLP options in deployment manifests

Signed-off-by: Prashant Shahi <[email protected]>

* fix: update manifests to resolve failing CI

Signed-off-by: Prashant Shahi <[email protected]>

---------

Signed-off-by: Prashant Shahi <[email protected]>
* adding in spec to logappevent params ARGO-1017:

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

* updating logappevent to include spec

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

* updated logevent to take marshal logfields into json

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

* removing spec from parameters just grabbing from app.spec

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

* removing app.spec from test

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

---------

Signed-off-by: Nandini <[email protected]>
Co-authored-by: Nandini Singh <[email protected]>
* fix: upgrade notifications-engine

Signed-off-by: Gilad Salmon <[email protected]>

* update notification-engine version

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

* go mod tidy

Signed-off-by: Michael Crenshaw <[email protected]>

* use correct go version in codeql

Signed-off-by: Michael Crenshaw <[email protected]>

* silly

Signed-off-by: Michael Crenshaw <[email protected]>

* make notifications-docs

Signed-off-by: Michael Crenshaw <[email protected]>

---------

Signed-off-by: Gilad Salmon <[email protected]>
Signed-off-by: pashakostohrys <[email protected]>
Signed-off-by: Michael Crenshaw <[email protected]>
Co-authored-by: pasha-codefresh <[email protected]>
Co-authored-by: Michael Crenshaw <[email protected]>
* fix: Address diff cache miss issues

Signed-off-by: Leonardo Luz Almeida <[email protected]>

* validate mergo.Merge errors

Signed-off-by: Leonardo Luz Almeida <[email protected]>

* Address review comments

Signed-off-by: Leonardo Luz Almeida <[email protected]>

* Allow setting log level at the controller

Signed-off-by: Leonardo Luz Almeida <[email protected]>

* remove unnecessary log setup

Signed-off-by: Leonardo Luz Almeida <[email protected]>

---------

Signed-off-by: Leonardo Luz Almeida <[email protected]>
Signed-off-by: Filip Rafaj <[email protected]>
Co-authored-by: Filip Rafaj <[email protected]>
…es (argoproj#16128)

* cluster.go, projectwindow.go

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

* updated-examples

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

* format-corrected

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

* spell-mistake

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

* format-correction

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

* retrigger

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

* retrigger-2

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

* retirgger-3

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

* update

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

---------

Signed-off-by: Surajyadav <[email protected]>
Co-authored-by: Ishita Sequeira <[email protected]>
* bug: add parent ref node info on resource list

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

* bug: add parent ref node info on resource list

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

* bug: add parent ref node info on resource list

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

* bug: add parent ref node info on resource list

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

* bug: add parent ref node info on resource list

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

* bug: add parent ref node info on resource list

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

---------

Signed-off-by: ashutosh16 <[email protected]>
…unManifestGenAsync not using cache (Issue argoproj#14725) (argoproj#16410)

* fix(repo-server): excess git requests part 1, resolveReferencedSources and runManifestGenAsync

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

* fix: remove unnecessary settings instantiation

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

---------

Signed-off-by: nromriell <[email protected]>
* self service notification

Signed-off-by: May Zhang <[email protected]>

* revert back the changes for redis-ha

Signed-off-by: May Zhang <[email protected]>

* revert back the changes for redis-ha

Signed-off-by: May Zhang <[email protected]>

* update notification engine

Signed-off-by: May Zhang <[email protected]>

* re-trigger build

Signed-off-by: May Zhang <[email protected]>

* self service notification

Signed-off-by: May Zhang <[email protected]>

* revert back the changes for redis-ha

Signed-off-by: May Zhang <[email protected]>

* revert back the changes for redis-ha

Signed-off-by: May Zhang <[email protected]>

* update notification engine

Signed-off-by: May Zhang <[email protected]>

* re-trigger build

Signed-off-by: May Zhang <[email protected]>

* fix conflict

Signed-off-by: May Zhang <[email protected]>

* fix conflict

Signed-off-by: May Zhang <[email protected]>

* fix conflict

Signed-off-by: May Zhang <[email protected]>

* fix conflict

Signed-off-by: May Zhang <[email protected]>

* update notification enginer version

Signed-off-by: May Zhang <[email protected]>

* update notification enginer version

Signed-off-by: May Zhang <[email protected]>

* fixing go tidy

Signed-off-by: May Zhang <[email protected]>

* fixing go tidy

Signed-off-by: May Zhang <[email protected]>

* fixing go tidy

Signed-off-by: May Zhang <[email protected]>

* fixing go tidy

Signed-off-by: May Zhang <[email protected]>

* fixing go tidy

Signed-off-by: May Zhang <[email protected]>

* add back checkAppNotInAdditionalNamespaces

Signed-off-by: May Zhang <[email protected]>

* add cm and secret to clusterRole

Signed-off-by: May Zhang <[email protected]>

* if applicationNamespaces is not used, then use namespaced appClient

Signed-off-by: May Zhang <[email protected]>

* fix merge conflict

Signed-off-by: May Zhang <[email protected]>

* fix doc and test based on review

Signed-off-by: May Zhang <[email protected]>

* self service notification

Signed-off-by: May Zhang <[email protected]>

* revert back the changes for redis-ha

Signed-off-by: May Zhang <[email protected]>

* revert back the changes for redis-ha

Signed-off-by: May Zhang <[email protected]>

* update notification engine

Signed-off-by: May Zhang <[email protected]>

* re-trigger build

Signed-off-by: May Zhang <[email protected]>

* fix conflict

Signed-off-by: May Zhang <[email protected]>

* self service notification

Signed-off-by: May Zhang <[email protected]>

* revert back the changes for redis-ha

Signed-off-by: May Zhang <[email protected]>

* revert back the changes for redis-ha

Signed-off-by: May Zhang <[email protected]>

* update notification engine

Signed-off-by: May Zhang <[email protected]>

* re-trigger build

Signed-off-by: May Zhang <[email protected]>

* fix conflict

Signed-off-by: May Zhang <[email protected]>

* fix conflict

Signed-off-by: May Zhang <[email protected]>

* fix conflict

Signed-off-by: May Zhang <[email protected]>

* update notification enginer version

Signed-off-by: May Zhang <[email protected]>

* update notification enginer version

Signed-off-by: May Zhang <[email protected]>

* fixing go tidy

Signed-off-by: May Zhang <[email protected]>

* fixing go tidy

Signed-off-by: May Zhang <[email protected]>

* fixing go tidy

Signed-off-by: May Zhang <[email protected]>

* fixing go tidy

Signed-off-by: May Zhang <[email protected]>

* fixing go tidy

Signed-off-by: May Zhang <[email protected]>

* add back checkAppNotInAdditionalNamespaces

Signed-off-by: May Zhang <[email protected]>

* add cm and secret to clusterRole

Signed-off-by: May Zhang <[email protected]>

* if applicationNamespaces is not used, then use namespaced appClient

Signed-off-by: May Zhang <[email protected]>

* fix doc and test based on review

Signed-off-by: May Zhang <[email protected]>

* disable defining and using secrets within notification templates for self-service

Signed-off-by: May Zhang <[email protected]>

* tweaks

Signed-off-by: Michael Crenshaw <[email protected]>

* fix docs formatting

Signed-off-by: Michael Crenshaw <[email protected]>

* more docs and Procfile update for local run convenience

Signed-off-by: Michael Crenshaw <[email protected]>

---------

Signed-off-by: May Zhang <[email protected]>
Signed-off-by: Michael Crenshaw <[email protected]>
Co-authored-by: Michael Crenshaw <[email protected]>
@nromriell nromriell force-pushed the lsremote-part2 branch 7 times, most recently from 1597b4c to c32957c Compare December 1, 2023 03:36
JessieTeng89 and others added 3 commits December 1, 2023 10:07
* fix: Tooltips point in wrong direction#11935

Signed-off-by: Teng, Jessie <[email protected]>

* fix: Tooltips point in wrong direction#11935

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

---------

Signed-off-by: Teng, Jessie <[email protected]>
Signed-off-by: Teng <[email protected]>
Co-authored-by: Teng, Jessie <[email protected]>
* fix(11164): Advanced templating using patchTemplate

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

* small changes

Signed-off-by: Michael Crenshaw <[email protected]>

---------

Signed-off-by: gmuselli <[email protected]>
Signed-off-by: Michael Crenshaw <[email protected]>
Co-authored-by: Michael Crenshaw <[email protected]>
…sInAnyNamespaceEnabled flag is set (argoproj#16249)

Signed-off-by: Eilers, Jonas <[email protected]>
dhruvang1 and others added 17 commits December 3, 2023 17:54
…rgoproj#16062) (argoproj#16241)

* fix(appset): store sha from webhook to get latest change during reconcile (argoproj#16062)

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

* fix(appset): Don't use revision cache when reconciling after webhook(argoproj#16062)

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

---------

Signed-off-by: dhruvang1 <[email protected]>
…ssue argoproj#16523) (argoproj#16520)

* Update cert-manager.opcertificate health.lua

Signed-off-by: Chris Murray <[email protected]>

* adding test case for cert issuing

Signed-off-by: Chris Murray <[email protected]>

* fixing typo

Signed-off-by: Chris Murray <[email protected]>

---------

Signed-off-by: Chris Murray <[email protected]>
Wrongly placed horizontal line (`----`) was formatting code-block as a header. Fixed it with a necessary line break

Signed-off-by: Elouan Keryell-Even <[email protected]>
… allow app's deletion (argoproj#12172) (argoproj#16506)

* fix(appset): remove unnecessary condition

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

* docs: update explanation about policy

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

---------

Signed-off-by: mikutas <[email protected]>
…oj#16581)

* chore: upgrade kubernetes dependencies from 0.26.4 to 0.26.11

Fixes some vulnerabilities trivy is reporting on (not necessarily
vulnerabe, trivy tends to have a lot of false positives when it comes to
golang projects):

* CVE-2023-3676
* CVE-2023-3955
* CVE-2023-5528
* CVE-2023-2431
* CVE-2023-2727
* CVE-2023-2728

Signed-off-by: Zoltán Reegn <[email protected]>

* go mod tidy

Signed-off-by: Zoltán Reegn <[email protected]>

* Add go mod tidy to kubernetes updater script

Signed-off-by: Zoltán Reegn <[email protected]>

---------

Signed-off-by: Zoltán Reegn <[email protected]>
@ishitasequeira
Copy link

@nromriell Do you mind rebasing the PR with argo-cd/master to be able to get the PR reviewed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.