-
Notifications
You must be signed in to change notification settings - Fork 525
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
[Controller] Set function status to unhealthy
instead of error
during fast-fail check
#3165
Conversation
…t is in CrashLoopBack or has scheduling issues (not enough resources)
docs/reference/function-configuration/function-configuration-reference.md
Outdated
Show resolved
Hide resolved
docs/reference/function-configuration/function-configuration-reference.md
Outdated
Show resolved
Hide resolved
docs/reference/function-configuration/function-configuration-reference.md
Outdated
Show resolved
Hide resolved
docs/reference/function-configuration/function-configuration-reference.md
Outdated
Show resolved
Hide resolved
docs/reference/function-configuration/function-configuration-reference.md
Outdated
Show resolved
Hide resolved
docs/reference/function-configuration/function-configuration-reference.md
Outdated
Show resolved
Hide resolved
docs/reference/function-configuration/function-configuration-reference.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last thing, that was maybe misunderstood from my previous review
pkg/platform/kube/platform.go
Outdated
// low severity to not over log in the warning | ||
createFunctionOptions.Logger.DebugWithCtx(ctx, "Function creation failed, brief error message extracted", | ||
"briefErrorsMessage", briefErrorsMessage) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leave this log, with the brief error message (will only be visible in the pod logs)
pkg/platform/kube/platform.go
Outdated
@@ -290,6 +283,14 @@ func (p *Platform) CreateFunction(ctx context.Context, createFunctionOptions *pl | |||
} | |||
} | |||
|
|||
if functionStatus.State == functionconfig.FunctionStateUnhealthy { | |||
createFunctionOptions.Logger.WarnWithCtx(ctx, "Function deployment failed, setting state to unhealthy. The issue might be transient or require manual redeployment", | |||
"briefErrorsMessage", briefErrorsMessage) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"briefErrorsMessage", briefErrorsMessage) | |
"err", errorStack.String()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 🚀
unhealthy
instead of error
during fast-fail check
This PR addresses pretty old bug where we set function to error state within the fast-fail controller's mechanism because of either CrashLoop or scheduling issues. So, we could end up with working and healthy function stacked in error state forever, because our approach to Nuclio states assumes that
error
state is a permanent state which can not be changed per time. Whileunhealthy
state is a state that we monitor with FunctionMonitor component, so it is changeable. In light of this, we decided to set function tounhealthy
state in case of any of above described errors occurred.Also,
resolveFailFast
function returns not only error, but also a status to which controller should set the function in case of error. For now, we only set function tounhealthy
state here, but we plan to add more checks, so this change provides a flexibly for future changes.Jira - https://jira.iguazeng.com/browse/NUC-26
[SOLVED - Have decided to change a
warn
log message to show the difference betweenerror
andunhealthy
states]Question: it is crucial to say that if function is unhealthy, we will anyway (with current logic) set the
status.Logs
so that user can understand what the problem is. Taking into account the fact thatunhealthy
status is monitored byFunctionMonitor
we should consider what will happen with the function if it is fixed and is set toready
state. Maybe it worth cleaning upstatus.logs
on that case?Example:
unhealthy