[Elastic Agent] Add install/uninstall sub-command #21206

blakerouse · 2020-09-22T01:51:01Z

What does this PR do?

Adds a new install and uninstall command to the Elastic Agent.

install is used to install the Elastic Agent onto the system persistently. This will place Elastic Agent in the correct location for the platform, register it as a service and start the service. install also has the ability to detect if an installation is broken and offer the option to overwrite that installation with the executing non-installed Elastic Agent. install cannot be called from the installed version on the system

uninstall is used to uninstall the Elastic Agent on the current system. This can only be called from the installed Elastic Agent. If the uninstall command is executed from a non-installed Elastic Agent it will provide information on how to run it from the installed Elastic Agent, if it is installed on the system.

Why is it important?

Provides a uniform path to installed the Elastic Agent onto the system, as well as allows that Elastic Agent to be self-upgradable.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
~~[ ] I have made corresponding changes to the documentation~~
~~[ ] I have made corresponding change to the default configuration files~~
~~[] I have added tests that prove my fix is effective or that my feature works~~ (impossible to unit test)
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

With a built package run

./elastic-agent install

Once installed run

elastic-agent uninstall

Related issues

elasticmachine · 2020-09-22T01:52:42Z

Pinging @elastic/ingest-management (Team:Ingest Management)

elasticmachine · 2020-09-22T02:10:03Z

💚 Build Succeeded

Expand to view the summary

Build stats

