operator/config: additionalConfiguration fields convert large integers to scientific notation

[cscorley]

Hi team,

I have ran into an issue where providing a large integer (such as an ms value) would generate a redpanda configuration YAML that is not parsable by redpanda.

For instance, the following cluster definition:

apiVersion: redpanda.vectorized.io/v1alpha1
kind: Cluster
metadata:
  name: kafka
spec:
  image: 'vectorized/redpanda'
  version: 'v21.7.6'
  replicas: 3
  resources:
    requests:
      cpu: 1
      memory: 4Gi
    limits:
      cpu: 1
      memory: 4Gi
  configuration:
    rpcServer:
      port: 33145
    kafkaApi:
      - port: 9092
    adminApi:
      - port: 9644
    developerMode: false
  additionalConfiguration:
    redpanda.delete_retention_ms: '604800000'
  storage:
    storageClassName: redpanda-ssd-xfs

This causes the following error on boot:

+ /usr/local/bin/supervisord -d
+ exec /usr/bin/rpk redpanda start --check=false --advertise-rpc-addr=kafka-2.kafka.staging.svc.cluster.local.:33145 --default-log-level=info --reserve-memory 0M --smp=1 --memory=4294967296
We'd love to hear about your experience with redpanda:
https://vectorized.io/feedback
Starting redpanda...
Running:
<SNIPPED>
Welcome to the Redpanda community!
INFO   redpanda::main - application.cc:120 - Redpanda v21.7.6 - f93eed1c6888466993107a558b2abde318151720
Slack: https://vectorized.io/slack - is the main way the community interacts with one another in real time :)
Twitter: https://twitter.com/vectorizedio - come say hi!
INFO   redpanda::main - application.cc:128 - kernel=5.4.120+, nodename=kafka-2, machine=x86_64
Github Discussion: https://github.com/vectorizedio/redpanda/discussions - is preferred for longer, async, thoughtful discussions
GitHub Issues: https://github.com/vectorizedio/redpanda/issues - is reserved only for actual issues. Please use the GitHub for discussions.
Documentation: https://vectorized.io/docs/ - official docs site
Support: https://support.vectorized.io/ - to share private information with the production support vectorized team
Product Feedback: https://vectorized.io/feedback/ - let us know how we can improve your experience
INFO  2021-08-19 02:46:45,567 [shard 0] redpanda::main - application.cc:115 - System resources: { cpus: 1, available memory: 4.000GiB, reserved memory: 0.000bytes}
INFO  2021-08-19 02:46:45,572 [shard 0] redpanda::main - application.cc:279 - Configuration:
config_file: /etc/redpanda/redpanda.yaml
pandaproxy: {}
redpanda:
  admin:
    - address: 0.0.0.0
      name: admin
      port: 9644
  advertised_kafka_api:
    - address: kafka-2.kafka.staging.svc.cluster.local.
      name: kafka
      port: 9092
  advertised_rpc_api:
    address: kafka-2.kafka.staging.svc.cluster.local.
    port: 33145
  auto_create_topics_enabled: false
  data_directory: /var/lib/redpanda/data
  delete_retention_ms: 6.048e+08
  developer_mode: false
  enable_idempotence: true
  enable_transactions: true
  kafka_api:
    - address: 0.0.0.0
      name: kafka
      port: 9092
  log_segment_size: 536870912
  node_id: 2
  rpc_server:
    address: 0.0.0.0
    port: 33145
  seed_servers:
    - host:
        address: kafka-0.kafka.staging.svc.cluster.local.
        port: 33145
    - host:
        address: kafka-1.kafka.staging.svc.cluster.local.
        port: 33145
    - host:
        address: kafka-2.kafka.staging.svc.cluster.local.
        port: 33145
rpk:
  coredump_dir: /var/lib/redpanda/coredump
  enable_memory_locking: false
  enable_usage_stats: false
  overprovisioned: false
  tune_aio_events: false
  tune_clocksource: false
  tune_coredump: false
  tune_cpu: false
  tune_disk_irq: false
  tune_disk_nomerges: false
  tune_disk_scheduler: false
  tune_disk_write_cache: false
  tune_fstrim: false
  tune_network: false
  tune_swappiness: false
  tune_transparent_hugepages: false
schema_registry: {}
INFO  2021-08-19 02:46:45,572 [shard 0] redpanda::main - application.cc:283 - Use `rpk config set <cfg> <value>` to change values below:
INFO  2021-08-19 02:46:45,572 [shard 0] redpanda::main - application.cc:158 - Failure during startup: YAML::TypedBadConversion<std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000l> > > (yaml-cpp: error at line 17, column 24: bad conversion)

The problematic configuration line being: delete_retention_ms: 6.048e+08

[emaxerrno]

Thanks @cscorley ! Good report. We’ll fix. Let us know if you want to take a pass at the code. Should be chasing the types as strings on the config parser. Cc @dimitriscruz

[cscorley]

I don't think I'm apt to change it (I'm very unfamiliar with Go). I took a swing at locating the bug while trying to debug my issue, but I am not sure about the solution. I think the configuration is being unmarshaled as JSON by default here: https://github.com/vectorizedio/redpanda/blob/d4165f1af5233448733ab5c1b481457a5d5ee4fe/src/go/rpk/pkg/config/manager.go#L344

[dimitriscruz]

The operator calls the rpk package, which parses the configuration. Afaik the parsing is eventually handled by the viper library (called by rpk).

I suspect that because "delete_retention_ms" isn't part of the rpk schema, but is part of "Other" (interface{}), the type isn't handled as we'd expect (it becomes a float64). In contrast, int fields work as expected.

I tried adding the following test case under rpk's config_test.go, which passes and shows that 6.048e+08 is expected:

		{
			key:   "redpanda.delete_retention_ms",
			value: "604800000",
			check: func(st *testing.T, c *Config, _ *manager) {
				require.Exactly(st, 6.048e+08, c.Redpanda.Other["delete_retention_ms"])
			},
		},

(requiring 604800000 fails the test)

Perhaps @0x5d or @twmb have some idea on how to handle this on the rpk side, or we could have redpanda handle this notation as well.

[emaxerrno]

Oh good catch with viper/rpk

[worldofgeese]

@senior7515 this is still an issue in the latest release. Just thought I'd reach out in case it was forgotten!

[r-vasquez]

#5061 removed viper library and fixed this issue, now rpk won't convert large integers to scientific notation.

This will be in the upcoming v22.2 release