-
Notifications
You must be signed in to change notification settings - Fork 729
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
Introduce EnterpriseSearch CRD #2688
Conversation
Bootstrap the Enterprise Search CRD & controller
This is cleaner, more similar to what we do for other resources, and avoids leaking secrets in Pod environment variables. Note that Pods do not get automatically rotated when config changes so far.
This is heavily based on the APMServer association controller. Would largely benefit from some common refactoring in the future.
I don't know why this isn't there in the first place. I plan to investigate in the future, but for now this is required to perform status updates.
Setup the EnterpriseSearch - Elasticsearch association controller
We want Pods to be rotated with the new config.
Let's query /swiftype-app-version which seems to be the default out there. The initial bootstrap is pretty slow so I set an InitialDelaySeconds of 60sec.
I *think* this user can end up being useful if Elasticsearch is down hence the native realm cannot be used.
This fixes CRD generation following a rebase on master.
Generate self-signed certificates for Enterprise Search and enable TLS encryption by default. This can be disabled in the EnterpriseSearch spec, and user-provided certificates can be used instead.
Depending on the TLS spec, let's use the correct protocol: http or https.
Automatically rotate the Pod if: - the ent search config changes - the ent search TLS certs change - the referenced Elasticsearch certs change This is done through hashing them all and using that hash as a label in the podTemplate.
* Minor cosmetic housekeeping * goimports files * Add missing license headers * Fix linter warnings * Add api docs
@@ -42,3 +42,10 @@ patchesJson6902: | |||
kind: CustomResourceDefinition | |||
name: kibanas.kibana.k8s.elastic.co | |||
path: apm-kibana-patches.yaml | |||
# custom patches for EnterpriseSearch |
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.
Proper name here is Enterprise Search, two words.
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 really refers to the EnterpriseSearch CRD
(the technical implementation), I'd be in favor of keeping it as is. Also this is def not a user-facing file.
docs/reference/api-docs.asciidoc
Outdated
[id="{anchor_prefix}-github-com-elastic-cloud-on-k8s-pkg-apis-enterprisesearch-v1beta1-enterprisesearch"] | ||
=== EnterpriseSearch | ||
|
||
EnterpriseSearch represents an Enterprise Search resource in a Kubernetes cluster. |
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.
Can we make sure to outline that EnterpriseSearch here is a technical way of referring to Enterprise Search, the product? Naming is extra sensitive and we need make sure that we perhaps wrap it in EnterpriseSearch
a pre
wrap or something.
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 doc is auto-generated from the following go code comment:
// EnterpriseSearch represents an Enterprise Search resource in a Kubernetes cluster.
type EnterpriseSearch struct {
There's a go linter rule that specifies each commented struct must start with // <structName> ...
, so it's not easy to change it to eg. <pre>EnterpriseSearch</pre>
.
I can change it to something like EnterpriseSearch is a Kubernetes CRD to represent Enterprise Search.
if that makes more sense?
This is the CRD API doc, we'll definitely have more user-facing docs in the future in which it's important we make that distinction clear 👍
[id="{anchor_prefix}-github-com-elastic-cloud-on-k8s-pkg-apis-enterprisesearch-v1beta1-enterprisesearchlist"] | ||
=== EnterpriseSearchList | ||
|
||
EnterpriseSearchList contains a list of EnterpriseSearch |
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.
Same comment on communicating tech-level items vs. product name.
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 I like the flatter package structure compared to our other controller implementations. I have more or less ignored all TODOs and the whole association controller and the fact that there are no tests in the assumption we will address this in follow up PRs. So I 👍 for merging as is and addressing any issues after that.
Introduce the EnterpriseSearch CRD and controller in ECK.
It seems to be working fine so far but has not been extensively tested.
To be used, it requires the
image
field to be set with a valid Docker image.This is obviously work in progress, since: