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.

Sign up for GitHub

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

Include upgrade details in diagnostics bundle during ongoing upgrade #3624

Merged
merged 16 commits into from
Nov 3, 2023
Merged

Include upgrade details in diagnostics bundle during ongoing upgrade #3624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
7bc8fc8
Remove context and handle cancellation internally instead
ycombinator Oct 12, 2023
b6bd1ea
More optimizations
ycombinator Oct 12, 2023
28ce34f
Add back context
ycombinator Oct 12, 2023
0ec900a
Adding FSM for upgrades
ycombinator Oct 3, 2023
5682966
Implementing TODO
ycombinator Oct 3, 2023
ba3e4f7
WIP
ycombinator Oct 4, 2023
ff0a038
WIP
ycombinator Oct 4, 2023
1730cd9
Reorganizing imports
ycombinator Oct 4, 2023
3cd9f84
Running go mod tidy
ycombinator Oct 4, 2023
b1f368f
Fixing booboos introduced during conflict resolution
ycombinator Oct 13, 2023
f98765d
Add nil guard
ycombinator Oct 17, 2023
2f864f0
Setting logger in test
ycombinator Oct 17, 2023
f7dbb2d
Include upgrade details in diagnostics bundle
ycombinator Oct 18, 2023
cba774d
Adding CHANGELOG entry
ycombinator Oct 18, 2023
f93d5e2
Add unit test
ycombinator Nov 1, 2023
a8aed84
Add godoc
ycombinator Nov 1, 2023
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
32 changes: 32 additions & 0 deletions changelog/fragments/1697588586-upgrade-details-in-diagnostics.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Kind can be one of:
# - breaking-change: a change to previously-documented behavior
# - deprecation: functionality that is being removed in a later release
# - bug-fix: fixes a problem in a previous version
# - enhancement: extends functionality but does not break or fix existing behavior
# - feature: new functionality
# - known-issue: problems that we are aware of in a given version
# - security: impacts on the security of a product or a user’s deployment.
# - upgrade: important information for someone upgrading from a prior version
# - other: does not fit into any of the other categories
kind: enhancement

# Change summary; a 80ish characters long description of the change.
summary: Include upgrade details in diagnostics bundle.

# Long description; in case the summary is not enough to describe the change
# this field accommodate a description without length limits.
# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment.
#description:

# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
component: elastic-agent

# PR URL; optional; the PR number that added the changeset.
# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
# Please provide it if you are adding a fragment for a different PR.
pr: https://github.com/elastic/elastic-agent/pull/3624

# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
# If not present is automatically filled by the tooling with the issue linked to the PR number.
#issue: https://github.com/owner/repo/1234
26 changes: 14 additions & 12 deletions internal/pkg/agent/application/coordinator/coordinator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -791,12 +791,13 @@ func (c *Coordinator) DiagnosticHooks() diagnostics.Hooks {
State runtime.ComponentState `yaml:"state"`
}
type StateHookOutput struct {
State agentclient.State `yaml:"state"`
Message string `yaml:"message"`
FleetState agentclient.State `yaml:"fleet_state"`
FleetMessage string `yaml:"fleet_message"`
LogLevel logp.Level `yaml:"log_level"`
Components []StateComponentOutput `yaml:"components"`
State agentclient.State `yaml:"state"`
Message string `yaml:"message"`
FleetState agentclient.State `yaml:"fleet_state"`
FleetMessage string `yaml:"fleet_message"`
LogLevel logp.Level `yaml:"log_level"`
Components []StateComponentOutput `yaml:"components"`
UpgradeDetails *details.Details `yaml:"upgrade_details,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

I don't see any changes in the tests related to this change... could you please add a unit test to document what we can expect in the hook output when UpgradeDetails is populated ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Added in 02625e8.

}

s := c.State()
Expand All @@ -809,12 +810,13 @@ func (c *Coordinator) DiagnosticHooks() diagnostics.Hooks {
}
}
output := StateHookOutput{
State: s.State,
Message: s.Message,
FleetState: s.FleetState,
FleetMessage: s.FleetMessage,
LogLevel: s.LogLevel,
Components: compStates,
State: s.State,
Message: s.Message,
FleetState: s.FleetState,
FleetMessage: s.FleetMessage,
LogLevel: s.LogLevel,
Components: compStates,
UpgradeDetails: s.UpgradeDetails,
Copy link
Member

Choose a reason for hiding this comment

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

When is this field present? Do we get details about the upgrade after a failure ? Or those get cleared as soon as the grace period is over ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When is this field present?

This field is populated when the upgrade process starts:

elastic-agent/internal/pkg/agent/application/coordinator/coordinator.go

Lines 475 to 476 in 2a93086

det := details.NewDetails(version, details.StateRequested, actionID)
det.RegisterObserver(c.SetUpgradeDetails)

Do we get details about the upgrade after a failure ?

Yes, if the upgrade process fails at any point, this field will continue to remain populated.

Or those get cleared as soon as the grace period is over ?

I assume you are referring to the Upgrade Watcher's grace period.

Exactly when we clear it out after that is not yet implemented. We need to decide whether we want to clear it out based on some timeout (maybe the grace period or maybe something even beyond that) or due to some user intervention (which could be retrying the upgrade or something else just to clear the upgrade details, not sure yet) or either (whichever happens first).

}
o, err := yaml.Marshal(output)
if err != nil {
Expand Down
29 changes: 25 additions & 4 deletions internal/pkg/agent/application/coordinator/diagnostics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,18 @@ package coordinator
import (
"context"
"errors"
"fmt"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gopkg.in/yaml.v2"

"github.com/elastic/elastic-agent-client/v7/pkg/proto"

"github.com/elastic/elastic-agent-client/v7/pkg/client"
"github.com/elastic/elastic-agent-client/v7/pkg/proto"

"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/details"
"github.com/elastic/elastic-agent/internal/pkg/agent/configuration"
"github.com/elastic/elastic-agent/internal/pkg/agent/transpiler"
monitoringCfg "github.com/elastic/elastic-agent/internal/pkg/core/monitoring/config"
Expand Down Expand Up @@ -413,6 +415,7 @@ func TestDiagnosticState(t *testing.T) {
// Create a coordinator with a test state and verify that the state
// diagnostic reports it

now := time.Now().UTC()
state := State{
State: agentclient.Starting,
Message: "starting up",
Expand All @@ -432,9 +435,19 @@ func TestDiagnosticState(t *testing.T) {
},
},
},
UpgradeDetails: &details.Details{
TargetVersion: "8.12.0",
State: "UPG_DOWNLOADING",
ActionID: "foobar",
Metadata: details.Metadata{
DownloadPercent: 0.17469,
ScheduledAt: now,
DownloadRate: 123.56,
},
},
}

expected := `
expected := fmt.Sprintf(`
state: 0
message: "starting up"
fleet_state: 1
Expand All @@ -451,7 +464,15 @@ components:
version_info:
name: "version name"
version: "version value"
`
upgrade_details:
target_version: 8.12.0
state: UPG_DOWNLOADING
action_id: foobar
metadata:
download_percent: 0.17469
scheduled_at: %s
download_rate: 123.56
`, now.Format(time.RFC3339Nano))

coord := &Coordinator{
// This test needs a broadcaster since the components-actual diagnostic
Expand Down
26 changes: 16 additions & 10 deletions internal/pkg/agent/application/upgrade/details/details.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,30 +24,36 @@ type Observer func(details *Details)

// Details consists of details regarding an ongoing upgrade.
type Details struct {
TargetVersion string `json:"target_version"`
State State `json:"state"`
ActionID string `json:"action_id,omitempty"`
Metadata Metadata `json:"metadata"`
TargetVersion string `json:"target_version" yaml:"target_version"`
State State `json:"state" yaml:"state"`
ActionID string `json:"action_id,omitempty" yaml:"action_id,omitempty"`
Metadata Metadata `json:"metadata" yaml:"metadata"`

observers []Observer
mu sync.Mutex
}

// Metadata consists of metadata relating to a specific upgrade state
type Metadata struct {
ScheduledAt time.Time `json:"scheduled_at,omitempty"`
DownloadPercent float64 `json:"download_percent,omitempty"`
DownloadRate downloadRate `json:"download_rate,omitempty"`
ScheduledAt time.Time `json:"scheduled_at,omitempty" yaml:"scheduled_at,omitempty"`

// DownloadPercent is the percentage of the artifact that has been
// downloaded. Minimum value is 0 and maximum value is 1.
DownloadPercent float64 `json:"download_percent,omitempty" yaml:"download_percent,omitempty"`

// DownloadRate is the rate, in bytes per second, at which the download
// is progressing.
DownloadRate downloadRate `json:"download_rate,omitempty" yaml:"download_rate,omitempty"`

// FailedState is the state an upgrade was in if/when it failed. Use the
// Fail() method of UpgradeDetails to correctly record details when
// an upgrade fails.
FailedState State `json:"failed_state,omitempty"`
FailedState State `json:"failed_state,omitempty" yaml:"failed_state,omitempty"`

// ErrorMsg is any error message encountered if/when an upgrade fails. Use
// the Fail() method of UpgradeDetails to correctly record details when
// an upgrade fails.
ErrorMsg string `json:"error_msg,omitempty"`
ErrorMsg string `json:"error_msg,omitempty" yaml:"error_msg,omitempty"`
}

func NewDetails(targetVersion string, initialState State, actionID string) *Details {
Expand All @@ -71,7 +77,7 @@ func (d *Details) SetState(s State) {
}

// SetDownloadProgress is a convenience method to set the download percent
// when the upgrade is in UPG_DOWNLOADING state.
// and download rate when the upgrade is in UPG_DOWNLOADING state.
func (d *Details) SetDownloadProgress(percent, rateBytesPerSecond float64) {
d.mu.Lock()
defer d.mu.Unlock()
Expand Down
3 changes: 1 addition & 2 deletions internal/pkg/agent/application/upgrade/step_download.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ import (
"strings"
"time"

"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/details"

"github.com/cenkalti/backoff/v4"

"go.elastic.co/apm"
Expand All @@ -26,6 +24,7 @@ import (
"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/artifact/download/http"
"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/artifact/download/localremote"
"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/artifact/download/snapshot"
"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/details"
"github.com/elastic/elastic-agent/internal/pkg/agent/errors"
"github.com/elastic/elastic-agent/internal/pkg/release"
"github.com/elastic/elastic-agent/pkg/core/logger"
Expand Down
Loading