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

[Agent] Change logic to find agent policy during Fleet enrollment #29774

Closed
juliaElastic opened this issue Jan 10, 2022 · 15 comments · Fixed by elastic/fleet-server#1157
Closed
Assignees
Labels
Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team v8.2.0

Comments

@juliaElastic
Copy link
Contributor

In Fleet we are making changes to remove the Default Fleet Server policy from on-prem installations: elastic/kibana#121628
This change is planned for 8.1 version.

This impacts Elastic Agent, as the current Fleet enrolment logic which looks for default policy id will not work anymore.
The proposed change is this (recommended by @blakerouse):

  1. ID given to install or enroll then always that policy, error if no match.
  2. No ID default to fleet-server-policy ID, if it exists. If it doesn't exist fallback to (3).
  3. Find any policy that has a type: fleet-server in the policy. If multiple exist with type: fleet-server then error, due to ambiquity.

elastic/kibana#121628 (comment)

@juliaElastic juliaElastic added Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team v8.1.0 labels Jan 10, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane)

@juliaElastic juliaElastic changed the title [Agent] Change logic to [Agent] Change logic to find agent policy during Fleet enrollment Jan 10, 2022
@jlind23
Copy link
Collaborator

jlind23 commented Jan 10, 2022

Thanks @juliaElastic and @joshdover for raising this up. I am sorry but it's quite late to integrate it in 8.1. Could we keep it for a later release?
ping @ruflin

@ruflin
Copy link
Contributor

ruflin commented Jan 11, 2022

I'm trying to understand why any changes in Elastic Agent are needed. If the first policy is created by default, can't it still have the same name as it does now?

@juliaElastic
Copy link
Contributor Author

@ruflin as part of elastic/kibana#121628 Fleet will no longer create any default policies on setup, and remove the is_default_fleet_server flag altogether.
So that, without a change in Agent, enrolling a fleet server without policy id will not work (as it will not find a default one). The workaround is to always use a policy id.
Policy id is already recommended in Fleet Server on prem instructions on UI.
However, in some places the guides don't include policy id for docker command: https://www.elastic.co/guide/en/fleet/7.16/elastic-agent-container.html#agent-in-container-self

I agree that this dependency was discovered quite late, for me it was not clear initially that we are targeting to remove the is_default_fleet_server flag as part of this change.

If the change in agent is not feasible for 8.1, and the breaking of functionality is not acceptable, we could phase out the Fleet change, and keep the is_default_fleet_server flag until 8.2 when agent can change the logic too.

cc @joshdover @mostlyjason

@ruflin
Copy link
Contributor

ruflin commented Jan 12, 2022

If possible, I would like to split up the changes of "not creating the default policy on startup" from removing is_default and is_default_fleet_server. Doing this in multiple steps will also make it easier for us to understand when and what broke in case we do break things.

The part I still don't understand is why we have to make this breaking change? What happens to policies coming from 8.0 that have is_default set? Will it be remove in 8.1? What if users have automation in place that does not set the id, will it suddenly break with 8.1?

@juliaElastic
Copy link
Contributor Author

@ruflin as discussed with @mostlyjason, we agreed to leave in the is_default flags for 8.1 and create a separate issue for removing them in 8.2.
The intention was to remove the concept of default policies, so that users only install policies (with packages) when they intend to use Fleet.

@ruflin
Copy link
Contributor

ruflin commented Jan 13, 2022

Great to hear, this gives us some more time to figure out if it will be a breaking change and how we could deal with it. Is my assumption correct that we will also keep is_default_fleet_server

@juliaElastic
Copy link
Contributor Author

@ruflin yes, I mean we keep both flags for now

@juliaElastic
Copy link
Contributor Author

juliaElastic commented Jan 13, 2022

@jlind23 just to confirm, is there any similar logic that expects a default policy for enrolling an agent (not fleet server)?
I checked the docs and didn't find it, agent enroll command as an enrollment key argument which determines the policy.

@ruflin
Copy link
Contributor

ruflin commented Jan 14, 2022

@blakerouse Can you comment on the question from @juliaElastic ?

@blakerouse
Copy link
Contributor

@juliaElastic Correct, there is no default in the case of an Elastic Agent enrollment.

@juliaElastic
Copy link
Contributor Author

juliaElastic commented Jan 31, 2022

@blakerouse Actually I found a logic which falls back to default policy in case enrollment token is not specified in agent enroll: https://github.com/elastic/beats/blob/master/x-pack/elastic-agent/pkg/agent/cmd/container.go#L544
Currently:

  • find policy by id or name
  • fallback to default policy

I don't know if it makes sense to change this logic, in case we completely want to move away from marking policies as default. Maybe in lack of a specified policy/token, the logic could try to find a policy that doesn't have fleet_server integration and choose that?

@lykkin
Copy link
Contributor

lykkin commented Feb 8, 2022

It doesn't implement all the logic discussed in this issue, but I started a PR to address this #30256

@juliaElastic I wasn't actually able to find anywhere else in the agent code base that references those flags. There are some more references to kibana I need to run down in the enroll flow, but I couldn't find any references to the default flags.

Currently it just picks the first either fleet/non-fleet policy available based on the package policies assigned to each agent policy if there is no id/name specified.

@lykkin lykkin self-assigned this Feb 8, 2022
@juliaElastic
Copy link
Contributor Author

juliaElastic commented Feb 8, 2022

It doesn't implement all the logic discussed in this issue, but I started a PR to address this #30256

@juliaElastic I wasn't actually able to find anywhere else in the agent code base that references those flags. There are some more references to kibana I need to run down in the enroll flow, but I couldn't find any references to the default flags.

Currently it just picks the first either fleet/non-fleet policy available based on the package policies assigned to each agent policy if there is no id/name specified.

@lykkin What happens if there are more than one fleet/non-fleet policy available? Would the logic pick the first one (which might not be deterministic)? I think for this reason we said to try with the id fleet-server-policy if none specified, and only fallback to any policy if there is one of them (Fleet or NonFleet)

@lykkin
Copy link
Contributor

lykkin commented Feb 8, 2022

Would the logic pick the first one (which might not be deterministic)?
Yeah, this was a concern I had as well. I'll update the PR to handle that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team v8.2.0
Projects
None yet
6 participants