[Controller] Enrich invocation status for unhealthy function #3448
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TomerShor
reviewed
Jan 12, 2025
Comment on lines
+406
to
+412
| // if function is unhealthy, it might be invokable, and | ||
| // it can become ready, so enrich invocation status | ||
| if functionErrorState == functionconfig.FunctionStateUnhealthy && resources != nil { | ||
| if err := fo.populateFunctionInvocationStatus(function, functionStatus, resources); err != nil { | ||
| return errors.Wrap(err, "Failed to populate function invocation status") | ||
| } | ||
| } |
There was a problem hiding this comment.
There's only 1 place the function state can be set to unhealthy (line 275). Why not handle it specifically there instead of in the more general setFunctionError function?
There was a problem hiding this comment.
@TomerShor because status object is defined there. I think, if doing it properly, the whole code of this function should be refactored and creation of any additional status object should be avoided because status is already a part of the function. The way it's done in the pr to fix it is the least intrusive I can think of.
Also, imagine we have another place of setting function to unhealthy. In this case the approach where we set function in the CreateOrUpdate won't be a very good idea
TomerShor
approved these changes
Jan 12, 2025
rokatyy
added a commit
to rokatyy/nuclio
that referenced
this pull request
Mar 6, 2025
…3448) Enrich function invocation URLs even if function is Unhealthy, because it can still be moved to Ready status and then the URLs are valid. jira - https://iguazio.atlassian.net/browse/NUC-331
TomerShor
pushed a commit
that referenced
this pull request
Mar 6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Enrich function invocation URLs even if function is Unhealthy, because it can still be moved to Ready status and then the URLs are valid.
jira - https://iguazio.atlassian.net/browse/NUC-331