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

eck.yaml outputs as single-line string since ECK v2.6 #6695

Closed
PixelOrange opened this issue Apr 17, 2023 · 3 comments · Fixed by #6711
Closed

eck.yaml outputs as single-line string since ECK v2.6 #6695

PixelOrange opened this issue Apr 17, 2023 · 3 comments · Fixed by #6711
Assignees
Labels
>bug Something isn't working

Comments

@PixelOrange
Copy link

PixelOrange commented Apr 17, 2023

Bug Report

What did you do?
Download https://download.elastic.co/downloads/eck/2.7.0/operator.yaml

What did you expect to see?

Starting at line 38:

data:
  eck.yaml: |-
    log-verbosity: 0
    metrics-port: 0
(etc)

What did you see instead? Under which circumstances?
Starting at line 39:
eck.yaml: "log-verbosity: 0\nmetrics-port: 0\ncontainer-registry: docker.elastic.co\ncontainer-suffix: \nmax-concurrent-reconciles: 3\nca-cert-validity: 8760h\nca-cert-rotate-before: 24h\ncert-validity: 8760h\ncert-rotate-before: 24h\nexposed-node-labels: [topology.kubernetes.io/.*,failure-domain.beta.kubernetes.io/.*]\nset-default-security-context: auto-detect\nkube-client-timeout: 60s\nelasticsearch-client-timeout: 180s\ndisable-telemetry: false\ndistribution-channel: all-in-one\nvalidate-storage-class: true\nenable-webhook: true\nwebhook-name: elastic-webhook.k8s.elastic.co\nenable-leader-election: true\nelasticsearch-observation-interval: 10s"

Environment

  • ECK version: v2.6.0, v2.6.1, v2.6.2, v2.7.0

  • Kubernetes information: N/A - issue exists in the text file itself

This makes it much more difficult to edit and adjust the values and flags set in the ConfigMap.

@botelastic botelastic bot added the triage label Apr 17, 2023
@thbkrkr
Copy link
Contributor

thbkrkr commented Apr 18, 2023

It's always been like that, isn't it? I'm not sure to understand the problem? Do you want to manually edit the eck configuration? Don't you have a tool / script / ... to install it?

It seems to me that it's quite simple to use a little bit of bash to update the config on the fly, like this for example:

> eckconf() { sed "s|$1: [^\]*|$1: $2|"; }

>  curl -s https://download.elastic.co/downloads/eck/2.7.0/operator.yaml \
	| eckconf max-concurrent-reconciles 10 \
	| eckconf kube-client-timeout 120s \
	| k apply -f-

> getcm() { kubectl -n elastic-system get cm elastic-operator -o json | jq '.data["eck.yaml"]' -r; }

> getcm | grep -E "max|kube-client"
max-concurrent-reconciles: 10
kube-client-timeout: 120s

or post-install:

> upcm() {
	local key=$1 val=$2
	getcm > eck.yaml;
	sed -i.bak -E "s|^$1:.*|$1: $2|" eck.yaml
	kubectl -n elastic-system delete cm elastic-operator
	kubectl -n elastic-system create cm elastic-operator --from-file=eck.yaml
	rm eck.yaml eck.yaml.bak
}

> upcm max-concurrent-reconciles 4
configmap "elastic-operator" deleted
configmap/elastic-operator created

> getcm | grep max
max-concurrent-reconciles: 4

@PixelOrange
Copy link
Author

It's always been like that, isn't it?

No, in 2.5 and before, it looked like this:

data:
  eck.yaml: |-
    log-verbosity: 0
    metrics-port: 0
    container-registry: docker.elastic.co
    max-concurrent-reconciles: 3
    ca-cert-validity: 8760h
    ca-cert-rotate-before: 24h
    cert-validity: 8760h
    cert-rotate-before: 24h
    exposed-node-labels: [topology.kubernetes.io/.*,failure-domain.beta.kubernetes.io/.*]
    set-default-security-context: auto-detect
    kube-client-timeout: 60s
    elasticsearch-client-timeout: 180s
    disable-telemetry: false
    distribution-channel: all-in-one
    validate-storage-class: true
    enable-webhook: true
    webhook-name: elastic-webhook.k8s.elastic.co
    enable-leader-election: true
    elasticsearch-observation-interval: 10s

Post 2.5 it looks like this:

data:
  eck.yaml: "log-verbosity: 0\nmetrics-port: 0\ncontainer-registry: docker.elastic.co\ncontainer-suffix: \nmax-concurrent-reconciles: 3\nca-cert-validity: 8760h\nca-cert-rotate-before: 24h\ncert-validity: 8760h\ncert-rotate-before: 24h\nexposed-node-labels: [topology.kubernetes.io/.*,failure-domain.beta.kubernetes.io/.*]\nset-default-security-context: auto-detect\nkube-client-timeout: 60s\nelasticsearch-client-timeout: 180s\ndisable-telemetry: false\ndistribution-channel: all-in-one\nvalidate-storage-class: true\nenable-webhook: true\nwebhook-name: elastic-webhook.k8s.elastic.co\nenable-leader-election: true\nelasticsearch-observation-interval: 10s"

The new format reduces readability and introduces a higher chance for syntax errors.

Do you want to manually edit the eck configuration?

Yes, but also we want to be able to easily review the changes. The past couple of versions included container-suffix: for example.

Don't you have a tool / script / ... to install it?

We download the files and add them to git and then have our CI/CD install them. We're not editing these values programmatically. There are a few things we change (disabling webhook, adding namespaces:) but otherwise we leave these values alone as much as possible.

@thbkrkr thbkrkr self-assigned this Apr 20, 2023
@thbkrkr
Copy link
Contributor

thbkrkr commented Apr 20, 2023

Makes sense. Apologies for the regression.

It was introduced by #6086. As soon as you there is an entry in the eck config with an empty value, the file is rendered with \n. I don't know exactly why.

In #6086, we unconditionally sets container-suffix to an empty value.

# containerSuffix: ""

container-registry: {{ .Values.config.containerRegistry }}
container-suffix: {{ .Values.config.containerSuffix }}
max-concurrent-reconciles: {{ int .Values.config.maxConcurrentReconciles }}

@thbkrkr thbkrkr added the >bug Something isn't working label Apr 20, 2023
@botelastic botelastic bot removed the triage label Apr 20, 2023
@thbkrkr thbkrkr closed this as completed Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants