Skip to content

Commit 51c2269

Browse files
authored
RSDK-11248 Disable restart checking logic with new agents and update restart_status endpoint (#5324)
1 parent 7458ff3 commit 51c2269

File tree

5 files changed

+27
-10
lines changed

5 files changed

+27
-10
lines changed

robot/web/web.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -971,14 +971,23 @@ func (svc *webService) Stats() any {
971971
// RestartStatusResponse is the JSON response of the `restart_status` HTTP
972972
// endpoint.
973973
type RestartStatusResponse struct {
974-
// RestartAllowed represents whether this instance of the viam-server can be
974+
// RestartAllowed represents whether this instance of the viamserver can be
975975
// safely restarted.
976976
RestartAllowed bool `json:"restart_allowed"`
977+
// DoesNotHandleNeedsRestart represents whether this instance of the viamserver does
978+
// not check for the need to restart against app itself and, thus, needs agent to do so.
979+
// Newer versions of viamserver (>= v0.9x.0) will report true for this value, while
980+
// older versions won't report it at all, and agent should let viamserver handle
981+
// NeedsRestart logic.
982+
DoesNotHandleNeedsRestart bool `json:"does_not_handle_needs_restart,omitempty"`
977983
}
978984

979985
// Handles the `/restart_status` endpoint.
980986
func (svc *webService) handleRestartStatus(w http.ResponseWriter, r *http.Request) {
981-
response := RestartStatusResponse{RestartAllowed: svc.r.RestartAllowed()}
987+
response := RestartStatusResponse{
988+
RestartAllowed: svc.r.RestartAllowed(),
989+
DoesNotHandleNeedsRestart: true,
990+
}
982991

983992
w.Header().Set("Content-Type", "application/json")
984993
// Only log errors from encoding here. A failure to encode should never

utils/env.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,16 @@ const (
7979

8080
// GetImagesInStreamServerEnvVar is the environment variable that enables the GetImages feature flag in stream server.
8181
GetImagesInStreamServerEnvVar = "VIAM_GET_IMAGES_IN_STREAM_SERVER"
82+
83+
// ViamAgentHandlesNeedsRestartChecking is the environment variable that viam-agent will
84+
// set before starting viam-server to indicate that agent is a new enough version to
85+
// have its own background loop that runs NeedsRestart against app.viam.com to determine
86+
// if the system needs a restart. MUST be kept in line with the equivalent value in the
87+
// agent repo.
88+
//
89+
// TODO(RSDK-12057): Remove sensitivity to this environment variable once we fully
90+
// remove all NeedsRestart checking logic from viam-server.
91+
ViamAgentHandlesNeedsRestartChecking = "VIAM_AGENT_HANDLES_NEEDS_RESTART_CHECKING"
8292
)
8393

8494
// EnvTrueValues contains strings that we interpret as boolean true in env vars.

web/cmd/droid/droid.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ var logger = logging.NewDebugLogger("droid-entrypoint")
1919

2020
// DroidStopHook used by android harness to stop the RDK.
2121
func DroidStopHook() { //nolint:revive
22-
server.ForceRestart = true
22+
// We have no users of this droid code, so we no longer support this method.
23+
logger.Error("DroidStopHook is no longer supported")
2324
}
2425

2526
// MainEntry is called by our android app to start the RDK.

web/server/entrypoint.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,10 @@ func (s *robotServer) serveWeb(ctx context.Context, cfg *config.Config) (err err
529529
// This functionality is tested in `TestLogPropagation` in `local_robot_test.go`.
530530
config.UpdateLoggerRegistryFromConfig(s.registry, fullProcessedConfig, s.logger)
531531

532-
if fullProcessedConfig.Cloud != nil {
532+
// Only start cloud restart checker if cloud config is non-nil, and viam-agent is not
533+
// handling restart checking for us (relevant environment variable is unset).
534+
if fullProcessedConfig.Cloud != nil && os.Getenv(rutils.ViamAgentHandlesNeedsRestartChecking) == "" {
535+
s.logger.CInfo(ctx, "Agent does not handle checking needs restart functionality; will handle in server")
533536
cloudRestartCheckerActive = make(chan struct{})
534537
utils.PanicCapturingGo(func() {
535538
defer close(cloudRestartCheckerActive)

web/server/restart_checker.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,7 @@ type needsRestartCheckerGRPC struct {
2727
client rpc.ClientConn
2828
}
2929

30-
// ForceRestart lets random other parts of the app request an exit.
31-
var ForceRestart bool
32-
3330
func (c *needsRestartCheckerGRPC) needsRestart(ctx context.Context) (bool, time.Duration, error) {
34-
if ForceRestart {
35-
return true, minNeedsRestartCheckInterval, nil
36-
}
3731
service := apppb.NewRobotServiceClient(c.client)
3832
res, err := service.NeedsRestart(ctx, &apppb.NeedsRestartRequest{Id: c.cfg.ID})
3933
if err != nil {

0 commit comments

Comments
 (0)