-
Notifications
You must be signed in to change notification settings - Fork 163
[ECK] Update documentation for 3.1.0 #2019
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
Changes from 5 commits
b696c13
8f9ab83
9eb86fc
02d6b31
a87dc88
a2b1869
8bfed5d
10267d2
9138b0d
8359ee0
fb200bd
eb216f8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -146,8 +146,23 @@ By default, every reference targets all instances in your {{es}}, {{kib}} and {{ | |||||
|
||||||
## Customize {{agent}} configuration [k8s-elastic-agent-fleet-configuration-custom-configuration] | ||||||
|
||||||
In contrast to {{agents}} in standalone mode, the configuration is managed through {{fleet}}, and it cannot be defined through `config` or `configRef` elements. | ||||||
In contrast to {{agents}} in standalone mode, the configuration is managed through {{fleet}}, and it cannot be defined through `config` or `configRef` elements with a few exceptions. | ||||||
|
||||||
One of those exceptions is the configuration of providers as described in [advanced Agent configuration managed by Fleet](/reference/fleet/advanced-kubernetes-managed-by-fleet.md). When {{agent}} is managed by {{fleet}} and is orchestrated by ECK, the configuration of providers can simply be done through the `.spec.config` element in the Agent resource as of {applies_to}`stack: ga 8.13`: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @barkbay : what do we mean with the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Possible version switching the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This content was already reviewed here without notes: #1446 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @pebrc for the extra details! My opinion is still the same but of course it's not a big deal. Also when we reviewed the linked PR, I think the Anyway the current text and usage of the badge is all right too, so whatever you want. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can totally change it to whatever makes most sense from a docs perspective. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perfect, let's allow @shainaraskas to share her thoughts for a final decision :) Shaina, do you like the usage of the inline There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that this is not ideal, partially because our labels don't look right in sentences. one reason this is hard to reframe is that this is positioned as "one of these exceptions" - is this the only exception? are exceptions only valid as of 8.13? this could get an if that doesn't make sense, I'd go with prose inline or a note inline. we'll have to refactor it later when we have more components at our disposal, but will read better in the short term. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I approved this PR already but some ECK 3.1 tagging should be added here before this is shipped |
||||||
|
||||||
```yaml | ||||||
apiVersion: agent.k8s.elastic.co/v1alpha1 | ||||||
kind: Agent | ||||||
metadata: | ||||||
name: elastic-agent | ||||||
spec: | ||||||
config: | ||||||
fleet: | ||||||
enabled: true | ||||||
providers.kubernetes: | ||||||
add_resource_metadata: | ||||||
deployment: true | ||||||
``` | ||||||
|
||||||
## Upgrade the {{agent}} specification [k8s-elastic-agent-fleet-configuration-upgrade-specification] | ||||||
|
||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because these docs are going to continue to be valid for 3.0, we need to keep the 3.0 compatibility in this doc present.
could do it in tabs, table, or bulleted list. quickest hack would be:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think tabs would be the most appropriate, but let's try your suggestion first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll now try the tabs...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tab variant:
I think I'll stick with tabs, happy to get feedbacks though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any option is fine. I like both.
The list will probably not scale well when having 8-10 releases to mention, so better the tabs for that reason probably.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a strong argument but this is already the case today. Also I can only remember users asking if the last version of K8s or OpenShift is supported by ECK. My feeling is that we're going to have to revisit this in the future, whatever our choice is today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyways, I created this issue: elastic/docs-builder#1570
Maybe this could be a case for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if we go with tabs, the whole list will need to go in the tabs, because it's weird to split a list between tabs and not tabs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking at the mock of the bulleted list, I think removing the colon would make it look right if you were still considering using it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, I'll do the change