-
Notifications
You must be signed in to change notification settings - Fork 463
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
Fleet Server package #544
Fleet Server package #544
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
|
Pinging @elastic/agent (Team:Agent) |
@ycombinator This is a package without a data_stream which I think is valid? Update: change would need to happen here? https://github.com/elastic/package-spec/blob/master/versions/1/spec.yml#L25 |
@@ -0,0 +1,23 @@ | |||
{ |
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.
@aleksmaus Very interesting that it just works with the dot prefix. It will make it fun to develop as some OS hide these files :-)
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.
you mean windowz? :-)
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.
OS X ...
"index_patterns": [ | ||
".fleet-servers" | ||
], | ||
"template": { |
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 wonder how restrictive we should be in these templates. Should we allow additional fields or not? If yes, keyword
as default?
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.
For upgrades would that be good to add to all the templates? Just incase that Fleet Server itself is upgraded before the package?
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.
Probably. In the ideal scenario the template would of course always be first and we could even use strict mappings, but not the case for now.
@aleksmaus What I suggest to keep moving here is adding a "fake" data_stream to pass validation, merge PR, promote it to snapshot so we can start using this for testing in Kibana / fleet-server. I think we will have follow up discussions around more details in the template and ILM policy but this can come later. |
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 good, some questions and comments.
packages/fleet_server/manifest.yml
Outdated
title: Fleet Server | ||
description: Fleet Server setup | ||
inputs: | ||
- type: fleet_server |
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.
Change type to fleet-server
. Fleet Server already expects the configuration file format to be:
inputs:
- type: fleet-server
host: 0.0.0.0
port: 8000
packages/fleet_server/manifest.yml
Outdated
inputs: | ||
- type: fleet_server | ||
vars: | ||
# TODO: Mainly for testing, probably no config options at 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 think we should do host binding and port binding.
Which are the fields host
and port
.
packages/fleet_server/manifest.yml
Outdated
required: true | ||
show_user: true | ||
default: | ||
- 127.0.0.1 |
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.
Should default to 0.0.0.0
.
packages/fleet_server/manifest.yml
Outdated
default: | ||
- 127.0.0.1 | ||
title: "Fleet Server parts" | ||
description: "Hello World Fleet Server" |
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.
Should add port with default set to 8000
(as it's the default now). We should probably change that.
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.
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.
LGTM. It doesn't make it into the policy yet but lets get it in so we have all the assets.
title: Fleet Server | ||
description: Fleet Server setup | ||
inputs: | ||
- type: fleet-server |
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.
This does not end up in the policy currently as we don't use it in any of the data streams and have a template for it. Maybe a good use for our fake data stream?
We had a discussion around this for pure input packages. I'm trying to find the issue.
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.
Ah, found it: elastic/kibana#83878 We can take inspiration from apm package: https://github.com/elastic/package-storage/blob/snapshot/packages/apm/0.1.0-dev.4/agent/input/template.yml.hbs
Hi @aleksmaus, @ruflin, I was on PTO all last week and am catching up on PRs today. I see that this PR was merged with a fake |
* Add fleet-server package * add ilm policy examples * fix template formatting * fix priority so it does not conflict * apply proper formatting * Functional indices boostrap * Make lint happier * Address review feedback. Add fake datastream, to pass linter * Better Fleet Server configuration description Co-authored-by: ruflin <spam@ruflin.com>
Bootstraps all the indexes needed for the Fleet Server once the integration is installed.
@ruflin @blakerouse
Since it creates only ILM policies and templates, will have another PR for the Fleet Server to make it resilient to the situation where the indexes do not exists yet.
Closing the initial draft #540 that @ruflin started.