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

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

Open
RafalKorepta opened this issue Mar 30, 2023 · 3 comments

Comments

@RafalKorepta
Copy link

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 redpanda-data/helm-charts#395

@hcoyote
Copy link
Contributor

hcoyote commented Mar 30, 2023

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
Copy link
Contributor

hcoyote commented Mar 30, 2023

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 }}

@gene-redpanda
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants