Skip to content

Commit

Permalink
[cmd/opampsupervisor] Fix early exits not shutting down client/server…
Browse files Browse the repository at this point in the history
… in bootstrapping (#31944)

**Description:**
* Fix early exits not shutting down client/server in bootstrapping

**Link to tracking Issue:** Closes #31943

**Testing:**
I tested manually adding errors into the code. There isn't a good,
consistent way to add e.g. the timeout error as-is.

---------

Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>
  • Loading branch information
BinaryFissionGames and djaglowski authored Apr 16, 2024
1 parent 0a33a96 commit 97db72a
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 12 deletions.
13 changes: 13 additions & 0 deletions .chloggen/fix_opamp-supervisor-bootstrap-error-shutdown.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: cmd/opampsupervisor

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Fix collector subprocess not being stopped if bootstrapping fails

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [31943]
24 changes: 12 additions & 12 deletions cmd/opampsupervisor/supervisor/supervisor.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,12 @@ func (s *Supervisor) getBootstrapInfo() (err error) {
return err
}

defer func() {
if stopErr := srv.Stop(context.Background()); stopErr != nil {
err = errors.Join(err, fmt.Errorf("error when stopping the opamp server: %w", stopErr))
}
}()

cmd, err := commander.NewCommander(
s.logger,
s.config.Agent,
Expand All @@ -300,6 +306,12 @@ func (s *Supervisor) getBootstrapInfo() (err error) {
return err
}

defer func() {
if stopErr := cmd.Stop(context.Background()); stopErr != nil {
err = errors.Join(err, fmt.Errorf("error when stopping the collector: %w", stopErr))
}
}()

select {
// TODO make timeout configurable
case <-time.After(3 * time.Second):
Expand All @@ -309,20 +321,8 @@ func (s *Supervisor) getBootstrapInfo() (err error) {
return errors.New("collector's OpAMP client never connected to the Supervisor")
}
case err = <-done:
if err != nil {
return err
}
}

if err = cmd.Stop(context.Background()); err != nil {
return err
}

if err = srv.Stop(context.Background()); err != nil {
return err
}

return nil
}

func (s *Supervisor) Capabilities() protobufs.AgentCapabilities {
Expand Down

0 comments on commit 97db72a

Please sign in to comment.