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-9440 Report machine state through GetMachineStatus #4616

Merged
merged 32 commits into from
Jan 9, 2025
Merged
Changes from 1 commit
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
9026f00
initial impl
benjirewis Dec 10, 2024
53b9d29
atomic
benjirewis Dec 10, 2024
4802b8d
start config watcher goroutine last
benjirewis Dec 11, 2024
8fa9830
addrderef -> CopyOnlyPublicFields
benjirewis Dec 11, 2024
c78e16e
update api dependency and lint
benjirewis Dec 11, 2024
62c3f5e
more lint sigh
benjirewis Dec 11, 2024
3b9d6b9
basic entrypoint test
benjirewis Dec 11, 2024
d97e583
reconfigure in goroutine and check for state running in client new
benjirewis Dec 13, 2024
58b4b94
typos
benjirewis Dec 13, 2024
eacd561
client test inject fixes
benjirewis Dec 13, 2024
4948b13
move out of initializing in setupLocalRobot
benjirewis Dec 16, 2024
d756396
add log lines for debugging 32 bit tests
benjirewis Dec 16, 2024
fcf03b5
move running check higher
benjirewis Dec 16, 2024
09b76fd
potentially simpler API
benjirewis Dec 18, 2024
35f8f3d
more missing injections
benjirewis Dec 18, 2024
e299ab4
fix more tests sigh
benjirewis Dec 18, 2024
19f7940
maybe this time
benjirewis Dec 18, 2024
d68e6f4
actually use value from options
benjirewis Dec 18, 2024
2d8bc1b
make configWatcher a method instead of an anonymous function
benjirewis Dec 19, 2024
f997bbb
put back lints
benjirewis Dec 19, 2024
f97feef
try to simplify updateWeakDependents check in completeConfig
benjirewis Dec 22, 2024
72bada6
avoid CopyOnlyPublicFields
benjirewis Jan 2, 2025
fc67364
initializing -> running test
benjirewis Jan 2, 2025
18555b3
move redefinition of context above slow shutdown goroutine
benjirewis Jan 3, 2025
e46aa9d
remove debug logs
benjirewis Jan 3, 2025
7f0fffd
table drive and fix server GetMachineStatus test
benjirewis Jan 3, 2025
da02e54
better table driving ?
benjirewis Jan 3, 2025
2c591ad
dan comments
benjirewis Jan 3, 2025
d5e484f
fix client test
benjirewis Jan 6, 2025
5bed7f2
Initial field on Config in place of SetInitializing
benjirewis Jan 7, 2025
e00ad7d
test that local package file is handled properly
benjirewis Jan 7, 2025
3e6e59e
final comments
benjirewis Jan 9, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
avoid CopyOnlyPublicFields
benjirewis committed Jan 3, 2025
commit 72bada67f01ebca40fc8628efbbfb26cc73ed30d
9 changes: 3 additions & 6 deletions web/server/entrypoint.go
Original file line number Diff line number Diff line change
@@ -477,10 +477,7 @@ func (s *robotServer) serveWeb(ctx context.Context, cfg *config.Config) (err err
// and immediately start web service. We need the machine to be reachable
// through the web service ASAP, even if some resources take a long time to
// initially configure.
minimalProcessedConfig, err := fullProcessedConfig.CopyOnlyPublicFields()
if err != nil {
return err
}
minimalProcessedConfig := *fullProcessedConfig
minimalProcessedConfig.Components = nil
minimalProcessedConfig.Services = nil
minimalProcessedConfig.Remotes = nil
@@ -489,7 +486,7 @@ func (s *robotServer) serveWeb(ctx context.Context, cfg *config.Config) (err err

// Start robot in an initializing state with minimal config.
robotOptions = append(robotOptions, robotimpl.WithInitializing())
Copy link
Member

Choose a reason for hiding this comment

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

What's the consequence of not having this line?

  • Do we go back to the old behavior of having to do an initial reconfigure before opening a server port?
  • Or do we still open a server port before configuring -- but we just lie to clients that the robot is in "MachineState.RUNNING"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Or do we still open a server port before configuring -- but we just lie to clients that the robot is in "MachineState.RUNNING"?

This one is the consequence; the robot would falsely report itself as running.

myRobot, err := robotimpl.New(ctx, minimalProcessedConfig, s.logger, robotOptions...)
myRobot, err := robotimpl.New(ctx, &minimalProcessedConfig, s.logger, robotOptions...)
if err != nil {
cancel()
return err
@@ -527,7 +524,7 @@ func (s *robotServer) serveWeb(ctx context.Context, cfg *config.Config) (err err
}()

// Create initial web options with `minimalProcessedConfig`.
options, err := s.createWebOptions(minimalProcessedConfig)
options, err := s.createWebOptions(&minimalProcessedConfig)
if err != nil {
return err
}