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.

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

Remove created agents in service deployer #1771

Merged
merged 7 commits into from
Apr 16, 2024

Conversation

mrodm
Copy link
Contributor

@mrodm mrodm commented Apr 15, 2024

Part of #787

Currently, service deployers like custom_agent or kubernetes create new Elastic Agent instances and those agents are never removed (nor unenrolled) from Fleet once the tests are finished (system tests).

This PR removes these Elastic Agents created during tests from Fleet, so the stack state after running the tests is always without those Elastic Agents.

Moreover, to avoid errors like this if it is a long test execution:

 $ kubectl logs -n kube-system -f pod/elastic-agent-5ln2w
Error: unable to find enrollment token named "Default" in policy "Elastic-Agent (elastic-package)"
For help, please see our troubleshooting guide at https://www.elastic.co/guide/en/fleet/8.12/fleet-troubleshooting.html

Those new Elastic Agents are enrolled directly with the test Agent Policy created in each test.

@mrodm mrodm self-assigned this Apr 15, 2024
@mrodm mrodm marked this pull request as ready for review April 15, 2024 16:11
@mrodm mrodm requested a review from a team April 15, 2024 16:11
internal/servicedeployer/kubernetes.go Outdated Show resolved Hide resolved
internal/servicedeployer/kubernetes.go Outdated Show resolved Hide resolved
Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>
@mrodm
Copy link
Contributor Author

mrodm commented Apr 16, 2024

test integrations

@elasticmachine
Copy link
Collaborator

Created or updated PR in integrations repository to test this version. Check elastic/integrations#9604

@mrodm
Copy link
Contributor Author

mrodm commented Apr 16, 2024

test integrations

@elasticmachine
Copy link
Collaborator

Created or updated PR in integrations repository to test this version. Check elastic/integrations#9604

@elasticmachine
Copy link
Collaborator

elasticmachine commented Apr 16, 2024

💔 Build Failed

Failed CI Steps

History

cc @mrodm

@mrodm
Copy link
Contributor Author

mrodm commented Apr 16, 2024

After testing with the integrations repository, it was noticed that kubernetes package could not enroll an Elastic Agent in one of the latest data streams that it has defined.

The error raised when testing in that data stream was:


 $ kubectl logs -n kube-system -f pod/elastic-agent-5ln2w
Error: unable to find enrollment token named "Default" in policy "Elastic-Agent (elastic-package)"
For help, please see our troubleshooting guide at https://www.elastic.co/guide/en/fleet/8.12/fleet-troubleshooting.html

To avoid this kind of errors, in those scenarios where new Elastic Agents are created, those agents will be enrolled using the new Agent Policy created in each test. Changes done in 096811f

Updated description accordingly.

@mrodm mrodm requested a review from jsoriano April 16, 2024 11:31
if !r.options.RunIndependentElasticAgent {
r.removeAgentHandler = func(ctx context.Context) error {
// When not using independent agents, service deployers like kubernetes or custom agents create new Elastic Agent
createdNewAgent := svcInfo.Agent.Host.NamePrefix == "docker-custom-agent" || svcInfo.Agent.Host.NamePrefix == "kind-control-plane"
Copy link
Member

Choose a reason for hiding this comment

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

Nit. Depending on the name of the agent may be fragile, we may forget to modify this if the set of names change.

Copy link
Contributor Author

@mrodm mrodm Apr 16, 2024

Choose a reason for hiding this comment

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

I could add a new field into svcInfo to set whether or not this service requires a new Agent (different from the stack).

Maybe something like this (adding Stack field) ?

	// Agent related properties.
	Agent struct {
		// Host describes the machine which is running the agent.
		Host struct {
			// Name prefix for the host's name
			NamePrefix string
		}
		// Stack indicates whether or not this Agent is the one running/set in "elastic-package stack up"
		Stack bool
	}

WDYT @jsoriano ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or maybe change it to be Independent

svcInfo.Agent.Independent (true or false)

Copy link
Member

Choose a reason for hiding this comment

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

Both options SGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perfect, I'll merge this PR as it is then, and I'll create a follow-up PR to remove those agent names with one of these options and run the tests with integrations again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created PR #1776

@mrodm mrodm merged commit f9ed754 into elastic:main Apr 16, 2024
3 checks passed
@mrodm mrodm deleted the remove_created_agents branch April 16, 2024 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants