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

chore(conf): add description about cluster_rpc and cluster_rpc_sync #14278

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

chronolaw
Copy link
Contributor

@chronolaw chronolaw commented Feb 14, 2025

Summary

KAG-6373
We need someone else to make it more readable.

Checklist

  • The Pull Request has tests
  • A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
  • There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Issue reference

Fix #[issue number]

@bungle bungle changed the title chore(conf): add discription about cluster_rpc and cluster_rpc_sync chore(conf): add description about cluster_rpc and cluster_rpc_sync Feb 14, 2025
@chronolaw chronolaw marked this pull request as ready for review February 19, 2025 03:33
@EuclidYongSun
Copy link

per the blog: https://konghq.com/blog/product-releases/incremental-config-sync-tech-preview
the two flags are blow, are they matching with the cluster_rpc and cluster_rpc_sync?

-e "KONG_CLUSTER_RPC_SYNC=on"
-e "KONG_CLUSTER_RPC=on" \

@chronolaw
Copy link
Contributor Author

per the blog: https://konghq.com/blog/product-releases/incremental-config-sync-tech-preview the two flags are blow, are they matching with the cluster_rpc and cluster_rpc_sync?

-e "KONG_CLUSTER_RPC_SYNC=on"
-e "KONG_CLUSTER_RPC=on" \

Yes,KONG_XXX is the env form.

@@ -345,6 +345,13 @@
# - certificate content
# - base64 encoded certificate content

#cluster_rpc = # Enable/disable cluster rpc framework.
# The default value is `off`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestions for the description:
The settings to enable or disable the RPC based communication between control plane and data plane nodes.
The valid values are on and off, default is off.
This should be set to on when cluster_rpc_sync is set to on.

#cluster_rpc = # Enable/disable cluster rpc framework.
# The default value is `off`.

#cluster_rpc_sync = # Enable/disable cluster rpc sync protocol
Copy link

@EuclidYongSun EuclidYongSun Feb 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestions for the description:
The settings to enable or disable the feature of incremental synchronization of configuration changes.
The valid values are on and off, default is off.
To enable this feature, cluster_rpc set as on is required, otherwise this field is ignored.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And should we mention it needs to be set to on for both control plane and data plane nodes in hybrid mode.
It needs to be set to on for data plane nodes for Konnect.

@@ -345,6 +345,13 @@
# - certificate content
# - base64 encoded certificate content

#cluster_rpc = # Enable/disable cluster rpc framework.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And should we mention it needs to be set to on for both control plane and data plane nodes in hybrid mode.
It needs to be set to on for data plane nodes for Konnect.

Comment on lines +348 to +349
#cluster_rpc = # Enable/disable cluster rpc framework.
# The default value is `off`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#cluster_rpc = # Enable/disable cluster rpc framework.
# The default value is `off`.
#cluster_rpc = off # The setting to enable or disable RPC-based
# communication between control plane and data plane nodes.
# The valid values are `on` and `off.` Set to `on` to enable.
# If `cluster_rpc_sync` is set to `on`, this parameter
# must also be set to `on`.

I used Euclid's suggestion and rephrased it for readability.

You don't need to say what the default value is in the description, that goes next to the parameter itself.

Comment on lines +351 to +353
#cluster_rpc_sync = # Enable/disable cluster rpc sync protocol
# The default value is `off`.
# If cluster_rpc is `off`, then cluster_rpc_sync will always be `off`.
Copy link
Contributor

@lena-larionova lena-larionova Feb 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#cluster_rpc_sync = # Enable/disable cluster rpc sync protocol
# The default value is `off`.
# If cluster_rpc is `off`, then cluster_rpc_sync will always be `off`.
#cluster_rpc_sync = off # The setting to enable or disable the incremental
# synchronization of configuration changes.
# The valid values are `on` and `off.` To enable, set this
# value to `on`, and set `cluster_rpc` to `on`.
# This setting will be ignored if `cluster_rpc` is `off`.
#
# In hybrid mode, this setting must be configured
# on both control plane and data plane nodes.

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

Successfully merging this pull request may close these issues.

4 participants