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

APM configuration #1181

Merged
merged 33 commits into from
Jul 5, 2019
Merged

APM configuration #1181

merged 33 commits into from
Jul 5, 2019

Conversation

barkbay
Copy link
Contributor

@barkbay barkbay commented Jul 3, 2019

This PR:

  • Add support for a fully configurable APM server
  • Add support for the APM keystore
  • Factorize some code by adding utility functions shared by Kibana and APM controllers in order to manage a keystore and the Elastic authentication
  • Improve naming for APM resources
  • Add some E2E tests for these features

This PR do not (new issues will be open):

  • Improve APM deployment comparison by using a hash
  • Allow the APM server to run in an other namespace
  • Improve the code for user management
  • Allow the APM server to listen with https
  • Remove ElasticsearchInlineAuth

Fix #53

@barkbay barkbay changed the title Apm configuration 53 APM configuration Jul 3, 2019
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.

I like that you tackled the code reuse between Kibana and APM as well.
I would expect a follow up issue to use hash comparisons etc. to be created before merging. Otherwise LGTM

operators/pkg/apis/common/v1alpha1/association.go Outdated Show resolved Hide resolved
// ElasticsearchAuth contains auth config for Kibana to use with an Elasticsearch cluster
type ElasticsearchAuth struct {
// Inline is auth provided as plaintext inline credentials.
Inline *ElasticsearchInlineAuth `json:"inline,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need inline auth?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍
I would like to do it in an other PR, this one is already quite big and removing ElasticsearchInlineAuth has some implications in the Kibana tests.

operators/pkg/controller/apmserver/config/config.go Outdated Show resolved Hide resolved
operators/pkg/controller/kibana/pod/pod.go Outdated Show resolved Hide resolved
operators/test/e2e/apm/configuration.go Outdated Show resolved Hide resolved
operators/test/e2e/stack/keystore.go Outdated Show resolved Hide resolved
@@ -35,7 +35,7 @@ func CheckApmServerDeployment(stack Builder, k *helpers.K8sHelper) helpers.TestS
var dep appsv1.Deployment
err := k.Client.Get(types.NamespacedName{
Namespace: params.Namespace,
Name: stack.ApmServer.Name + "-apm-server",
Name: stack.ApmServer.Name + "-apm",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason for moving away from apm-server?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first it was not intentional, I did it when I was working on the naming. I thought it was not so bad since "-server" does not bring much value. But since the name of the container is apm-server I'm ok to keep it 😃

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.

👍 I vote for merging this and fixing the follow up issues identified. separately

@barkbay barkbay merged commit 8f3756d into elastic:master Jul 5, 2019
sebgl added a commit that referenced this pull request Jul 12, 2019
* Support for APM server configuration (#1181)

* Add a config section to the APM server configuration

* APM: Add support for keystore

* Factorize ElasticsearchAuthSettings

* Update dev setup doc + fix GKE bootstrap script (#1203)

* Update dev setup doc + fix GKE bootstrap script

* Update wording of container registry authentication

* Ensure disks removal after removing cluster in GKE (#1163)

* Update gke-cluster.sh

* Implement cleanup for unused disks in GCP

* Update Makefile

* Update CI jobs to do proper cleanup

* Normalize the raw config when creating canonical configs (#1208)

This aims at counteracting the difference between JSON centric serialization and the use of YAML as the serialization format in canonical config. If not normalizing numeric values
like 1 will differ when comparing configs as JSON deserializes integer numbers to float64 and YAML to uint64.

* Homogenize logs (#1168)

* Don't run tests if only docs are changed (#1216)

* Update Jenkinsfile

* Simplify notOnlyDocs()

* Update Jenkinsfile

* Push snapshot ECK release on successful PR build (#1184)

* Update makefile's to support snapshots

* Add snapshot releases to Jenkins pipelines

* Cleanup

* Rename RELEASE to USE_ELASTIC_DOCKER_REGISTRY

* Update Jenkinsfile

* Add a note on EKS inbound traffic & validating webhook (#1211)

EKS users must explicitly enable communication from the k8s control
plane and nodes port 443 in order for the control plane to reach the
validating webhook.

Should help with #896.

* Update PodSpec with Hostname from PVC when re-using (#1204)

* Bind the Debug HTTP server to localhost by default (#1220)

* Run e2e tests against custom Docker image (#1135)

* Add implementation

* Update makefile's

* Update Makefile

* Rename Jenkisnfile

* Fix review comments

* Update e2e-custom.yml

* Update e2e-custom.yml

* Return deploy-all-in-one to normal

* Delete GKE cluster only if changes not in docs (#1223)

* Add operator version to resources (#1224)

* Warn if unsupported distribution (#1228)

The operator only works with the official ES distributions to enable the security
available with the basic (free), gold and platinum licenses in order to ensure that
all clusters launched are secured by default.

A check is done in the prepare-fs script by looking at the existence of the
Elastic License. If not present, the script exit with a custom exit code.

Then the ES reconcilation loop sends an event of type warning if it detects that
a prepare-fs init container terminated with this exit code.

* Document Elasticsearch update strategy change budget & groups (#1210)

Add documentation for the `updateStrategy` section of the Elasticsearch
spec.

It documents how (and why) `changeBudget` and `groups` are used by ECK,
and how both settings can be specified by the user.
@pebrc pebrc added >feature Adds or discusses adding a feature to the product v0.9.0 labels Jul 19, 2019
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 v0.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for APM server configuration
2 participants