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 is_default and is_default_fleet_server flag from ECK recipes #6678

Closed
juliaElastic opened this issue Apr 11, 2023 · 3 comments · Fixed by #6724
Closed

Remove is_default and is_default_fleet_server flag from ECK recipes #6678

juliaElastic opened this issue Apr 11, 2023 · 3 comments · Fixed by #6724
Assignees

Comments

@juliaElastic
Copy link
Contributor

Flags is_default and is_default_fleet_server are deprecated since 8.1.

Remove these from ECK recipes:
https://github.com/elastic/cloud-on-k8s/blob/main/config/recipes/elastic-agent/fleet-kubernetes-integration.yaml#L30

The change should be tested, so that Fleet on Kubernetes still works after removing these flags.

Originally posted by @jen-huang in elastic/ingest-docs#76 (comment)

@barkbay
Copy link
Contributor

barkbay commented Apr 11, 2023

It's actually not just a doc change. We do rely on these flags to detect what policy should be used:

func (f fleetAPI) defaultFleetServerPolicyID(ctx context.Context) (string, error) {
policy, err := f.findAgentPolicy(ctx, func(policy Policy) bool {
return policy.IsDefaultFleetServer && policy.Status == "active"
})
if err != nil {
return "", err
}
return policy.ID, nil
}
func (f fleetAPI) defaultAgentPolicyID(ctx context.Context) (string, error) {
policy, err := f.findAgentPolicy(ctx, func(policy Policy) bool {
return policy.IsDefault && policy.Status == "active"
})
if err != nil {
return "", err
}
return policy.ID, nil
}

I can see 2 alternatives so far:

  • We could find another way to detect defaults (I have some ideas but not familiar enough with the Fleet API...)
  • Make policyID a mandatory field (which would be a breaking change):
apiVersion: agent.k8s.elastic.co/v1alpha1
kind: Agent
metadata:
  name: fleet-server
spec:
  policyID: eck-fleet-server ## Here
  version: 8.7.0
  kibanaRef:
    name: kibana
  elasticsearchRefs:
  - name: elasticsearch
  mode: fleet
  fleetServerEnabled: true
  deployment:
    replicas: 1
    podTemplate:
      spec:
        serviceAccountName: fleet-server
        automountServiceAccountToken: true
        securityContext:
          runAsUser: 0
---
apiVersion: agent.k8s.elastic.co/v1alpha1
kind: Agent
metadata: 
  name: elastic-agent
spec:
  policyID: eck-agent ## Here
  version: 8.7.0
  kibanaRef:
    name: kibana
  fleetServerRef: 
    name: fleet-server
  mode: fleet
  daemonSet:
    podTemplate:
      spec:
        serviceAccountName: elastic-agent
        hostNetwork: true
        dnsPolicy: ClusterFirstWithHostNet
        automountServiceAccountToken: true
        securityContext:
          runAsUser: 0

@barkbay barkbay self-assigned this Apr 24, 2023
@barkbay
Copy link
Contributor

barkbay commented Apr 24, 2023

We could find another way to detect defaults (I have some ideas but not familiar enough with the Fleet API...)

My idea was to automatically detect a "Default Fleet Server policy" and a "Default Agent Policy" as long as the following conditions were met:

  • There are only 2 policies.
  • One of these 2 policies can clearly be detected as a "Fleet Server policy". That policy would have been the "Default Fleet Server Policy" Consequently the other one would have been the "Default Agent Policy".

Unfortunately, without is_default and is_default_fleet_server, I don't think we can select a default policy from the Fleet API answer:

{
	"items": [{
		"id": "eck-agent",
		"namespace": "default",
		"monitoring_enabled": ["logs", "metrics"],
		"name": "Elastic Agent on ECK policy",
		"unenroll_timeout": 900,
		"inactivity_timeout": 1209600,
		"is_preconfigured": true,
		"status": "active",
		"is_managed": false,
		"revision": 3,
		"updated_at": "2023-04-24T14:44:26.541Z",
		"updated_by": "system",
		"schema_version": "1.0.0",
		"agents": 0
	}, {
		"id": "eck-fleet-server",
		"namespace": "default",
		"monitoring_enabled": ["logs", "metrics"],
		"name": "Fleet Server on ECK policy",
		"unenroll_timeout": 900,
		"inactivity_timeout": 1209600,
		"is_preconfigured": true,
		"status": "active",
		"is_managed": false,
		"revision": 2,
		"updated_at": "2023-04-24T14:44:20.503Z",
		"updated_by": "system",
		"schema_version": "1.0.0",
		"agents": 0
	}],
	"total": 2,
	"page": 1,
	"perPage": 20
}

While poking around with the Fleet API, by using the full parameter in the query (agent_policies?perPage=20&full=true), I noticed that the Fleet Server policy holds a package named fleet_server. But it seems a bit "hacky" or "brittle" do detect the default Fleet Server policy that way? 😕 By "brittle" I mean that I have the feeling that we would rely on something (a specific policy name), which should be out of the scope of ECK.

		"package_policies": [{
			"id": "fleet_server-1",
			"version": "WzY5MSwxXQ==",
			"name": "fleet_server-1",
			"namespace": "default",
			"package": {
				"name": "fleet_server", ## Here
				"title": "Fleet Server",
				"version": "1.2.0"
			},

@juliaElastic please let me know if my reasoning is correct 🙏

Unless someone else has an idea, I think I'll create a PR to make policyID a required field and flag this as a breaking change.

@juliaElastic
Copy link
Contributor Author

@barkbay Yes, the policy with the fleet_server integration is the fleet server policy.

Would it be possible to remove the is_default flags from the recipes and pass the policyID instead, but also keep the default detection logic in place to avoid the breaking change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants