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

Draft of ES declarative config #3775

Closed
wants to merge 19 commits into from
Closed

Conversation

anyasabo
Copy link
Contributor

Fix #3598

Opening up a draft to get feedback on the approach, still needs a lot of shoring up to do. But it "works".

An overview

  • users provide us a list of URLs and an optional JSON body that URL should contain, and the elasticsearch resource it should apply to
  • we do a get against that URL
    • if it's a 200 and the response body is a superset of the user-specified JSON body, we consider this operation successful and move on to the next operation in the list
    • if it is a 404 or a 200 but the response is not a superset of the user-specified body, we send a put to that URL with the user-specified body

Example manual test

---
apiVersion: elasticsearch.k8s.elastic.co/v1
kind: Elasticsearch
metadata:
  name: quickstart
spec:
  version: 7.9.1
  nodeSets:
  - name: default
    count: 1
    config:
      node.master: true
      node.data: true
      node.ingest: true
      node.store.allow_mmap: false
---
apiVersion: elasticsearchconfig.k8s.elastic.co/v1alpha1
kind: ElasticsearchConfig
metadata:
  name: quickstart-config
spec:
  operations:
  - url: "/_component_template/component_template1"
    body: |-
      {
        "template": {
          "mappings": {
            "properties": {
              "@timestamp": {
                "type": "date"
              }
            }
          }
        }
      }
  - url: "/_component_template/other_component_template"
    body: |-
      {
        "template": {
          "mappings": {
            "properties": {
              "ip_address": {
                "type": "ip"
              }
            }
          }
        }
      }
  elasticsearchRef:
    name: quickstart
PASSWORD=$(kubectl get secret quickstart-es-elastic-user -o go-template='{{.data.elastic | base64decode}}')

kubectl port-forward service/quickstart-es-http 9200

 curl -s -u "elastic:$PASSWORD" -k "https://localhost:9200/_component_template/component_template1" | jq

 curl -s -u "elastic:$PASSWORD" -k "https://localhost:9200/_component_template/other_component_template" | jq

Open questions

  • does the association interface make sense to use here? it has a meaning in other resources because the operator is configuring a resource (e.g. kibana) to create an association. here, the operator is making the calls itself, so it has a slightly different meaning. I cargo culted it in because I was used to it, but thinking about it more I'm not sure it's necessary

  • ES sometimes supports additional arguments in PUTs that it does not support for GETs (e.g. verify=false for snapshot repo creation), but we provide no mechanism for that here. It's not clear to me how important that might be though.

  • does checking specifically for 404s/200s make sense? I'm not sure if there's other status codes ES use that make sense for us to proceed on

  • now is a good time for naming/structure feedback too

@anyasabo anyasabo added the >feature Adds or discusses adding a feature to the product label Sep 22, 2020
@@ -171,6 +171,7 @@ go-run:
-tags "$(GO_TAGS)" \
./cmd/main.go manager \
--development \
--enable-leader-election=false \
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 is just to enable quicker iteration when running locally and could be moved to a separate PR

@shubhaat shubhaat mentioned this pull request Sep 22, 2020
@sebgl
Copy link
Contributor

sebgl commented Oct 2, 2020

I added a few comments about the general approach in the issue: #3598 (comment)

}

// TODO checking for a 200 might be wrong, really any status code not 200 might mean we need to update. but a 200 indicates we can and should compare the bodies
// TODO should bodies always be required to be specified? I think probably? I don't know of any ES API that you can send an empty PUT. maybe we require at a minimum "{}"?
Copy link
Collaborator

Choose a reason for hiding this comment

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

@anyasabo
Copy link
Contributor Author

anyasabo commented Nov 3, 2020

I'll go ahead and close this out while we investigate an alternative approach (a single fire and forget job) that I'll detail in the issue

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API call controller
3 participants