-
Notifications
You must be signed in to change notification settings - Fork 111
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-8765 - Add CloudMetadata to MachineStatus #4637
Changes from 30 commits
07a1e6a
184adfa
ffa052c
379e48a
7047e40
ca92ca6
5e3c416
2d2663d
0014486
757b405
96f3bd4
8847686
4506964
234338e
804c587
c3bf357
c7dc40a
3034f19
9d26f97
5fadd59
db7c921
369a8a8
edfa12e
47ed7b7
8d9a2f8
ac22c89
ecffe7d
c7e0db8
1e00b67
ac42a26
3d62334
e8f1c78
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -555,37 +555,30 @@ func (w *GraphNode) transitionTo(state NodeState) { | |
w.transitionedAt = time.Now() | ||
} | ||
|
||
// ResourceStatus returns the current [Status]. | ||
func (w *GraphNode) ResourceStatus() Status { | ||
// Status returns the current [NodeStatus]. | ||
func (w *GraphNode) Status() NodeStatus { | ||
w.mu.RLock() | ||
defer w.mu.RUnlock() | ||
|
||
return w.resourceStatus() | ||
return w.status() | ||
} | ||
|
||
func (w *GraphNode) resourceStatus() Status { | ||
var resName Name | ||
if w.current == nil { | ||
resName = w.config.ResourceName() | ||
} else { | ||
resName = w.current.Name() | ||
} | ||
|
||
func (w *GraphNode) status() NodeStatus { | ||
err := w.lastErr | ||
logger := w.Logger() | ||
|
||
// check invariants between state and error | ||
switch { | ||
case w.state == NodeStateUnhealthy && w.lastErr == nil: | ||
logger.Warnw("an unhealthy node doesn't have an error", "resource", resName) | ||
logger.Warnw("an unhealthy node doesn't have an error") | ||
case w.state == NodeStateReady && w.lastErr != nil: | ||
logger.Warnw("a ready node still has an error", "resource", resName, "error", err) | ||
logger.Warnw("a ready node still has an error", "error", err) | ||
// do not return leftover error in status if the node is ready | ||
err = nil | ||
} | ||
|
||
return Status{ | ||
Name: resName, | ||
// TODO (RSDK-9550): Node should have the correct notion of its name | ||
return NodeStatus{ | ||
State: w.state, | ||
LastUpdated: w.transitionedAt, | ||
Revision: w.revision, | ||
|
@@ -623,8 +616,8 @@ func (w *GraphNode) Stats() any { | |
return ret | ||
} | ||
|
||
// Status encapsulates a resource name along with state transition metadata. | ||
type Status struct { | ||
// NodeStatus encapsulates a resource name along with state transition metadata. | ||
type NodeStatus struct { | ||
Name Name | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could also move Name out of NodeStatus and into resource.Status, but keeping it there for now since I think it would be good to have the name come from the node and not the graph There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://viam.atlassian.net/browse/RSDK-9550 is the ticket, marked it as low since it's low-impact |
||
State NodeState | ||
LastUpdated time.Time | ||
|
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.
the issue here is that test objects are not very flexible and there is not a good way to pass in the correct node name to the resource (especially if going across "remotes")
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.
Could you elaborate on why this won't match the resource name? Is it only in the case of remote resources? Just want an example of a mismatch so I better understand in which cases the original
config.Name
orcurrent.Name()
are incorrect.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.
mock remote resources in tests - especially how I've opted to not actually spin up real servers, the "remote" resources are real resources and no way to get the names to be accurate.