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

Introduce EnterpriseSearch CRD #2688

Merged
merged 26 commits into from
Mar 13, 2020
Merged

Introduce EnterpriseSearch CRD #2688

merged 26 commits into from
Mar 13, 2020

Conversation

sebgl
Copy link
Contributor

@sebgl sebgl commented Mar 10, 2020

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:

  • parts of the controller should be refactored to share common code with APMServer and Kibana
  • most of the association controller should be refactored to share common code with APMServer and Kibana
  • there are 0 unit tests (mostly because I wanted to go fast in the prototype phase, and because many unit tests should be written along with the refactoring above)
  • there are 0 E2E tests
  • it is missing a few features (podDisruptionBudget, secret config refs, version upgrade handling, etc.)

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
@sebgl sebgl added >feature Adds or discusses adding a feature to the product v1.1.0 labels Mar 10, 2020
@@ -42,3 +42,10 @@ patchesJson6902:
kind: CustomResourceDefinition
name: kibanas.kibana.k8s.elastic.co
path: apm-kibana-patches.yaml
# custom patches for EnterpriseSearch
Copy link

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.

Copy link
Contributor Author

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.

[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.
Copy link

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.

Copy link
Contributor Author

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
Copy link

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.

Copy link
Collaborator

@pebrc pebrc left a 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.

pkg/controller/enterprisesearch/state.go Outdated Show resolved Hide resolved
@sebgl sebgl merged commit a1bd830 into master Mar 13, 2020
@anyasabo anyasabo added v1.2.0 and removed v1.1.0 labels Mar 31, 2020
@sebgl sebgl deleted the enterprise-search branch May 25, 2020 07:08
@pebrc pebrc added the release-highlight Candidate for the ECK release highlight summary label Jun 4, 2020
@charith-elastic charith-elastic changed the title Add the EnterpriseSearch CRD and controller Introduce EnterpriseSearch CRD Jun 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>feature Adds or discusses adding a feature to the product release-highlight Candidate for the ECK release highlight summary v1.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants