config: update config descriptions to match docs #23347

Deflaimun · 2024-09-17T16:11:30Z

Backfeed changes to configuration descriptions that were added to docs.

Backports Required

Release Notes

Improvements

Improve property configuration descriptions.

dotnwat · 2024-09-17T16:54:27Z

src/v/config/node_config.cc

+
+* Disabled - Redpanda does not start in FIPS mode.
+
+* Permissive - Redpanda performs the same check as enabled, but a warning is logged, and Redpanda continues to run. Redpanda loads the OpenSSL FIPS provider into the OpenSSL library. After this completes, Redpanda is operating in FIPS mode, which means that the TLS cipher suites available to users are limited to the TLSv1.2 and TLSv1.3 NIST-approved cryptographic methods.


we try to line wrap around 80 columns

maybe we need to maintain the nicely formatted version somewhere else?

It would be awesome if we could move those messages to a message bundle of sorts.

Feediver1 · 2024-09-30T17:27:11Z

@dotnwat @Deflaimun Can we get a status update please?

aanthony-rp

We do have some functionality problems caused by several changes within. Please see my comments for more details. briefly: any change that remains as a single line of text with no breaks will be accepted. Any multi-line doc strings cannot be accepted without a code change. let's work together on next steps.

aanthony-rp · 2024-10-01T20:49:59Z

src/v/config/node_config.cc

-      "List of the seed servers used to join current cluster. If the "
-      "seed_server list is empty the node will be a cluster root and it will "
-      "form a new cluster",
+      "List of the seed servers used to join current cluster. If the `seed_servers` list is empty the node will be a cluster root and it will form a new cluster.


Unfortunately, multi-line rich formatting like this will cause UX issues because we do output these strings in certain scenarios where we expect the string to be a one-line string.

@dotnwat and I had an idea that we could extend the C++ to have a _desc and a _longdesc member so that we could safely integrate really nice summaries like this into the product.

summary for today: any changes coming into this file need to be one-line strings. We'll need to follow up to accept larger blobs like this one. I'm happy to get on a call to help clarify what needs to change.

aanthony-rp · 2024-10-01T20:51:12Z

src/v/config/node_config.cc

-                "Duplicate items in seed_servers: {}", *s_dupe_i);
-          }
-          return std::nullopt;
+    std::sort(s.begin(), s.end());


I don't think you meant to change this, looks like a formatter did it.

Yes, that's correct.

aanthony-rp · 2024-10-01T20:51:36Z

src/v/config/node_config.cc

-      "seed_servers list must be identical among those nodes' configurations, "
-      "and those nodes will form the initial cluster.",
+      "Controls how a new cluster is formed. All brokers in a cluster must have the same value.
+


even in cases like this, we don't want to have line breaks.

aanthony-rp · 2024-10-01T20:51:57Z

src/v/config/node_config.cc

      {.visibility = visibility::user},
      {model::broker_endpoint(net::unresolved_address("127.0.0.1", 9644))})
  , admin_api_tls(
      *this,
      "admin_api_tls",
-      "TLS configuration for admin HTTP server",
+      "Specifies the TLS configuration for the HTTP Admin API.",


changes like this are 100% compatible.

Deflaimun · 2024-10-02T20:36:14Z

Force-push: Updated peer code review. Removed multi-line strings. For some specific cases, added the guide to check docs "For more information see [link]"

dotnwat · 2024-10-02T23:19:46Z

@Deflaimun i've fixed the compilation error and formatting. lemme know if it looks ok.

dotnwat · 2024-10-02T23:25:03Z

src/v/config/node_config.cc

-      "form a new cluster",
+      "`seed_servers` list is empty the node will be a cluster root and it "
+      "will form a new cluster. When `empty_seed_starts_cluster` is `true`, "
+      "Redpanda enables one broker with an empty `seed_servers` list to "


seed_servers

this isn't a big deal, but these strings really aren't intended to contain markdown, and i'm assuming that is what the backticks are for.

vbotbuildovich · 2024-10-03T02:52:17Z

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/55693#01924fe1-764a-435b-b947-38510ef6ad66

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/55693#01924fe1-7648-4d09-a62f-65df6414774f

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/55693#01924fe1-7644-44b7-a138-7d978b531e44

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/55858#01925a75-3af4-44a6-a53c-49827dae27be

BenPope

Having a poke with this over here: #23619

BenPope · 2024-10-03T10:24:48Z

src/v/config/configuration.cc

@@ -1190,7 +1299,7 @@ configuration::configuration()
      *this,
      "storage_max_concurrent_replay",
      "Maximum number of partitions' logs that will be replayed concurrently "
-      "at startup, or flushed concurrently on shutdown.",
+      "at startup, or flushed concurrently on shutdown. asdasdasdasdas",


Oops, not sure how that got in there. Will remove! Thanks for the catch

BenPope · 2024-10-03T14:26:38Z

src/v/config/configuration.cc

+      "Raft heartbeat RPC (remote procedure call) timeout. Raft uses a "
+      "heartbeat mechanism to maintain leadership authority and to trigger "
+      "leader elections. The `raft_heartbeat_interval_ms` is a periodic "
+      "heartbeat sent by the partition leader to all followers to assert its "


The easy fix for the ducktape tests is to avoid the word "assert"

should we reword to remove "assert" ? or are we writing an exception for these strings?

i think we should add exceptions for these cases in ducktape. we don't want to restrict the language we can use in descriptions.

i think we should add exceptions for these cases in ducktape. we don't want to restrict the language we can use in descriptions.

I agree, which is what I attempted here - but the NodeCrash regex still needs changing. I suggested an easy workaround.

I don't think the description of the config really needs to be rendered to the log at startup, the value should suffice.

BenPope

Please don't escape the % (\%), I count 4 of them.

Deflaimun · 2024-10-03T19:31:07Z

Force-push:

Remove wrong text identified in config: update config descriptions to match docs #23347 (comment)
Remove % escapes.

BenPope · 2024-10-03T22:31:49Z

Force-push:

Remove wrong text identified in config: update config descriptions to match docs #23347 (comment)

Remove % escapes.

It doesn't lint anymore, the lines are too long.

Deflaimun · 2024-10-04T13:59:01Z

force-push: remove the word "assert" from description string.

BenPope · 2024-10-04T14:33:11Z

The interesting diff between my review and now.

BenPope · 2024-10-04T16:58:49Z

Known CI Failures:

https://redpandadata.atlassian.net/browse/CORE-7777 (x2)

Unknown (possible new)

In test_describe_topics_with_documentation_and_types: doc_string == prop.doc_string

Signed-off-by: Noah Watkins <noahwatkins@gmail.com>

Deflaimun requested a review from a team as a code owner September 17, 2024 16:11

github-actions bot added the area/redpanda label Sep 17, 2024

dotnwat reviewed Sep 17, 2024

View reviewed changes

aanthony-rp requested changes Oct 1, 2024

View reviewed changes

Deflaimun force-pushed the update-properties-descriptions branch 2 times, most recently from a36eec9 to 3c3b521 Compare October 2, 2024 19:08

Deflaimun mentioned this pull request Oct 2, 2024

updates the description of the node properties #14579

Closed

Deflaimun force-pushed the update-properties-descriptions branch from 3c3b521 to d65ae8c Compare October 2, 2024 20:34

Deflaimun requested a review from aanthony-rp October 2, 2024 20:36

dotnwat force-pushed the update-properties-descriptions branch from d65ae8c to 84bc7c6 Compare October 2, 2024 23:19

dotnwat reviewed Oct 2, 2024

View reviewed changes

This comment was marked as outdated.

Sign in to view

BenPope reviewed Oct 3, 2024

View reviewed changes

BenPope mentioned this pull request Oct 3, 2024

Update properties descriptions: Now with extra fixes! #23619

Closed

7 tasks

BenPope reviewed Oct 3, 2024

View reviewed changes

Deflaimun force-pushed the update-properties-descriptions branch from 84bc7c6 to efe56d8 Compare October 3, 2024 19:30

Deflaimun force-pushed the update-properties-descriptions branch from efe56d8 to 6577f4e Compare October 4, 2024 13:58

config: update config descriptions to match docs

0b1ae30

BenPope force-pushed the update-properties-descriptions branch from 6577f4e to 0b1ae30 Compare October 4, 2024 14:02

BenPope previously approved these changes Oct 4, 2024

View reviewed changes

aanthony-rp previously approved these changes Oct 4, 2024

View reviewed changes

tests: sync description updates into integration test

4b74f30

Signed-off-by: Noah Watkins <noahwatkins@gmail.com>

dotnwat dismissed stale reviews from aanthony-rp and BenPope via 4b74f30 October 5, 2024 01:02

dotnwat approved these changes Oct 5, 2024

View reviewed changes

dotnwat merged commit a8e608e into dev Oct 5, 2024
18 checks passed

dotnwat deleted the update-properties-descriptions branch October 5, 2024 04:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

config: update config descriptions to match docs #23347

config: update config descriptions to match docs #23347

Deflaimun commented Sep 17, 2024 •

edited

Loading

dotnwat Sep 17, 2024 •

edited

Loading

Deflaimun Sep 17, 2024

Feediver1 commented Sep 30, 2024

aanthony-rp left a comment

aanthony-rp Oct 1, 2024

aanthony-rp Oct 1, 2024

Deflaimun Oct 1, 2024

aanthony-rp Oct 1, 2024

aanthony-rp Oct 1, 2024

Deflaimun commented Oct 2, 2024

dotnwat commented Oct 2, 2024

dotnwat Oct 2, 2024

This comment was marked as outdated.

vbotbuildovich commented Oct 3, 2024 •

edited

Loading

BenPope left a comment •

edited

Loading

BenPope Oct 3, 2024

dotnwat Oct 3, 2024

Deflaimun Oct 3, 2024

Deflaimun Oct 3, 2024

BenPope Oct 3, 2024

Deflaimun Oct 3, 2024 •

edited

Loading

dotnwat Oct 3, 2024

BenPope Oct 3, 2024

BenPope left a comment

Deflaimun commented Oct 3, 2024

BenPope commented Oct 3, 2024

Deflaimun commented Oct 4, 2024

BenPope commented Oct 4, 2024

BenPope commented Oct 4, 2024 •

edited

Loading


		* Disabled - Redpanda does not start in FIPS mode.

		* Permissive - Redpanda performs the same check as enabled, but a warning is logged, and Redpanda continues to run. Redpanda loads the OpenSSL FIPS provider into the OpenSSL library. After this completes, Redpanda is operating in FIPS mode, which means that the TLS cipher suites available to users are limited to the TLSv1.2 and TLSv1.3 NIST-approved cryptographic methods.

config: update config descriptions to match docs #23347

config: update config descriptions to match docs #23347

Conversation

Deflaimun commented Sep 17, 2024 • edited Loading

Backports Required

Release Notes

Improvements

dotnwat Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Feediver1 commented Sep 30, 2024

aanthony-rp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Deflaimun commented Oct 2, 2024

dotnwat commented Oct 2, 2024

Choose a reason for hiding this comment

This comment was marked as outdated.

vbotbuildovich commented Oct 3, 2024 • edited Loading

BenPope left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Deflaimun Oct 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenPope left a comment

Choose a reason for hiding this comment

Deflaimun commented Oct 3, 2024

BenPope commented Oct 3, 2024

Deflaimun commented Oct 4, 2024

BenPope commented Oct 4, 2024

BenPope commented Oct 4, 2024 • edited Loading

Deflaimun commented Sep 17, 2024 •

edited

Loading

dotnwat Sep 17, 2024 •

edited

Loading

vbotbuildovich commented Oct 3, 2024 •

edited

Loading

BenPope left a comment •

edited

Loading

Deflaimun Oct 3, 2024 •

edited

Loading

BenPope commented Oct 4, 2024 •

edited

Loading