Add mandatory kafka env values to parseable helm template #1291
Conversation
WalkthroughThe changes introduce conditional Kafka connector configurations to two Helm template files by adding new environment variables within container specifications. Additionally, a new section is added to the Helm values file that defines Kafka connector parameters. These modifications ensure that when the Kafka connector is enabled, the corresponding environment variables are injected into the deployments. The existing container settings remain unchanged while extending functionality for Kafka integration. Changes
Sequence Diagram(s)sequenceDiagram
actor User as User
participant V as helm/values.yaml
participant T as Helm Template
participant K as Kubernetes Deployment
User->>V: Set Kafka connector configuration
V->>T: Provide Kafka connector values
alt Kafka connector enabled
T->>K: Deploy with Kafka environment variables
else Kafka connector disabled
T->>K: Deploy without Kafka environment variables
end
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
helm/values.yaml (1)
251-255: Kafka Connector Configuration Block Added
The newkafkaConnectorblock adds essential Kafka configuration parameters. Please verify that its indentation and placement are consistent with the rest of the Parseable configuration. In particular, confirm that it is nested under the appropriate parent key (if intended) and that the types—such as using a comma‐separated string forconsumerTopics—align with how these values will be consumed later in the Helm templates.helm/templates/standalone-deployment.yaml (1)
115-122: Kafka Environment Variables in Deployment Template
A new block conditionally injects environment variables for Kafka when.Values.parseable.kafkaConnector.enabledis true. The variablesP_KAFKA_BOOTSTRAP_SERVERS,P_KAFKA_CONSUMER_TOPICS, andP_KAFKA_PARTITION_LISTENER_CONCURRENCYare correctly templated using the values from the chart configuration. Please verify that the value types match your expectations (for example, if you need a list for topics, you might consider YAML’s native list syntax instead of a comma‐separated string).helm/templates/ingestor-statefulset.yaml (1)
142-149: Kafka Environment Variables in Ingestor StatefulSet
The addition of the Kafka-related environment variables under a conditional block is consistent with the approach used in the standalone deployment template. Just ensure that the values provided inhelm/values.yamlare accurate and that the conditional check on.Values.parseable.kafkaConnector.enabledbehaves as expected. This consistency across deployment types will help maintain uniformity in Kafka integration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
helm/templates/ingestor-statefulset.yaml(1 hunks)helm/templates/standalone-deployment.yaml(2 hunks)helm/values.yaml(1 hunks)
🔇 Additional comments (1)
helm/templates/standalone-deployment.yaml (1)
38-40: Deployment Command Arguments Formatting
A minor change was made to the formatting of theargsarray (i.e. the added space after the bracket). This change does not affect functionality, but ensure that formatting remains consistent with other similar templates if you are standardizing your style.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
helm/templates/ingestor-statefulset.yaml (1)
142-147: Inject Kafka Connector Environment Variables ConditionallyThe newly added block conditionally injects Kafka-related environment variables when
.Values.parseable.kafkaConnector.enabledis true. The use oftpl $value $ | quoteis consistent with similar patterns for existing environment variables, helping to resolve template expressions properly. Please double-check the YAML indentation within theenv:section to ensure that the injected items align correctly with other environment variables.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
helm/templates/ingestor-statefulset.yaml(1 hunks)helm/templates/standalone-deployment.yaml(3 hunks)helm/values.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- helm/templates/standalone-deployment.yaml
🔇 Additional comments (1)
helm/values.yaml (1)
250-267: Introduce Kafka Connector Configuration SectionA new
kafkaConnectorsection has been added with the following properties:
enabledis set tofalseby default, ensuring that the Kafka connector is opt-in.- The
envmap includes key Kafka configuration parameters such as:
P_KAFKA_BOOTSTRAP_SERVERSP_KAFKA_PARTITION_LISTENER_CONCURRENCYP_KAFKA_CONSUMER_TOPICSThe inline comments provide useful context for these parameters. Consider supplementing with additional documentation if these values might require further explanation for end users in the future.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
helm/values.yaml (1)
12-26: High Availability Ingestor Configuration UpdateThe updates to the
ingestorblock underhighAvailability(including the addition ofaffinity: { },podAnnotations: { },nodeSelector: { }, andtolerations: [ ]) help standardize pod configuration. Please review the commented-out lines (lines 19–20) to decide whether they should be retained as documentation or removed to avoid confusion in future maintenance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
helm/values.yaml(11 hunks)
🔇 Additional comments (3)
helm/values.yaml (3)
156-159: ServiceAccount Configuration UpdateThe addition of an empty
annotationsfield and the newnodeSelectorunder theserviceAccountsection are useful for ensuring consistent metadata configuration. If these fields are intended as placeholders for future customization, consider adding a clarifying comment.
244-249: Metrics Section FormattingThe introduction of empty objects for
attachMetadataandbodySizeLimitin the metrics configuration provides clear placeholders for potential future parameters. Please verify that these empty defaults do not inadvertently override any expected defaults in downstream processing.
249-281: New Kafka Connector Configuration SectionA new
kafkaConnectorsection has been added, which includes the mandatory Kafka environment configuration:
enabled: set tofalseby default.env: defines key Kafka parameters such as
P_KAFKA_BOOTSTRAP_SERVERS:"my-kafka.kafka.svc.cluster.local:9092"P_KAFKA_PARTITION_LISTENER_CONCURRENCY:"2"P_KAFKA_CONSUMER_TOPICS:"test-log-stream-0,test-log-stream-1"The inline comments are clear and offer context for both required and optional settings. Please ensure that downstream Helm templates (for example, in the ingestor-statefulset and standalone-deployment) correctly reference these values when the connector is enabled.
|
Thank you @hippalus looks great, merging now |
#1290.
Description
This PR has:
Summary by CodeRabbit