Build Cause: [Pull request #21206 updated]
Start Time: 2020-09-29T12:39:07.389+0000
Duration: 87 min 11 sec

Test stats 🧪

Test	Results
Failed	0
Passed	20309
Skipped	1858
Total	22167

blakerouse · 2020-09-22T12:52:23Z

CI failures are unrelated

ph · 2020-09-22T12:57:42Z

x-pack/elastic-agent/pkg/agent/warn/warn.go

@@ -21,4 +21,5 @@ func LogNotGA(log *logger.Logger) {
 // PrintNotGA writes to the received writer that the Agent is not GA.
 func PrintNotGA(output io.Writer) {
 	fmt.Fprintln(output, message)
+	fmt.Fprintln(output)


This removes the print of the message?

No it just adds an extra blank line between the warning and the next message. It just makes it easier to read when using the sub-commands.

Ah I've missread the diff :(

ph · 2020-09-22T19:23:34Z

@blakerouse Can you check with @EricDavisX for testing this PR, I think code review and some testing would allow us to catch any bugs early.

blakerouse · 2020-09-24T17:25:43Z

/package

ruflin

Started to review the code but realised I must run it locally to get the full experience and comment on it. Post my comments already but more to come.

ruflin · 2020-09-28T08:01:36Z

NOTICE.txt

@@ -10907,6 +10907,36 @@ OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
 WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.


+--------------------------------------------------------------------------------
+Dependency : github.com/blakerouse/service


We should probably move this repo under elastic?

At the moment this is just pointing to a fix of the upstream, once this lands in the upstream repo then the dependency will not point to my personal repo but instead it will point to: https://github.com/kardianos/service

Which is what is defined in the go.mod, its just that I am using replace at the moment to get the needed fix for systemd on link from my repository.

Can you file an issue to switch it over when upstream is fixed. Otherwise we forget for sure ;-)

ruflin · 2020-09-28T08:02:51Z

dev-tools/notice/rules.json

@@ -9,7 +9,8 @@
    "ISC",
    "MIT",
    "MPL-2.0",
-    "Public Domain"
+    "Public Domain",
+    "Zlib"


I wonder if we should add this in a separate PR to have it "documented" in case someone looks for the diff so it does not look like "accidentally" added.

ruflin · 2020-09-28T08:04:03Z

x-pack/elastic-agent/pkg/agent/application/paths/paths.go

-	fs.StringVar(&logsPath, "path.logs", initialHome, "Logs path contains Agent log output")
+	fs.StringVar(&topPath, "path.home", topPath, "Agent root path")
+	fs.StringVar(&configPath, "path.config", configPath, "Config path is the directory Agent looks for its config file")
+	fs.StringVar(&logsPath, "path.logs", logsPath, "Logs path contains Agent log output")


Agent and process log output?

This is just the human read-able agent.log file. All the JSON log files including the logs for the processes go under ${paths.home}/data/logs.

We should have a follow up discussion if it is worth to keep the agent.log special. Good for now.

x-pack/elastic-agent/pkg/agent/application/paths/paths.go

x-pack/elastic-agent/pkg/agent/application/upgrade/step_relink.go

ruflin · 2020-09-28T08:15:11Z

x-pack/elastic-agent/pkg/agent/cmd/install.go

+
+	err := install.Install()
+	if err != nil {
+		return fmt.Errorf("Error: %s", err)


Perhaps a bit more details? That it is an error is probably already known.

The detail is already in the err. This is just to get the nice clear message in the CLI output and for it to exit with exit code not 0.

ruflin · 2020-09-28T08:15:33Z

x-pack/elastic-agent/pkg/agent/cmd/install.go

+		if !force {
+			confirm, err := c.Confirm("Elastic Agent will be installed onto your system and will run as a service. Do you want to continue?", true)
+			if err != nil {
+				return fmt.Errorf("Error: problem reading prompt response")


Nit: Should we skip Error: as it is already an error? Same above?

The Error: prefix on all of the fmt.Errorf in the install or uninstall commands is so that when the error is printed to the console it is known as an error to the user, otherwise without it its not super clear.

This also returns an error so that the command will exit with a non-zero exit code, so if installer is being ran form a script its easy to know if it worked or failed.

ruflin · 2020-09-28T08:17:11Z

x-pack/elastic-agent/pkg/agent/control/server/listener_windows.go

+	if err != nil {
+		return "", errors.Wrap(err, "failed to get current user")
+	}
+	// Named pipe security and access rights.


Thanks for the docs here. Later devs will appreciate it.

ruflin · 2020-09-28T08:32:40Z

x-pack/elastic-agent/pkg/agent/cmd/install.go

+		}
+	} else {
+		if !force {
+			confirm, err := c.Confirm("Elastic Agent will be installed onto your system and will run as a service. Do you want to continue?", true)


It would be nice to tell the user under which path it will be installed here.

Also should we tell them about where the wrapper is installed?

Added the path, I don't think we should mention the wrapper as that is just going to make the question even longer and don't want to make it confusing.

ruflin · 2020-09-28T08:35:16Z

x-pack/elastic-agent/pkg/agent/install/paths_darwin.go

+	BinaryName = "elastic-agent"
+
+	// InstallPath is the installation path using for install command.
+	InstallPath = "/Library/Elastic/Agent"


I somehow stumbled over this path as I expected it under /opt but I'm probably just used to this from homebrew.

What is the reason it is Elastic/Agent and not elastic-agent or similar in one word? Same question applies to all other OS? Is uppercase recommended? Do we expect other elastic tools to be installed here?

Hm, I now stumbled over the elastic-agent shell wrapper below, things are making now more sense.

This is so Elastic Endpoint will be next to each other.

Endpoint is installed at /Library/Elastic/Endpoint so we install agent at /Library/Elastic/Agent. This is the same for all OS's so we can be next to Endpoint, which make it cleaner from a user perspective of seeing everything that is installed from Elastic.

Makes sense to align it with Endpoint.

blakerouse

Thanks for the review. I added inline comments to your questions and I am pushing a new commit to address some of the changes you suggested.

blakerouse · 2020-09-28T12:14:50Z

NOTICE.txt

@@ -10907,6 +10907,36 @@ OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
 WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.


+--------------------------------------------------------------------------------
+Dependency : github.com/blakerouse/service


At the moment this is just pointing to a fix of the upstream, once this lands in the upstream repo then the dependency will not point to my personal repo but instead it will point to: https://github.com/kardianos/service

Which is what is defined in the go.mod, its just that I am using replace at the moment to get the needed fix for systemd on link from my repository.

blakerouse · 2020-09-28T12:15:51Z

dev-tools/notice/rules.json

@@ -9,7 +9,8 @@
    "ISC",
    "MIT",
    "MPL-2.0",
-    "Public Domain"
+    "Public Domain",
+    "Zlib"


I can move it to a single PR just for this one thing. Might be weird to add in an individual PR, think it makes better sense in context.

blakerouse · 2020-09-28T12:16:58Z

x-pack/elastic-agent/pkg/agent/application/paths/paths.go

-	fs.StringVar(&logsPath, "path.logs", initialHome, "Logs path contains Agent log output")
+	fs.StringVar(&topPath, "path.home", topPath, "Agent root path")
+	fs.StringVar(&configPath, "path.config", configPath, "Config path is the directory Agent looks for its config file")
+	fs.StringVar(&logsPath, "path.logs", logsPath, "Logs path contains Agent log output")


This is just the human read-able agent.log file. All the JSON log files including the logs for the processes go under ${paths.home}/data/logs.

x-pack/elastic-agent/pkg/agent/application/upgrade/step_relink.go

blakerouse · 2020-09-28T12:20:58Z

x-pack/elastic-agent/pkg/agent/cmd/install.go

+		}
+	} else {
+		if !force {
+			confirm, err := c.Confirm("Elastic Agent will be installed onto your system and will run as a service. Do you want to continue?", true)


Added the path, I don't think we should mention the wrapper as that is just going to make the question even longer and don't want to make it confusing.

blakerouse · 2020-09-28T12:22:33Z

x-pack/elastic-agent/pkg/agent/cmd/install.go

+		if !force {
+			confirm, err := c.Confirm("Elastic Agent will be installed onto your system and will run as a service. Do you want to continue?", true)
+			if err != nil {
+				return fmt.Errorf("Error: problem reading prompt response")


The Error: prefix on all of the fmt.Errorf in the install or uninstall commands is so that when the error is printed to the console it is known as an error to the user, otherwise without it its not super clear.

This also returns an error so that the command will exit with a non-zero exit code, so if installer is being ran form a script its easy to know if it worked or failed.

blakerouse · 2020-09-28T12:23:07Z

x-pack/elastic-agent/pkg/agent/cmd/install.go

+
+	err := install.Install()
+	if err != nil {
+		return fmt.Errorf("Error: %s", err)


The detail is already in the err. This is just to get the nice clear message in the CLI output and for it to exit with exit code not 0.

blakerouse · 2020-09-28T12:24:58Z

x-pack/elastic-agent/pkg/agent/install/paths_darwin.go

+	BinaryName = "elastic-agent"
+
+	// InstallPath is the installation path using for install command.
+	InstallPath = "/Library/Elastic/Agent"


This is so Elastic Endpoint will be next to each other.

Endpoint is installed at /Library/Elastic/Endpoint so we install agent at /Library/Elastic/Agent. This is the same for all OS's so we can be next to Endpoint, which make it cleaner from a user perspective of seeing everything that is installed from Elastic.

ruflin · 2020-09-29T08:47:48Z

@blakerouse Can you check what is up with CI. @michalpristas would be great if you could also take a look at the change.

michalpristas · 2020-09-29T10:52:15Z

testing on linux so far:
Scenario UNTAR:

unpacked, ran install failed: Exit code 1:

The Elastic Agent is currently in BETA and should not be used in production

Elastic Agent is installed but currently broken: service is not installed
Continuing will re-install Elastic Agent over the current installation at /opt/Elastic/Agent. Do you want to continue? [Y/n]:y
Error: failed to install service (elastic-agent): exit status 1

Maybe related to ^^ but running uninstall and install after ^ now complained that *.service file is already there

sudo ./elastic-agent install
The Elastic Agent is currently in BETA and should not be used in production

Elastic Agent will be installed at /opt/Elastic/Agent and will run as a service. Do you want to continue? [Y/n]:y
Error: failed to install service (elastic-agent): Init already exists: /etc/systemd/system/elastic-agent.service

Scenario DEB:

dpkg -i *.deb
systemctl start elastic-agent
Failed: Sep 29 10:46:26 vagrant systemd[1]: [/etc/systemd/system/elastic-agent.service:3] Missing '='.

michalpristas · 2020-09-29T11:18:36Z

On windows it installed ok, was running fine

blakerouse · 2020-09-29T12:00:08Z

@michalpristas The Linux issue looks like you already had an elastic-agent installed. The error seems to be that you already had a elastic-agent.service file present on your system? Did you have the .deb installed? Are you sure the system is clean? Did you purge uninstall it before trying the install command.

Looking at the errors as you reported them look correct to me. I think that is the correct behavior, it should not overwrite the installed elastic-agent.service file. The reason is that if it does, and then if the *.deb was installed or not uninstalled properly and that is fixed it will then remove the elastic-agent.service. So its better to be caution on install in this case.

michalpristas · 2020-09-29T12:19:50Z

yeah i had it installed and i uninstalled it before trying, systemd is persistent in this case, tried removing files manually and daemon reload it but i was not able to make it go away.

michalpristas

after service file fix it worked for me ok

* Add install command. * Fix binary name on darwin. * Fix install of mkdir. * Update shell wrapper path for darwin. * Add ability to fix broken installation. * Fix issues with install/uninstall. * Add dedicated uninstall command. * Add enrollment at the end of install command. * Fix installation of shell wrapper. * Fix root_windows.go * Fix uninstall on Windows. * Add sleep to removal on Windows. * Fix sleep to timeout in uninstall subcommand. * Fix uninstall for nested timeout with cmd.exe. * Fix uninstall on windows to actually sleep correctly. * Fix formatting. * Add changelog. * Fixes for mage check. * Create the symlink on Windows on install, fix issue with enroll questions during install. * Refactor to remove the paths.yml and symlink dance on Windows, because new install command handles that. * Cleanup repeat GA warning. * Fix symlink on Windows. * Fix control socket on windows. * Fix socket path and authority on Windows. * Fix username matching for SYSTEM. * Fix socket path on darwin. * Fix darwin service name. Fix uninstall on systemd. * Fix install on linux. * Fix RunningInstalled on Linux. * Prevent upgrade unless conditions are correct. * Add ability to force upgradable with DEV=true when built. * Fixes from code review. * Fix issue with service description. (cherry picked from commit 2996b6f)

…ci-build-label-support * upstream/master: [JJBB] Set shallow cloning to 10 (elastic#21409) docs: add link to release notes for 7.9.2 (elastic#21405) (elastic#21419) docs: Prepare Changelog for 7.9.2 (elastic#21229) (elastic#21403) fix: mark flaky tests (elastic#21300) fix: use a fixed version of setuptools (elastic#21393) Move Kubernetes events metricset to its own block in reference config (elastic#21407) [libbeat] Enable WriteAheadLimit in the disk queue (elastic#21391) docs: fix apt/yum formatting (elastic#21362) Fix shutdown tracking in s3 input (elastic#21380) [libbeat] Fix position writing in the disk queue Add UBI 8 image to the dependencies report (elastic#21374) Fix debug message to show actual SQS message ID (elastic#20614) [Elastic Agent] Rename *ConfigChange to PolicyChange (elastic#20779) [Elastic Agent] Add install/uninstall sub-command (elastic#21206) [Filebeat][httpjson] Make httpjson use cursor input when using date cursor (elastic#20751) feat: prepare release pipelines (elastic#21238) Add IP validation to Security module (elastic#21325)

* Add install command. * Fix binary name on darwin. * Fix install of mkdir. * Update shell wrapper path for darwin. * Add ability to fix broken installation. * Fix issues with install/uninstall. * Add dedicated uninstall command. * Add enrollment at the end of install command. * Fix installation of shell wrapper. * Fix root_windows.go * Fix uninstall on Windows. * Add sleep to removal on Windows. * Fix sleep to timeout in uninstall subcommand. * Fix uninstall for nested timeout with cmd.exe. * Fix uninstall on windows to actually sleep correctly. * Fix formatting. * Add changelog. * Fixes for mage check. * Create the symlink on Windows on install, fix issue with enroll questions during install. * Refactor to remove the paths.yml and symlink dance on Windows, because new install command handles that. * Cleanup repeat GA warning. * Fix symlink on Windows. * Fix control socket on windows. * Fix socket path and authority on Windows. * Fix username matching for SYSTEM. * Fix socket path on darwin. * Fix darwin service name. Fix uninstall on systemd. * Fix install on linux. * Fix RunningInstalled on Linux. * Prevent upgrade unless conditions are correct. * Add ability to force upgradable with DEV=true when built. * Fixes from code review. * Fix issue with service description. (cherry picked from commit 2996b6f)

blakerouse added the Team:Ingest Management label Sep 22, 2020

blakerouse self-assigned this Sep 22, 2020

botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Sep 22, 2020

blakerouse marked this pull request as ready for review September 22, 2020 01:52

ph reviewed Sep 22, 2020

View reviewed changes

blakerouse force-pushed the agent-install-command branch from 380d24a to 4768af3 Compare September 22, 2020 12:57

blakerouse added 18 commits September 22, 2020 11:11

Add install command.

09fabb5

Fix binary name on darwin.

f99d7f9

Fix install of mkdir.

4fabf0a

Update shell wrapper path for darwin.

a6f0549

Add ability to fix broken installation.

92d64da

Fix issues with install/uninstall.

de393a3

Add dedicated uninstall command.

86983aa

Add enrollment at the end of install command.

29e507b

Fix installation of shell wrapper.

7a7716a

Fix root_windows.go

d54768e

Fix uninstall on Windows.

d260e25

Add sleep to removal on Windows.

3099b66

Fix sleep to timeout in uninstall subcommand.

c7eeb77

Fix uninstall for nested timeout with cmd.exe.

2c6f0bb

Fix uninstall on windows to actually sleep correctly.

189b1e0

Fix formatting.

1eb59ae

Add changelog.

387e4c1

Fixes for mage check.

2d9e0bb

blakerouse force-pushed the agent-install-command branch from 4768af3 to 2d9e0bb Compare September 22, 2020 15:11

This was referenced Sep 24, 2020

[Elastic Agent] Report to Fleet when Agent can be upgraded #21318

Closed

[Elastic Agent] Disable upgrade capability of Agent #21001

Closed

ph requested review from michalpristas and ruflin September 24, 2020 17:50

blakerouse mentioned this pull request Sep 24, 2020

[Ingest Manager] Send snapshot flag together with metadata #21285

Merged

6 tasks

ruflin reviewed Sep 28, 2020

View reviewed changes

Fixes from code review.

df6e995

blakerouse commented Sep 28, 2020

View reviewed changes

Fix issue with service description.

ee370ce

blakerouse mentioned this pull request Sep 29, 2020

Filebeat error is displayed in activity logs on installing elastic agent with new install command #21378

Closed

michalpristas approved these changes Sep 29, 2020

View reviewed changes

blakerouse merged commit 2996b6f into elastic:master Sep 29, 2020

blakerouse deleted the agent-install-command branch September 29, 2020 15:01

blakerouse mentioned this pull request Sep 29, 2020

Cherry-pick #21206 to 7.x: [Elastic Agent] Add install/uninstall sub-command #21387

Merged

3 tasks

blakerouse added the v7.10.0 label Sep 29, 2020

dedemorton mentioned this pull request Oct 2, 2020

Document Elastic Agent install and uninstall commands #21499

Closed

ph mentioned this pull request Oct 19, 2020

Separate enroll and run Elastic agent into two steps elastic/kibana#65749

Closed

ph mentioned this pull request Nov 17, 2020

in-product instructions for deploying an agent need to be spread to multiple commands elastic/integrations#249

Closed

[Elastic Agent] Add install/uninstall sub-command #21206

[Elastic Agent] Add install/uninstall sub-command #21206

Conversation

blakerouse commented Sep 22, 2020 • edited Loading

What does this PR do?

Why is it important?

Checklist

How to test this PR locally

Related issues

elasticmachine commented Sep 22, 2020

elasticmachine commented Sep 22, 2020 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

blakerouse commented Sep 22, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ph commented Sep 22, 2020

blakerouse commented Sep 24, 2020

ruflin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blakerouse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin commented Sep 29, 2020

michalpristas commented Sep 29, 2020 • edited Loading

michalpristas commented Sep 29, 2020

blakerouse commented Sep 29, 2020

michalpristas commented Sep 29, 2020

michalpristas left a comment

Choose a reason for hiding this comment

blakerouse commented Sep 22, 2020 •

edited

Loading

elasticmachine commented Sep 22, 2020 •

edited by jenkins-beats-ci bot

Loading

michalpristas commented Sep 29, 2020 •

edited

Loading