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

RSDK-8595 Unhealthy resource state #4257

Merged
merged 27 commits into from
Aug 20, 2024

Conversation

maximpertsov
Copy link
Member

@maximpertsov maximpertsov commented Aug 2, 2024

Expose an Unhealthy state along with the related error on resource.Status.

Details

stateDiagram-v2
[*] --> Unconfigured
Unconfigured --> Configuring
Unconfigured --> Removing
Configuring --> Ready
Configuring --> Unhealthy
Ready --> Configuring
Unhealthy --> Ready
Ready --> Removing
Unhealthy --> Removing
Removing --> Configuring
Removing --> [*]
Loading

Depends on

@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Aug 2, 2024
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Aug 2, 2024
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Aug 2, 2024
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Aug 2, 2024
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Aug 2, 2024
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Aug 3, 2024
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Aug 3, 2024
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Aug 8, 2024
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Aug 20, 2024
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Aug 20, 2024
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Aug 20, 2024
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Aug 20, 2024
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Aug 20, 2024
resource/graph_node.go Outdated Show resolved Hide resolved
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Aug 20, 2024
@maximpertsov maximpertsov changed the title RSDK-7903 Unhealthy resource state RSDK-8595 Unhealthy resource state Aug 20, 2024
Copy link
Member

@benjirewis benjirewis left a comment

Choose a reason for hiding this comment

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

LGTM! Just a final question.

func (w *GraphNode) getLoggerOrGlobal() logging.Logger {
if w.logger == nil {
// This node has not yet been configured with a logger - use the global logger as
// a fall-back.
Copy link
Member

Choose a reason for hiding this comment

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

[q] Any idea when this happens? Just want to get a sense of when resources are logging in the absence of a logger. Seems like a violation of my mental model of graph node loggers...

Copy link
Member Author

@maximpertsov maximpertsov Aug 20, 2024

Choose a reason for hiding this comment

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

i forgot the specifics but there is a lag between when a node is first created and when it receives an initial logger. if we remove the w.logger == nil guards from this file then we will get a nil-access panic pretty quickly.

I suppose we should use this function through the file instead of relying on the guard to get more uniform behavior -- happy to do this as a follow-up.

EDIT: something tells me the lag is specific to modular resources, since we pass logging configs to them over a unix socket after startup.

@maximpertsov maximpertsov merged commit 4c6b388 into viamrobotics:main Aug 20, 2024
19 checks passed
@maximpertsov maximpertsov deleted the RSDK-7903-unhealthy-state branch August 20, 2024 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to test This pull request is marked safe to test from a trusted zone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants