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.

Sign up for GitHub

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

Jump to bottom

ssa: Skip suspended resources from wait result errors #765

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

ssa: Skip suspended resources from wait result errors #765

wants to merge 1 commit into from
+64 −1

Conversation

stefanprodan
Copy link
Member

@stefanprodan stefanprodan added the area/server-side-apply SSA related issues and pull requests label Apr 23, 2024
souleb
souleb reviewed Apr 23, 2024
View reviewed changes
ssa/manager_wait.go
@@ -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 •
edited
Loading

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
Reviewers

@souleb souleb souleb left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
area/server-side-apply SSA related issues and pull requests hold Issues and pull requests put on hold
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@stefanprodan @souleb