-
Notifications
You must be signed in to change notification settings - Fork 44
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
feat(fleet-cmdr): FM-210 initial implementation of fleet-cmdr daemon #363
Conversation
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.
Generally LGTM. I was nit-picky with comments. I will leave it up to orb-software experts to give the final stamp
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.
review wip, will discuss more in our meeting. Overall very good.
d5e19e7
to
1be49aa
Compare
let _request = OrbDetailsRequest::decode(command.payload.as_slice()).unwrap(); | ||
// TODO(paulquinn00): Consult with @oldgalileo and @sfikastheo to determine where to get this info from. | ||
let response = OrbDetailsResponse { | ||
orb_id: "".to_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.
Should usually be fetched through an environment variable which is populated with the /usr/local/bin/orb-id
command.
// TODO(paulquinn00): Consult with @oldgalileo and @sfikastheo to determine where to get this info from. | ||
let response = OrbDetailsResponse { | ||
orb_id: "".to_string(), | ||
orb_name: "".to_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.
/usr/persistent/orb-name
let response = OrbDetailsResponse { | ||
orb_id: "".to_string(), | ||
orb_name: "".to_string(), | ||
jabil_id: "".to_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.
/usr/persistent/jabil-id
orb_id: "".to_string(), | ||
orb_name: "".to_string(), | ||
jabil_id: "".to_string(), | ||
hardware_version: "".to_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.
Get this information from the MCUs or over DBus
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.
can't we just use mcu-util?
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.
Yes, that's fine too. @TheButlah we should probably honestly normalize the collection of all of this early-boot.
hardware_version: "".to_string(), | ||
software_version: "".to_string(), | ||
software_update_version: "".to_string(), | ||
os_release_type: "".to_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.
What does this mean?
software_version: "".to_string(), | ||
software_update_version: "".to_string(), | ||
os_release_type: "".to_string(), | ||
active_slot: "".to_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.
Environment variable filled in by orb-slot-ctrl
, or through the orb-slot-ctrl
library.
software_update_version: "".to_string(), | ||
os_release_type: "".to_string(), | ||
active_slot: "".to_string(), | ||
uptime_seconds: 0, |
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.
When should the uptime be counted?
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.
approving, assuming that @paulquinn00 removes signal handlers and also reworks to get rid of the trait objects (probably using a match statement)
orb_id: "".to_string(), | ||
orb_name: "".to_string(), | ||
jabil_id: "".to_string(), | ||
hardware_version: "".to_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.
can't we just use mcu-util?
let opts = ClientOpts::entity(EntityType::Orb) | ||
.id(args.orb_id.clone().unwrap()) | ||
.endpoint(endpoints.clone()) | ||
.namespace(args.relay_namespace.clone().unwrap()) |
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.
.namespace(args.relay_namespace.clone().unwrap()) | |
.namespace(args.relay_namespace.clone().unwrap()) | |
// TODO: FM-369 |
Co-authored-by: Ryan Butler <[email protected]>
8f6191f
to
57dbb08
Compare
msg: &RecvMessage, | ||
) -> Result<(), OrbCommandError> { | ||
let any = Any::decode(msg.payload.as_slice()).unwrap(); | ||
if any.type_url == OrbDetailsRequest::type_url() { |
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.
it works, although maybe this is better:
match any.type_url {
u if u == OrbDetailsRequest::type_url() => /* ... */ ,
u if u == OrbRebootRequest::type_url() => /* .. */,
}
Dismissing @oldgalileo's review as stale, since @paulquinn00 has agreed to implement the following subsequent PRs:
|
paul will address feedback in subsequent PRs
Fleet Commander (fleet-cmdr) is a daemon which permanently connects to the Orb Relay to receive job commands from the Orb Fleet Backend. This initial implementation of the fleet-cmdr daemon includes the following changes: