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

bug: cache does not get invalidated on resource deletion? #446

Open
nazarewk opened this issue Aug 16, 2022 · 3 comments · May be fixed by #461
Open

bug: cache does not get invalidated on resource deletion? #446

nazarewk opened this issue Aug 16, 2022 · 3 comments · May be fixed by #461

Comments

@nazarewk
Copy link

nazarewk commented Aug 16, 2022

TLDR: I believe that with before-hook-creation under heavy load our ArgoCD (2.3.3 on EKS) processes a PreSync phase as Completed before fully processing resource (Job) deletion.

This is a followup from argoproj/argo-cd#10077

Probably watch event arrives late or gets stuck in a queue for a while and I could not track down any cache invalidation during deletion in argo-cd or gitops-engine itself.

// delete anything that need deleting
hooksPendingDeletion := createTasks.Filter(func(t *syncTask) bool { return t.deleteBeforeCreation() })
if hooksPendingDeletion.Len() > 0 {
ss := newStateSync(state)
for _, task := range hooksPendingDeletion {
t := task
ss.Go(func(state runState) runState {
sc.log.WithValues("dryRun", dryRun, "task", t).V(1).Info("Deleting")
if !dryRun {
err := sc.deleteResource(t)
if err != nil {
// it is possible to get a race condition here, such that the resource does not exist when
// delete is requested, we treat this as a nop
if !apierr.IsNotFound(err) {
state = failed
sc.setResourceResult(t, "", common.OperationError, fmt.Sprintf("failed to delete resource: %v", err))
}
} else {
// if there is anything that needs deleting, we are at best now in pending and
// want to return and wait for sync to be invoked again
state = pending
}
}
return state
})
}
state = ss.Wait()
}

@nazarewk nazarewk changed the title bug: resource cache does not get invalidated on deletion? bug: cache does not get invalidated on resource deletion? Aug 16, 2022
@nazarewk
Copy link
Author

nazarewk commented Aug 17, 2022

After brief look into the code seems like the fix would be simple, but still an architectural change:

  1. in addition to waiting for event: add cache invalidation at https://github.com/argoproj/gitops-engine/blob/master/pkg/sync/sync_context.go#L1009-L1016
  2. pass and store cache on syncContext
  3. make deleteAPIResource public

WDYT?

@nazarewk
Copy link
Author

TODO: try to find a place to improve/introduce "has the deletion completed?" or "is the object fresh enough after deletion policy: before-hook-creation?" kind of checks.

nazarewk added a commit to surgeventures/gitops-engine that referenced this issue Sep 13, 2022
nazarewk added a commit to surgeventures/gitops-engine that referenced this issue Sep 13, 2022
nazarewk added a commit to surgeventures/gitops-engine that referenced this issue Sep 13, 2022
- fixes argoproj#446
- closes argoproj/argo-cd#10579
- original issue argoproj/argo-cd#10077

Signed-off-by: Krzysztof Nazarewski <[email protected]>
nazarewk added a commit to surgeventures/gitops-engine that referenced this issue Sep 13, 2022
- fixes argoproj#446
- closes argoproj/argo-cd#10579
- original issue argoproj/argo-cd#10077

Signed-off-by: Krzysztof Nazarewski <[email protected]>
nazarewk added a commit to surgeventures/gitops-engine that referenced this issue Sep 14, 2022
- fixes argoproj#446
- closes argoproj/argo-cd#10579
- original issue argoproj/argo-cd#10077

Signed-off-by: Krzysztof Nazarewski <[email protected]>
thober35 pushed a commit to thober35/gitops-engine that referenced this issue Oct 25, 2023
- fixes argoproj#446
- closes argoproj/argo-cd#10579
- original issue argoproj/argo-cd#10077

Signed-off-by: Krzysztof Nazarewski <[email protected]>
thober35 pushed a commit to thober35/gitops-engine that referenced this issue Oct 26, 2023
thober35 added a commit to thober35/gitops-engine that referenced this issue Oct 26, 2023
thober35 added a commit to thober35/gitops-engine that referenced this issue Oct 26, 2023
thober35 added a commit to thober35/gitops-engine that referenced this issue Nov 6, 2023
thober35 added a commit to thober35/gitops-engine that referenced this issue Nov 6, 2023
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 a pull request may close this issue.

1 participant