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

ssa: Skip suspended resources from wait result errors #765

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

stefanprodan
Copy link
Member

@stefanprodan stefanprodan added the area/server-side-apply SSA related issues and pull requests label Apr 23, 2024
Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
@@ -136,6 +136,12 @@ func (m *ResourceManager) WaitForSet(set object.ObjMetadataSet, opts WaitOptions
errs = append(errs, builder.String())
case errors.Is(ctx.Err(), context.DeadlineExceeded) && lastStatus[id].Status != status.CurrentStatus:
var builder strings.Builder
if utils.IsSuspended(lastStatus[id].Resource) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it safe to do this? If I have several resources deployed by a kustomization, and I want them all healthy before triggering another dependent kustomization, skipping a suspended kustomization might have unforseen consequences.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's debatable, if you suspend any Flux object, than all it's dependants will fail to reconcile unless you resume it, even if the object is marked as Ready=True.

@stefanprodan
Copy link
Member Author

stefanprodan commented Apr 23, 2024

Found some issue with this approach, since we remove the suspended resources from the result, this means wait has to reach the timeout. We need to find a way to filter the objects before passing them to the wait function... but without DDoS-ing the API and make new queries for each resource.

@stefanprodan stefanprodan added the hold Issues and pull requests put on hold label Apr 23, 2024
@stefanprodan stefanprodan marked this pull request as draft April 23, 2024 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/server-side-apply SSA related issues and pull requests hold Issues and pull requests put on hold
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants