[Ingest Manager] Improved verify experience #21573
Conversation
michalpristas
added
bug
enhancement
Team:Ingest Management
Ingest Management:beta2
botelastic
bot
added
needs_team
|
Pinging @elastic/ingest-management (Team:Ingest Management) |
|
/package |
| @@ -117,7 +117,7 @@ func (a *Application) Name() string { | |||
|
|
|||
| // Started returns true if the application is started. | |||
| func (a *Application) Started() bool { | |||
| return a.state.Status != state.Stopped | |||
| return a.state.Status != state.Stopped && a.state.Status != state.Crashed && a.state.Status != state.Failed | |||
There was a problem hiding this comment.
Why this change?
I think Started() is trying to say if the application was started at all. If it is crashed or failed you don't want Start() to be called again, because that state is handled internally in the application abstraction.
There was a problem hiding this comment.
yes in case verification fails, it sets state to FAILING
when it passes in second round is tries to start the app but according to this check it is already running so it tries to configure it right away.
i just unified this check with the one in start.go so start.go should be unaffected but operation_start.Check should start functioning as intended (not skipping operation)
| @@ -39,7 +39,7 @@ func (a *Application) start(ctx context.Context, t app.Taggable, cfg map[string] | |||
| }() | |||
|
|
|||
| // already started if not stopped or crashed | |||
| if a.state.Status != state.Stopped && a.state.Status != state.Crashed && a.state.Status != state.Failed { | |||
| if a.Started() { | |||
There was a problem hiding this comment.
The internal start() is called to restart on a crash. We sure this change with the above change is not going to affect that behavior?
There was a problem hiding this comment.
this should be ok we check it only 2 places here (which stays unaffected, it just looks more readable)
and operation_start.Check which is called on config resolution
|
I think this change will fix #21120. As that issue has to do with concurrency of updates, that this added mutex will solve. |
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
|
[Ingest Manager] Improved verify experience (elastic#21573)
What does this PR do?
In this PR agent removes downloaded bits even if asc signature does not match (it was only the case for hash up until now)
Then fetch-verify is retried.
This helps mainly with sceanrio when artifacts are built without
.ascfiles. So asc file downloaded fromsnapshotrepository matches other build as self build binaries included in agent package.IN this case after initial failure, agent removes self build binaries and downloads snapshot from repository including
sha512andascfiles.BUT there's a bit more going on in this PR. there's added lock for update method in
emitterbecause when dynamic input called Set and Config was Loaded it resulted in two concurrent processing of configuration which tried to setup and run own set of beats and they were fighting overpath.datalocation, crashing...This problem sometimes manifested sometimes not, more probably during standalone scenario.
Why is it important?
Checklist
CHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.