Skip to content

Conversation

mangelajo
Copy link
Member

@mangelajo mangelajo commented May 22, 2025

Summary by CodeRabbit

  • Chores
    • Updated container images for exporters to use the latest version and ensured images are always pulled on startup.
    • Modified startup commands for exporters to use a new executable and updated argument format.
    • Updated scripts to use new command-line tools and improved YAML output handling for exporters.
    • Adjusted exporter configuration to use updated driver type strings and removed redundant labeling steps.
    • Added support for QEMU-related exporters by extending configuration and deployment resources.

Copy link

coderabbitai bot commented May 22, 2025

Warning

Rate limit exceeded

@mangelajo has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 42 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between a776220 and 591826f.

📒 Files selected for processing (5)
  • hack/demoenv/exporters-statefulset.yaml (1 hunks)
  • hack/demoenv/kustomization.yaml (2 hunks)
  • hack/demoenv/prepare_exporters.sh (1 hunks)
  • hack/demoenv/qemu-exporters-statefulset.yaml (1 hunks)
  • hack/demoenv/vcan-exporters-statefulset.yaml (1 hunks)

Walkthrough

The changes update Kubernetes StatefulSet manifests for exporters and vcan-exporters to use the latest container image with Always imagePullPolicy, and modify container commands from jmp-exporter run -c to jmp run --exporter. Additionally, a shell script is refactored to replace CLI commands, update exporter configurations, and remove label commands, with new configurations for QEMU exporters.

Changes

Files Change Summary
hack/demoenv/exporters-statefulset.yaml
hack/demoenv/vcan-exporters-statefulset.yaml
Updated container image tag from 0.5.0 to latest, set imagePullPolicy to Always, and changed container command syntax.
hack/demoenv/prepare_exporters.sh Replaced bin/jmpctl with jmp admin commands, added explicit output and label arguments, removed kubectl label usage, updated driver type strings, and printed YAML output for mock exporters.
hack/demoenv/kustomization.yaml Added qemu-exporters-statefulset.yaml to resources, included new QEMU exporter config files in configMapGenerator.

Poem

In the meadow of code where containers dwell,
We pull the latest image, all is well.
Commands now fresh, with arguments new,
Exporters prepped with a streamlined view.
A bunny hops by, with a script in its paw—
“Run it again!” it cheers, in awe.
🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

env:
- name: JUMPSTARTER_GRPC_INSECURE
value: "1"
# note for some reason jmp-exporter run $(cat /etc/hostname) won't find the config, neither does list
command: ["/bin/sh", "-c", "echo running exporter $(cat /etc/hostname); jmp-exporter run -c /etc/jumpstarter/exporters/$(cat /etc/hostname).yaml"]
command: ["/bin/sh", "-c", "echo running exporter $(cat /etc/hostname); jmp run --exporter-config /etc/jumpstarter/exporters/$(cat /etc/hostname).yaml"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
command: ["/bin/sh", "-c", "echo running exporter $(cat /etc/hostname); jmp run --exporter-config /etc/jumpstarter/exporters/$(cat /etc/hostname).yaml"]
command: ["/bin/sh", "-c", "echo running exporter $(cat /etc/hostname); jmp run --exporter $(cat /etc/hostname)"]

env:
- name: JUMPSTARTER_GRPC_INSECURE
value: "1"
# note for some reason jmp-exporter run $(cat /etc/hostname) won't find the config, neither does list
command: ["/bin/sh", "-c", "echo running exporter $(cat /etc/hostname); jmp-exporter run -c /etc/jumpstarter/exporters/$(cat /etc/hostname).yaml"]
command: ["/bin/sh", "-c", "echo running exporter $(cat /etc/hostname); jmp run --exporter-config /etc/jumpstarter/exporters/$(cat /etc/hostname).yaml"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
command: ["/bin/sh", "-c", "echo running exporter $(cat /etc/hostname); jmp run --exporter-config /etc/jumpstarter/exporters/$(cat /etc/hostname).yaml"]
command: ["/bin/sh", "-c", "echo running exporter $(cat /etc/hostname); jmp run --exporter $(cat /etc/hostname)"]

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (3)
hack/demoenv/exporters-statefulset.yaml (2)

19-20: Use explicit version tag rather than latest
As noted above, pin to version 0.6 for reproducibility instead of latest.


25-25: Optional: Use HOSTNAME env var instead of cat /etc/hostname
Same recommendation as in the VCAN manifest: leverage Kubernetes’ HOSTNAME variable and the exec form to avoid sh -c.

hack/demoenv/prepare_exporters.sh (1)

37-37: Portability: fix duplicate sed -i '' usage
This instance of sed -i '' has the same portability issue. Please apply the same Linux-compatible fix as above.

🧹 Nitpick comments (1)
hack/demoenv/vcan-exporters-statefulset.yaml (1)

25-25: Optional: Simplify exporter command via Kubernetes Downward API
Instead of shelling out to cat /etc/hostname, consider using the built-in HOSTNAME env var and an exec form:

command:
  - "jmp"
  - "run"
  - "--exporter-config"
  - "/etc/jumpstarter/exporters/${HOSTNAME}.yaml"

You can ensure HOSTNAME is set via a Downward API fieldRef. This avoids the extra shell layer.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ea0c554 and ecea4a6.

📒 Files selected for processing (3)
  • hack/demoenv/exporters-statefulset.yaml (1 hunks)
  • hack/demoenv/prepare_exporters.sh (1 hunks)
  • hack/demoenv/vcan-exporters-statefulset.yaml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: e2e-tests (ubuntu-24.04-arm)
  • GitHub Check: e2e-tests (ubuntu-24.04)
  • GitHub Check: e2e-tests-28d6b1cc3b49ab9ae176918ab9709a2e2522c97e
🔇 Additional comments (5)
hack/demoenv/prepare_exporters.sh (5)

10-11: Approve: Updated to use jmp admin commands
The delete and create calls have been correctly migrated from bin/jmpctl to jmp admin, with appropriate flags for output and labels.


13-13: Approve: Output generated YAML for visibility
Emitting the file to stdout can aid debugging. If verbosity is a concern, consider gating it behind a DEBUG flag.


17-23: Approve: Updated mock exporter driver type paths
The driver type strings have been consistently renamed from jumpstarter.drivers.* to jumpstarter_driver_*. This aligns with the new package layout.


35-36: Approve: Updated jmp admin commands for CAN exporters
Same migration to jmp admin with --out and -l device-type=can is applied correctly.


41-47: Approve: Updated CAN exporter driver type paths
Driver types for CAN exporters have been updated to the new package format (jumpstarter_driver_can.driver.Can), matching the mock exporter changes.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🔭 Outside diff range comments (1)
hack/demoenv/prepare_exporters.sh (1)

1-1: 🛠️ Refactor suggestion

Enable strict error handling and use Bash shebang
The script currently starts with #!/bin/sh, which may not support set -o pipefail. To catch failures early and avoid silent errors, switch to Bash and enable strict mode.

-#!/bin/sh
+#!/usr/bin/env bash
+set -euo pipefail
♻️ Duplicate comments (1)
hack/demoenv/prepare_exporters.sh (1)

60-60: Make sed -i invocation portable
This BSD/macOS–specific syntax will fail on Linux. Replace with the portable form:

-    sed -i '' '/^\s*export: {}\s*$/d' "${OUT_FILE}"
+    sed -i '/^\s*export: {}\s*$/d' "${OUT_FILE}"
🧹 Nitpick comments (10)
hack/demoenv/qemu-exporters-statefulset.yaml (5)

23-25: Harden container security around privileged mode
Running a privileged container introduces high risk (CKV_K8S_16). If QEMU truly requires full privileges, please add explicit documentation and consider narrowing capabilities:

securityContext:
  privileged: true
  allowPrivilegeEscalation: false
  capabilities:
    drop:
      - ALL
    add:
      - SYS_ADMIN
      - MKNOD

Otherwise, avoid privileged mode.

🧰 Tools
🪛 YAMLlint (1.37.1)

[warning] 24-24: wrong indentation: expected 10 but found 12

(indentation)


27-32: Use binary SI units for memory
Switch from decimal ("2G", "1G") to binary units ("2Gi", "1Gi") for consistency with Kubernetes conventions and clearer resource semantics.


36-37: Refactor shell command to use command + args
Embedding complex shell logic in a single -c string reduces readability. Consider:

command: ["jmp"]
args: ["run", "--exporter-config", "/etc/jumpstarter/exporters/$(HOSTNAME).yaml"]
env:
  - name: HOSTNAME
    valueFrom:
      fieldRef:
        fieldPath: metadata.name

39-47: Consider mounting only the needed config file via subPath
Mounting the entire directory exposes all configs. If each pod only needs /etc/jumpstarter/exporters/<pod-name>.yaml, use:

volumeMounts:
- name: exporter-configs
  mountPath: /etc/jumpstarter/exporters/$(HOSTNAME).yaml
  subPath: $(HOSTNAME).yaml

24-24: Cleanup YAML formatting

  • Align privileged: under securityContext to match two-space indentation.
  • Remove trailing spaces on blank lines (line 48).
  • Correct indentation of the volumes list (line 50).
  • Ensure a newline at the end of the file (line 62).

Also applies to: 48-48, 50-50, 62-62

🧰 Tools
🪛 YAMLlint (1.37.1)

[warning] 24-24: wrong indentation: expected 10 but found 12

(indentation)

hack/demoenv/prepare_exporters.sh (5)

5-5: Quote variable expansions to handle spaces
Unquoted ${OUT_DIR} can break if the path contains spaces. Wrap it in quotes:

- mkdir -p ${OUT_DIR}
+ mkdir -p "${OUT_DIR}"

6-29: Remove or archive large commented-out blocks
Keeping large chunks of legacy code commented out adds maintenance overhead and noise. If these mock-exporter loops are no longer needed, consider removing them or moving them to a separate archival script or documentation.


30-52: Remove or archive additional commented exporter loops
Similarly, the vcan-exporter loop is fully commented out. Clean up these blocks to improve readability or extract them into version-controlled examples if they need to be retained.


54-54: Use POSIX command substitution
Backticks are harder to nest and read—prefer $(…):

- for i in `seq 0 4`; do
+ for i in $(seq 0 4); do

68-70: Parameterize hardcoded OVMF paths for portability
The script assumes QEMU firmware files at /usr/share/AAVMF/.... It’s more robust to allow these paths to be overridden via environment variables and fall back to defaults:

-                OVMF_CODE.fd: /usr/share/AAVMF/OVMF_CODE.fd
-                OVMF_VARS.fd: /usr/share/AAVMF/OVMF_VARS.fd
+                OVMF_CODE.fd: ${OVMF_CODE_PATH:-/usr/share/AAVMF/OVMF_CODE.fd}
+                OVMF_VARS.fd: ${OVMF_VARS_PATH:-/usr/share/AAVMF/OVMF_VARS.fd}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ecea4a6 and 77159e3.

📒 Files selected for processing (3)
  • hack/demoenv/kustomization.yaml (2 hunks)
  • hack/demoenv/prepare_exporters.sh (1 hunks)
  • hack/demoenv/qemu-exporters-statefulset.yaml (1 hunks)
🧰 Additional context used
🪛 Checkov (3.2.334)
hack/demoenv/qemu-exporters-statefulset.yaml

[MEDIUM] 1-62: Containers should not run with allowPrivilegeEscalation

(CKV_K8S_20)


[HIGH] 1-62: Container should not be privileged

(CKV_K8S_16)


[MEDIUM] 1-62: Minimize the admission of root containers

(CKV_K8S_23)

🪛 YAMLlint (1.37.1)
hack/demoenv/qemu-exporters-statefulset.yaml

[warning] 24-24: wrong indentation: expected 10 but found 12

(indentation)


[error] 48-48: trailing spaces

(trailing-spaces)


[warning] 50-50: wrong indentation: expected 6 but found 8

(indentation)


[error] 62-62: no new line character at the end of file

(new-line-at-end-of-file)

🔇 Additional comments (5)
hack/demoenv/kustomization.yaml (1)

6-6: QEMU exporter StatefulSet resource added
The new entry qemu-exporters-statefulset.yaml correctly extends the resources block alongside the existing exporters.

hack/demoenv/qemu-exporters-statefulset.yaml (3)

1-11: Selector and Pod labels are aligned correctly
The spec.selector.matchLabels (exporter-mock: qemu) matches the Pod template labels exactly, ensuring stable network identities for each replica.

🧰 Tools
🪛 Checkov (3.2.334)

[MEDIUM] 1-62: Containers should not run with allowPrivilegeEscalation

(CKV_K8S_20)


[HIGH] 1-62: Container should not be privileged

(CKV_K8S_16)


[MEDIUM] 1-62: Minimize the admission of root containers

(CKV_K8S_23)


13-16: Confirm the requirement for privileged SCC annotation
You’ve added openshift.io/required-scc: privileged to grant elevated permissions on OpenShift. Please verify this is intentional for your demo environment and document it in your cluster’s security policy.


34-35: Verify insecure gRPC setting
JUMPSTARTER_GRPC_INSECURE="1" disables TLS encryption for gRPC. Confirm this is acceptable for a demo and doesn’t leak sensitive data in your test environment.

hack/demoenv/prepare_exporters.sh (1)

86-90: Verify namespace consistency
The script uses NAMESPACE=${NAMESPACE:-"jumpstarter-lab"} for jmp admin commands but hardcodes -n jumpstarter-exporters for Kubernetes operations. Please confirm whether this divergence is intentional or if a separate variable (e.g., EXPORTERS_NAMESPACE) should be introduced for clarity and maintainability.

@mangelajo mangelajo force-pushed the deploy-exporters-0.6 branch from 77159e3 to f2adcd7 Compare May 26, 2025 12:46
@mangelajo mangelajo changed the title Update deploy-exporters to 0.6 Update deploy-exporters to latest cli and add qemu exporters May 26, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (2)
hack/demoenv/prepare_exporters.sh (1)

60-60: Ensure portable sed -i usage
The sed -i '' invocation is macOS-specific and will fail on Linux. Update it to a portable form, for example:

-sed -i '' '/^\s*export: {}\s*$/d' "${OUT_FILE}"
+sed -i '/^\s*export: {}\s*$/d' "${OUT_FILE}"
hack/demoenv/vcan-exporters-statefulset.yaml (1)

19-20: ⚠️ Potential issue

Pin exporter image to specific version
Using latest breaks reproducibility and diverges from the PR title. Replace with the explicit release-0.6 tag:

-        image: quay.io/jumpstarter-dev/jumpstarter:latest
+        image: quay.io/jumpstarter-dev/jumpstarter:release-0.6
🧹 Nitpick comments (2)
hack/demoenv/prepare_exporters.sh (1)

6-28: Remove large commented-out code blocks
The script retains extensive commented loops for mock and vcan exporters. If these are no longer needed, consider removing them to improve readability and maintainability.

Also applies to: 30-53

hack/demoenv/qemu-exporters-statefulset.yaml (1)

22-23: Resolve TODO for continuous release image
There's a TODO suggesting a continuous release image tag (e.g., jumpstarter-with-qemu:latest). Please clarify which image tag should be used or automate version updates.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 77159e3 and f2adcd7.

📒 Files selected for processing (5)
  • hack/demoenv/exporters-statefulset.yaml (1 hunks)
  • hack/demoenv/kustomization.yaml (2 hunks)
  • hack/demoenv/prepare_exporters.sh (1 hunks)
  • hack/demoenv/qemu-exporters-statefulset.yaml (1 hunks)
  • hack/demoenv/vcan-exporters-statefulset.yaml (1 hunks)
🧰 Additional context used
🪛 Checkov (3.2.334)
hack/demoenv/qemu-exporters-statefulset.yaml

[MEDIUM] 1-63: Containers should not run with allowPrivilegeEscalation

(CKV_K8S_20)


[HIGH] 1-63: Container should not be privileged

(CKV_K8S_16)


[MEDIUM] 1-63: Minimize the admission of root containers

(CKV_K8S_23)

🪛 YAMLlint (1.37.1)
hack/demoenv/qemu-exporters-statefulset.yaml

[warning] 25-25: wrong indentation: expected 10 but found 12

(indentation)


[warning] 51-51: wrong indentation: expected 6 but found 8

(indentation)


[error] 63-63: no new line character at the end of file

(new-line-at-end-of-file)

⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: deploy-kind
  • GitHub Check: e2e-tests (ubuntu-24.04-arm)
  • GitHub Check: e2e-tests (ubuntu-24.04)
  • GitHub Check: e2e-tests-28d6b1cc3b49ab9ae176918ab9709a2e2522c97e
🔇 Additional comments (4)
hack/demoenv/kustomization.yaml (1)

6-6:

✅ Verification successful

Add QEMU exporter StatefulSet to resources
The new qemu-exporters-statefulset.yaml has been correctly added under resources. Ensure this file exists and is properly configured.


🏁 Script executed:

#!/bin/bash
# Verify that kustomization references the QEMU StatefulSet manifest
if [[ ! -f "hack/demoenv/qemu-exporters-statefulset.yaml" ]]; then
  echo "❌ Missing hack/demoenv/qemu-exporters-statefulset.yaml"
  exit 1
fi
echo "✅ QEMU StatefulSet manifest is present"

Length of output: 144


QEMU exporter StatefulSet resource correctly added
The qemu-exporters-statefulset.yaml manifest is present in hack/demoenv and properly referenced in hack/demoenv/kustomization.yaml. No further action required.

hack/demoenv/exporters-statefulset.yaml (2)

19-21: Updated exporter image tag and pull policy
The container image is pinned to release-0.6 and imagePullPolicy is set to Always, aligning deployments with the PR objectives.


25-25: Use streamlined run command
The command is updated to invoke jmp run --exporter $(cat /etc/hostname), replacing the previous longer invocation. This aligns with other exporter configurations.

hack/demoenv/qemu-exporters-statefulset.yaml (1)

23-27: Address container privilege requirements
Running the container as privileged: true raises security flags (CKV_K8S_16). If device access must remain privileged, document this need; otherwise, scope down by dropping unnecessary capabilities or disabling privilege escalation.

🧰 Tools
🪛 YAMLlint (1.37.1)

[warning] 25-25: wrong indentation: expected 10 but found 12

(indentation)

Comment on lines 23 to 27
image: quay.io/mangelajo/jumpstarter:rel6-qemu
securityContext:
privileged: true
imagePullPolicy: Always
resources:
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Fix YAML indentation and add newline
YAML lint reports incorrect indentation for privileged (expected 8 spaces, found 12) and a missing newline at EOF. Apply this diff:

-        securityContext:
-            privileged: true
+        securityContext:
+          privileged: true

Ensure a blank line at the end of the file.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
image: quay.io/mangelajo/jumpstarter:rel6-qemu
securityContext:
privileged: true
imagePullPolicy: Always
resources:
image: quay.io/mangelajo/jumpstarter:rel6-qemu
securityContext:
privileged: true
imagePullPolicy: Always
resources:
🧰 Tools
🪛 YAMLlint (1.37.1)

[warning] 25-25: wrong indentation: expected 10 but found 12

(indentation)

🤖 Prompt for AI Agents
In hack/demoenv/qemu-exporters-statefulset.yaml between lines 23 and 27, the
indentation of the privileged field under securityContext is incorrect, causing
YAML lint errors. Fix the indentation to match the expected 8 spaces and add a
newline at the end of the file to resolve formatting issues and adhere to YAML
standards.

env:
- name: JUMPSTARTER_GRPC_INSECURE
value: "1"
# note for some reason jmp-exporter run $(cat /etc/hostname) won't find the config, neither does list
command: ["/bin/sh", "-c", "echo running exporter $(cat /etc/hostname); jmp-exporter run -c /etc/jumpstarter/exporters/$(cat /etc/hostname).yaml"]
command: ["/bin/sh", "-c", "echo running exporter $(cat /etc/hostname); jmp run --exporter (cat /etc/hostname)"]
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Correct shell command substitution
The command incorrectly uses (cat /etc/hostname) instead of shell substitution. It should be:

-        command: ["/bin/sh", "-c", "echo running exporter $(cat /etc/hostname); jmp run --exporter (cat /etc/hostname)"]
+        command: ["/bin/sh", "-c", "echo running exporter $(cat /etc/hostname); jmp run --exporter $(cat /etc/hostname)"]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
command: ["/bin/sh", "-c", "echo running exporter $(cat /etc/hostname); jmp run --exporter (cat /etc/hostname)"]
command: ["/bin/sh", "-c", "echo running exporter $(cat /etc/hostname); jmp run --exporter $(cat /etc/hostname)"]
🤖 Prompt for AI Agents
In hack/demoenv/vcan-exporters-statefulset.yaml at line 25, the command uses
incorrect syntax for shell command substitution. Replace the parentheses around
(cat /etc/hostname) with dollar sign and curly braces to properly perform shell
substitution, making it $(cat /etc/hostname). This ensures the hostname is
correctly inserted into the command string.

@mangelajo mangelajo force-pushed the deploy-exporters-0.6 branch from f2adcd7 to a776220 Compare May 26, 2025 13:08
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🔭 Outside diff range comments (1)
hack/demoenv/prepare_exporters.sh (1)

86-90: ⚠️ Potential issue

Inconsistent namespace usage—use the ${NAMESPACE} variable.
You define NAMESPACE at the top but hardcode jumpstarter-exporters below, leading to mismatches if overridden. Replace with the variable:

- kubectl delete statefulset -n jumpstarter-exporters exporter vcan-exporter
+ kubectl delete statefulset -n "${NAMESPACE}" exporter vcan-exporter
- kubectl delete pod --all -n jumpstarter-exporters --force --grace-period=0
+ kubectl delete pod --all -n "${NAMESPACE}" --force --grace-period=0
- kubectl create namespace jumpstarter-exporters || true
+ kubectl create namespace "${NAMESPACE}" || true
♻️ Duplicate comments (3)
hack/demoenv/exporters-statefulset.yaml (1)

19-20: Consider pinning image versions for reproducible deployments.
Using latest can introduce unexpected changes in production. An explicit version tag helps maintain consistency.

hack/demoenv/prepare_exporters.sh (1)

60-60: ⚠️ Potential issue

Make sed -i invocation portable.
sed -i '' is macOS-specific and will fail on Linux. Replace with a portable in-place edit:

-    sed -i '' '/^\s*export: {}\s*$/d' "${OUT_FILE}"
+    sed -i '/^\s*export: {}\s*$/d' "${OUT_FILE}"
hack/demoenv/kustomization.yaml (1)

20-24: ConfigMapGenerator references missing QEMU exporter config files.
The entries for gen/qemu-exporter-{0..4}.yaml must match actual files under hack/demoenv/gen. Ensure the prepare_exporters.sh run generates these before applying, or add the files to source control.

🧹 Nitpick comments (3)
hack/demoenv/prepare_exporters.sh (1)

6-29: Remove large commented-out blocks or extract to documentation.
The loops for mock and CAN exporters are fully commented out. If these sections are no longer needed, consider deleting them or moving to a separate archival script to keep this file concise.

hack/demoenv/qemu-exporters-statefulset.yaml (2)

22-24: Address the TODO or update the image reference.
The # TODO indicates you plan to switch to a continuously published image. Either remove the comment once the pipeline exists or update to the correct image path.


63-63: Add a newline at end of file.
YAML parsers and linters expect a trailing newline.

🧰 Tools
🪛 Checkov (3.2.334)

[MEDIUM] 1-63: Containers should not run with allowPrivilegeEscalation

(CKV_K8S_20)


[HIGH] 1-63: Container should not be privileged

(CKV_K8S_16)


[MEDIUM] 1-63: Minimize the admission of root containers

(CKV_K8S_23)

🪛 YAMLlint (1.37.1)

[error] 63-63: no new line character at the end of file

(new-line-at-end-of-file)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f2adcd7 and a776220.

📒 Files selected for processing (5)
  • hack/demoenv/exporters-statefulset.yaml (1 hunks)
  • hack/demoenv/kustomization.yaml (2 hunks)
  • hack/demoenv/prepare_exporters.sh (1 hunks)
  • hack/demoenv/qemu-exporters-statefulset.yaml (1 hunks)
  • hack/demoenv/vcan-exporters-statefulset.yaml (1 hunks)
🧰 Additional context used
🪛 Checkov (3.2.334)
hack/demoenv/qemu-exporters-statefulset.yaml

[MEDIUM] 1-63: Containers should not run with allowPrivilegeEscalation

(CKV_K8S_20)


[HIGH] 1-63: Container should not be privileged

(CKV_K8S_16)


[MEDIUM] 1-63: Minimize the admission of root containers

(CKV_K8S_23)

🪛 YAMLlint (1.37.1)
hack/demoenv/qemu-exporters-statefulset.yaml

[warning] 51-51: wrong indentation: expected 6 but found 8

(indentation)


[error] 63-63: no new line character at the end of file

(new-line-at-end-of-file)

⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: e2e-tests (ubuntu-24.04-arm)
  • GitHub Check: e2e-tests (ubuntu-24.04)
  • GitHub Check: e2e-tests-28d6b1cc3b49ab9ae176918ab9709a2e2522c97e
🔇 Additional comments (6)
hack/demoenv/vcan-exporters-statefulset.yaml (2)

19-20: Consider pinning image versions for reproducible deployments.
Switching to the latest tag can lead to non-deterministic rollouts. If auditability or rollback safety is a priority, you may want to pin to a specific semver (e.g., 0.6.0).


25-25: Command substitution is correct, good update.
The switch to jmp run --exporter $(cat /etc/hostname) aligns with the new CLI and properly resolves the container’s hostname.

hack/demoenv/exporters-statefulset.yaml (1)

25-25: Command substitution is correct, nice work.
Switching from jmp-exporter run to jmp run --exporter $(cat /etc/hostname) reflects the updated CLI usage.

hack/demoenv/qemu-exporters-statefulset.yaml (2)

24-27: Verify privileged container requirements and security context.
Running as privileged introduces security risks. If QEMU access to /dev/kvm and /dev/vhost-vsock is required, consider:

  • Adding allowPrivilegeEscalation: false
  • Limiting capabilities rather than full privilege
    Please confirm these settings meet your security policies.

38-38: Command looks good.
Running jmp run --exporter $(cat /etc/hostname) matches the pattern in other manifests and properly resolves the hostname.

hack/demoenv/kustomization.yaml (1)

6-6: Added QEMU exporter StatefulSet to resources—looks correct.
The new qemu-exporters-statefulset.yaml is referenced alongside the existing resources.

Comment on lines 50 to 53
volumes:
- name: exporter-configs
configMap:
name: exporter-configs
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix YAML indentation for volume definitions.
YAML lint reports incorrect indentation under volumes:. Align the list items two spaces under the parent key.

🧰 Tools
🪛 YAMLlint (1.37.1)

[warning] 51-51: wrong indentation: expected 6 but found 8

(indentation)

🤖 Prompt for AI Agents
In hack/demoenv/qemu-exporters-statefulset.yaml between lines 50 and 53, the
indentation of the volume list under the volumes key is incorrect, which can
cause YAML parsing errors. Fix the indentation by ensuring the dash and nested
keys are indented two spaces under the volumes key, aligning with YAML syntax
standards.

@mangelajo mangelajo force-pushed the deploy-exporters-0.6 branch from a776220 to 282729c Compare May 26, 2025 13:34
Add basic intel qemu exporters, and update the CLI to latest versions.
@mangelajo mangelajo force-pushed the deploy-exporters-0.6 branch from 282729c to 591826f Compare May 26, 2025 13:37
@mangelajo mangelajo requested a review from NickCao May 26, 2025 13:39
containers:
- name: jumpstarter-exporter
# TODO: we need a continuous release of this image in jumpstarter. i.e. quay.io/jumpstarter/jumpstarter-with-qemu:latest
image: quay.io/mangelajo/jumpstarter:rel6-qemu
Copy link
Member Author

Choose a reason for hiding this comment

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

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

Successfully merging this pull request may close these issues.

2 participants