deployment-automation icon indicating copy to clipboard operation
deployment-automation copied to clipboard

Validate/test if cluster configuration property of type string can be un-set

Open RafalKorepta opened this issue 2 years ago • 3 comments

The rpk cluster config set at the moment of writing has the following logic: https://github.com/redpanda-data/redpanda/blob/c3cca097b01c250715fd4012bac175f09c26778e/src/go/rpk/pkg/cli/cmd/cluster/config/set.go#L85-L99

			if meta.Nullable && value == "null" {
				// Nullable types may be explicitly set to null
				upsert[key] = nil
			} else if meta.Type != "string" && (value == "") {
				// Non-string types that receive an empty string
				// are reset to default
				remove = append(remove, key)
			} else if meta.Type == "array" {
				var a anySlice
				err = yaml.Unmarshal([]byte(value), &a)
				out.MaybeDie(err, "invalid list syntax")
				upsert[key] = a
			} else {
				upsert[key] = value
			}

This mean that if any property of type string which has different default value then "" (empty string) can not be unset.

I didn't test it, but it might be worth checking the rpk cluster config import https://github.com/redpanda-data/redpanda/blob/c3cca097b01c250715fd4012bac175f09c26778e/src/go/rpk/pkg/cli/cmd/cluster/config/import.go#L98-L223

Related issue https://github.com/redpanda-data/helm-charts/pull/395

RafalKorepta avatar Mar 30 '23 08:03 RafalKorepta

I don't think there's a way for us to null out something. We depend upon the existence of the key/value in the dict in order to send something to config set. And I'm not sure there's a way from the yaml to say that a particular value for a given key can be null. or empty. or whatever.

  ansible.builtin.command: rpk cluster config set {{ item.key }} {{ item.value }} {{ redpanda_rpk_opts }}
  loop: "{{ configuration.cluster | dict2items }}"
  register: result
  changed_when: '"New configuration version is {{ config_version.stdout|int() }}." not in result.stdout'
  run_once: true
  no_log: true```

hcoyote avatar Mar 30 '23 14:03 hcoyote

Thinking about this a bit more: since the command is exercised as a jinja2 template, we'd have to figure out how to get the {{ item.value }} to resolve to empty/nothing. Maybe we could do something like {{ item.value if item.value is defined and item.value|length }}

hcoyote avatar Mar 30 '23 15:03 hcoyote

To achieve this we'd likely need to write an RPK module for ansible and define state. We'd then use RPK to check the state of the config value, and remediate accordingly if state is set to present (set to the value provided) or absent (set to null).

This is a longer term project, so isn't something we can solve right away. It's necessary for most of the administrative playbooks people want us to put together so it is something on my personal todo list.

As it stands today, my recommendation for people who want to shift how their cluster works while using ansible would be for them to change up the playbook and then engage in manual remediation with RPK to ensure the changes are sound. That's the method most likely to lead to good outcomes today.

gene-redpanda avatar Mar 30 '23 15:03 gene-redpanda