New CRD StackConfigPolicy to declaratively configure multiple Elasticsearch clusters #6148

thbkrkr · 2022-11-07T21:53:50Z

New custom resource StackConfigPolicy to configure cluster settings, snapshot repositorires and lifecycle policies for a list of Elasticsearch clusters matching a labels selector.

apiVersion: stackconfigpolicy.k8s.elastic.co/v1alpha1
kind: StackConfigPolicy
metadata:
  name: config-staging
  namespace: elastic-system
spec:
  resourceSelector:
    matchLabels:
      env: staging
  elasticsearch:
    clusterSettings:
      indices.recovery.max_bytes_per_sec: "100mb"
    snapshotRepositories:
      backup:
        type: gcs
        settings:
          bucket: "gcs-bucket"
    snapshotLifecyclePolicies:
      test-snapshots:
        schedule: "0 1 2 3 4 ?"
        name: "<staging-snap-{now/d}>"
        repository: "backup"
        config:
          indices: ["*"]  
          ignore_unavailable: true
          include_global_state: false
        retention:
          expire_after: "30d"
          min_count: 1
          max_count: 50

Notables details:

The namespaces used to find Elasticsearch clusters to configure depend on the StackConfigPolicy namespace. If it is the operator namespace, all namespaces managed by the operator are used, otherwise only the StackConfigPolicy namespace.

A new FileSettings Secret <esName>-es-file-settings owns by Elasticsearch is created empty and mounted as data volume by the Elasticsearch controller. The StackConfigPolicy controller soft owns the Secret and only updates it. On deletion, soft owned labels are used to find Secrets to reset.

For the SecureSettings Secrets defined in the StackConfigPolicy, the StackConfigPolicy controller writes the Secret namespaces and names in an annotation of the FileSettings Secret. The ES controller watches and reads the Secrets from that and merges their content with the user-provided Secrets of the Elasticsearch resource in the existing Secret <esName>-es-secure-settings.

Discuss:

cleanStacktrace

TODO:

pebrc

Nice work! I did a first pass just by looking at the code. Will do some testing next.

pkg/controller/elasticsearch/client/v6.go

pkg/controller/elasticsearch/client/model.go

pkg/telemetry/telemetry.go

pkg/apis/stackconfigpolicy/v1alpha1/stackconfigpolicy_types.go

pkg/controller/stackconfigpolicy/controller.go

pkg/controller/elasticsearch/filesettings/secret_test.go

pkg/controller/common/keystore/user_secret.go

pkg/controller/common/reconciler/secret.go

pkg/controller/common/watches/secrets.go

pebrc

I did some tests. Looks really good. One oddity I noticed during testing is that errors during snapshot repository creation are actually not surfaced through the reserved cluster state but only through Elasticsearch logs. However that's not something we can fix here but maybe interesting to discuss with the Elasticsearch team. I tried both using GKE Workload Identity and explicit service account credentials, which both worked fine. The former is quite compelling as you don't have to configure any credentials inside the cluster and the policy just works with minimal delay (no rolling restarts!)

pkg/apis/stackconfigpolicy/v1alpha1/stackconfigpolicy_types.go

pkg/controller/elasticsearch/filesettings/secret.go

- getclusterstate only supported in v8 es client - adjust godoc - helper struct to simplify collecting telemetry - rename UpdateResourceStatusInPhase to AddPolicyErrorFor - refactor status.Update() - take into account es version errors in policy status - no finalizers - group var declaration - lookup in a nil map is safe - remove useless getters - simplification - remove dead code - better assertions in Test_newSettingsSecret - fix comment typos

naemono

Haven't finished the code review, and haven't tested yet, but some initial comments.

pkg/apis/stackconfigpolicy/v1alpha1/stackconfigpolicy_types.go

pkg/controller/common/keystore/user_secret.go

pkg/controller/common/reconciler/secret.go

pkg/controller/elasticsearch/filesettings/file_settings.go

pebrc

Almost LGTM!

pkg/controller/elasticsearch/filesettings/file_settings.go

test/e2e/es/stackconfigpolicy_test.go

pebrc · 2022-11-16T09:10:39Z

pkg/controller/stackconfigpolicy/controller.go

+		}
+		requests := make([]reconcile.Request, 0)
+		for _, stackConfig := range stackConfigList.Items {
+			stackConfig := stackConfig


Just to clarify I consider this a nice to have optimisation and not critical for this to be merged

pkg/controller/elasticsearch/filesettings/reconciler.go

pkg/controller/elasticsearch/filesettings/secret_test.go

barkbay

I did a quick review as I wanted to give it a try and understand how the feature is implemented.
Overall LGTM 👍 , I only left a few cosmetic comments.

pkg/controller/stackconfigpolicy/controller.go

pkg/controller/elasticsearch/filesettings/file_settings.go

pkg/controller/elasticsearch/filesettings/secret.go

pkg/controller/elasticsearch/filesettings/file_settings.go

pkg/controller/stackconfigpolicy/controller.go

pkg/apis/stackconfigpolicy/v1alpha1/stackconfigpolicy_types.go

- typo: s/Error/Errors/ - typo: s/Human/human - comment phaseOrder map - ensure AddPolicyErrorFor is called once per named resource - wrap error from GetSecureSettingsSecretSources - add unit tests in TestGarbageCollectSoftOwnedSecrets and TestGarbageCollectAllSoftOwnedOrphanSecrets - improve error messages with types

…notations

thbkrkr · 2022-11-21T14:00:16Z

If you would like to take another look, I've taken into account all your feedback.

pebrc

LGTM!

pkg/controller/elasticsearch/filesettings/secret.go

pkg/controller/stackconfigpolicy/controller.go

pkg/apis/stackconfigpolicy/v1alpha1/stackconfigpolicy_types.go

pebrc

🚀

thbkrkr · 2022-11-22T15:16:51Z

Thanks pebrc, naemono, barkbay!

thbkrkr added the >feature Adds or discusses adding a feature to the product label Nov 7, 2022

thbkrkr force-pushed the stackconfigpolicy branch 2 times, most recently from 23d1510 to 0af90a6 Compare November 7, 2022 22:47

thbkrkr added 8 commits November 8, 2022 11:13

StackConfigPolicy API

3eadab4

gen

d76de7e

es filesettings

4d28c23

es keystore

589f710

es controller

d0a7745

es client

ebe1bfc

StackConfigPolicy controller

2797f79

telemetry

a99c186

thbkrkr force-pushed the stackconfigpolicy branch 2 times, most recently from e554722 to d052970 Compare November 8, 2022 13:39

yaml for rbac, webhook, api-docs/operatorhub config

c736740

thbkrkr force-pushed the stackconfigpolicy branch from d052970 to c736740 Compare November 8, 2022 14:00

validation

77efda0

thbkrkr marked this pull request as ready for review November 8, 2022 17:04

pebrc self-assigned this Nov 9, 2022

pebrc reviewed Nov 9, 2022

View reviewed changes

pebrc reviewed Nov 10, 2022

View reviewed changes

pkg/apis/stackconfigpolicy/v1alpha1/stackconfigpolicy_types.go Outdated Show resolved Hide resolved

pkg/controller/elasticsearch/filesettings/secret.go Outdated Show resolved Hide resolved

thbkrkr added 10 commits November 10, 2022 14:03

e2e test

d8a032b

restrict secure settings secrets to the StackConfigPolicy namespace

53b7a78

refactoring with filesettings.ReconcileSecret

40e8e8c

version arg in settings secret constructor

7f9af81

defer update phase

65364e2

adjust initial scp status phase

0d20148

case where es is deleted along a scp

cac466c

ignore NamespacedSecretSource in api docs

6399f02

e2e test

828a939

thbkrkr force-pushed the stackconfigpolicy branch from 1684b3b to 828a939 Compare November 15, 2022 14:40

naemono added the v2.6.0 label Nov 15, 2022

naemono reviewed Nov 15, 2022

View reviewed changes

pebrc reviewed Nov 16, 2022

View reviewed changes

barkbay reviewed Nov 18, 2022

View reviewed changes

thbkrkr added 10 commits November 18, 2022 17:30

make generate

f21393e

barkbay's feedbacks

478edb9

pebrc's feedbacks

df29f63

rewrite reconcile file settings secret to preserve existing labels/an…

49cf095

…notations

refactor status.Error.Errors to status.Error.Message

36e3c3b

update min version

dbe7ff4

lint 🐬

340ff8c

pebrc's feedbacks

55ec78e

fileBasedSettingsMinVersion

ebf79bf

pebrc reviewed Nov 21, 2022

View reviewed changes

thbkrkr added 2 commits November 22, 2022 14:18

refactoring SettingsSecret parsing

5de88e0

reuse min version for compat version

184251d

pebrc approved these changes Nov 22, 2022

View reviewed changes

thbkrkr merged commit 47ffaf7 into elastic:main Nov 22, 2022

thbkrkr deleted the stackconfigpolicy branch December 20, 2022 16:53

barkbay mentioned this pull request Jan 11, 2023

Upgrading ECK to 2.6.0 and ES to 8.6.0 causes ES to fail to bootstrap/form a cluster #6303

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New CRD StackConfigPolicy to declaratively configure multiple Elasticsearch clusters #6148

New CRD StackConfigPolicy to declaratively configure multiple Elasticsearch clusters #6148

thbkrkr commented Nov 7, 2022 •

edited

Loading

pebrc left a comment

pebrc left a comment

naemono left a comment

pebrc left a comment

pebrc Nov 16, 2022

barkbay left a comment

thbkrkr commented Nov 21, 2022

pebrc left a comment

pebrc left a comment

thbkrkr commented Nov 22, 2022

New CRD StackConfigPolicy to declaratively configure multiple Elasticsearch clusters #6148

New CRD StackConfigPolicy to declaratively configure multiple Elasticsearch clusters #6148

Conversation

thbkrkr commented Nov 7, 2022 • edited Loading

pebrc left a comment

Choose a reason for hiding this comment

pebrc left a comment

Choose a reason for hiding this comment

naemono left a comment

Choose a reason for hiding this comment

pebrc left a comment

Choose a reason for hiding this comment

pebrc Nov 16, 2022

Choose a reason for hiding this comment

barkbay left a comment

Choose a reason for hiding this comment

thbkrkr commented Nov 21, 2022

pebrc left a comment

Choose a reason for hiding this comment

pebrc left a comment

Choose a reason for hiding this comment

thbkrkr commented Nov 22, 2022

thbkrkr commented Nov 7, 2022 •

edited

Loading