Restore ligthspeed-to-dataverse-exporter#143
Restore ligthspeed-to-dataverse-exporter#143openshift-merge-bot[bot] merged 1 commit intorh-ecosystem-edge:mainfrom
ligthspeed-to-dataverse-exporter#143Conversation
It was accidentally removed in a14100a
WalkthroughAdds a new lightspeed-to-dataverse-exporter sidecar container, a ConfigMap carrying its config, associated env/args/resources, and mounts/volumes to the assisted-chat Deployment in template.yaml. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant K as Kubernetes
participant AC as assisted-chat Pod
participant EX as exporter sidecar
participant CM as ConfigMap (exporter config)
participant DS as data-storage (PVC/volume)
participant IS as Insights Ingress Server
K->>AC: Deploy Pod with assisted-chat + exporter containers
K-->>EX: Mount CM at /etc/config/config.yaml
K-->>EX: Mount DS at ${STORAGE_MOUNT_PATH}
EX->>EX: Start with --mode manual, load config.yaml
loop Every collection_interval
EX->>DS: Scan ${STORAGE_MOUNT_PATH}/{feedback, transcripts}
EX->>IS: POST data (auth via secret token)
alt Success
EX->>DS: Cleanup sent items (if cleanup_after_send)
else Error/Timeout
EX->>EX: Log error and retry on next interval
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: keitwb, omertuc The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
![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 (5)
template.yaml (5)
509-512: ConfigMap changes won’t live-reload; add recycle and make config mount read-only
- Using
subPathdisables live updates for ConfigMap-mounted files; pods must be restarted for config changes anyway. To make rollouts automatic when the ConfigMap changes in AppSRE flows, mirror the pattern used elsewhere and addqontract.recycle: "true".- The config file should be mounted read-only.
Diffs:
- Make the config file mount read-only
volumeMounts: - name: lightspeed-exporter-config mountPath: /etc/config/config.yaml subPath: config.yaml + readOnly: true
- Add recycle annotation to the exporter ConfigMap
- apiVersion: v1 kind: ConfigMap metadata: + annotations: + qontract.recycle: "true" name: lightspeed-exporter-configAlso applies to: 573-577
528-530: EphemeralemptyDirmay lose unsent data on restart
data-storageisemptyDir. If the Pod restarts between collections, unsent feedback/transcripts are lost. If durability matters, use a PVC with aReadWriteOnce(or appropriate) access mode.Happy to propose a PVC +
volumeClaimTemplates(for StatefulSet) or a shared PVC if this Deployment is singleton.
485-487: Harden the exporter container with a restrictive securityContextSidecars that handle auth tokens should be non-root, drop caps, etc.
- name: lightspeed-to-dataverse-exporter image: quay.io/lightspeed-core/lightspeed-to-dataverse-exporter:${LIGHTSPEED_EXPORTER_IMAGE_TAG} imagePullPolicy: Always + securityContext: + runAsNonRoot: true + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + capabilities: + drop: ["ALL"] + seccompProfile: + type: RuntimeDefault
118-120: Nit: Typo in parameter name “LIGHTSSPEED_…”Minor consistency issue: “LIGHTSSPEED” has an extra “S”. It’s referenced consistently, so it works, but it’s easy to trip future readers and scripts.
If you want to correct it now, change both the parameter and its usage:
- name: LIGHTSSPEED_STACK_POSTGRES_SSL_MODE + name: LIGHTSPEED_STACK_POSTGRES_SSL_MODE ... - ssl_mode: ${LIGHTSSPEED_STACK_POSTGRES_SSL_MODE} + ssl_mode: ${LIGHTSPEED_STACK_POSTGRES_SSL_MODE}Double-check for any other references before applying.
Also applies to: 173-181
488-495: Optional: add a simple liveness probe if running in daemon modeIf you switch to a long-running mode, a basic HTTP or exec probe helps catch silent failures.
Example (if the exporter exposes a health endpoint) — adjust as supported:
args: - "--mode" - "daemon" - "--config" - "/etc/config/config.yaml" - "--log-level" - "INFO" + livenessProbe: + exec: + command: ["sh", "-c", "test -f /proc/1/cmdline"] # placeholder; replace with real check + initialDelaySeconds: 30 + periodSeconds: 10
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
template.yaml(1 hunks)
🔇 Additional comments (4)
template.yaml (4)
502-508: Resource sizing looks reasonable for a lightweight sidecarInitial requests/limits are conservative and should be fine to start. Plan to tune after observing usage.
579-591: Confirm config schema matches the exporter version (keys and units)Please verify that these keys are valid for the
${LIGHTSPEED_EXPORTER_IMAGE_TAG}you’ll deploy:
data_dir,allowed_subdirs,service_id,identity_id,ingress_server_url- Collection block:
collection_interval(seconds?),cleanup_after_send,ingress_connection_timeout(seconds?)If keys differ between versions, the exporter may fail at startup.
496-501: Verified Secret ConfigurationI’ve confirmed that the default
INSIGHTS_INGRESS_SECRET_NAMEis set to"insights-ingress"intemplate.yaml(lines 74–76) and that thedeploy_template.shscript automatically creates a generic secret namedinsights-ingresswith theauth_tokenkey if it’s missing (scripts/deploy_template.sh:26–29). No further action is required here.
488-495: Verify Exporter Mode and Sidecar BehaviorThe container spec uses
--mode manualalongside acollection_intervalsetting. If “manual” mode performs a single run and exits, Kubernetes (withrestartPolicy: Always) will immediately restart the container—potentially re-sending the same data repeatedly.• For a continuously running exporter sidecar, switch to the exporter’s daemon/periodic mode (often named
daemonor similar).
• If you intend to run the exporter on a schedule, consider using a Kubernetes CronJob instead of a sidecar.Suggested minimal change for a long-lived sidecar:
args: - "--mode" - - "manual" + - "daemon" - "--config" - "/etc/config/config.yaml" - "--log-level" - "INFO"Please confirm in the exporter’s documentation that:
--mode manualonly runs once and then exits.- There is a
daemon(or equivalent) mode for continuous operation.
openshift-merge-bot
bot
merged commit d08a183
into
rh-ecosystem-edge:main
2 participants
It was accidentally removed in a14100a
Summary by CodeRabbit
New Features
Chores