[Ingest Manage] Remove installed services on agent uninstall #24151
Conversation
michalpristas
added
bug
needs_backport
|
Pinging @elastic/agent (Team:Agent) |
|
Pinging @elastic/ingest-management (Team:Ingest Management) |
botelastic
bot
added
needs_team
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪💚 Flaky test reportTests succeeded. Expand to view the summary
Test stats 🧪
|
…nto uninstall-endpoint
There was a problem hiding this comment.
Really this seems mostly mechanical with all the refactoring of paths defined in install to the paths module.
I think there is an overall issue with dynamic input variables and conditions that might need to be handled.
There is also another approach to this, which is to use the control protocol. We could add an uninstall operation to the control protocol that can be called on the running Elastic Agent daemon. Then the daemon can perform the uninstall before it shuts down. That would ensure that based on the current variable resolution and conditions that uninstall of programs will be the true programs that are currently installed.
The negative of that approach is that the Elastic Agent must be running, which we could force but does provide a weird experience in the case that a user is actually trying to uninstall Elastic Agent.
| return nil | ||
| } | ||
|
|
||
| func programsFromConfig(cfg *config.Config) ([]program.Program, error) { |
There was a problem hiding this comment.
How is this code block going to handle dynamic input variables and conditions on inputs? It's possible that an input is only enabled based on a condition. Without that condition being resolved the Elastic Agent would not know truly which programs should be running.
I think this block needs to be wrapped in the composable inputs composer, so that variable resolution can occur and conditions can be applied.
There was a problem hiding this comment.
you;re right here, i need to add the code block to resolve dynamic inputs here.
this code block is also a copy paste and needs to be removed after refactor. i want to extract config resolution to a single package which can be used everywhere in a same manner. we have config resolutions in run, inspect, uninstall ...
so this was planned as a first step to fix the bug and refactor as a second step removing all unnecessary code
|
@blakerouse i was thinking about having uninstall in control but what in case agent refuses to start. then you dont have option to connect to it. we need to handle all the cases. i think having uninstall independent of running agent is safer. as i was mentioning in other issue, let's not change the behavior 2 weeks before release. let's do it proper way, create issue discuss consequences pros and cons and decide whether we should go for it. i know this is slower path but 7.12 is so close, let's fix bugs, atm QAS is unable to test windows at all. |
|
@blakerouse i added constraints and dynamics in a bit heartbreaking way. as i mentioned, everything is packed in application package which makes it difficult to recall from other places without circular dep (this will be solved in followup refactor) |
|
|
||
| // Dynamic inputs section | ||
| // TODO(michal): move to shared code during refactoring | ||
| func renderInputs(inputs transpiler.Node, varsArray []*transpiler.Vars) (transpiler.Node, error) { |
There was a problem hiding this comment.
I know you marked this as TODO. But I don't think we should land this duplicated. This code path is identical to the emitter piece.
With input templates this will need to be updated again. The duplication will most likely result in this code path being forgot or missed.
There was a problem hiding this comment.
let me think what we can do here
There was a problem hiding this comment.
Being this is going to go into 7.12. We can leave it, but lets quickly clean this up in a follow up.
There was a problem hiding this comment.
moved to transpiler, works good
|
/package |
…#24151) [Ingest Manage] Remove installed services on agent uninstall (elastic#24151)
…#24151) [Ingest Manage] Remove installed services on agent uninstall (elastic#24151)
What does this PR do?
This PR is a first step to call unenroll from uninstall
Due to how our code is structured we cannot easily do that. This PR moves around the least amount of code to call uninstall on installed programs. Further refactor is needed and planned so we can call full unenroll and decouple current codebase a bit.
Although this PR seems big what it does it that during agent uninstall
uninstallon each installed programWhy is it important?
Removes installed services (endpoint) on agent removal.
Checklist
CHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.