Skip to content

Fix missing image pull secrets for the mock device plugin - #3168

Closed
sanjayy0612 wants to merge 1 commit into
Project-HAMi:masterfrom
sanjayy0612:fix/mock-device-plugin-pull-secrets
Closed

sanjayy0612 wants to merge 1 commit into
Project-HAMi:masterfrom
sanjayy0612:fix/mock-device-plugin-pull-secrets

Conversation

@sanjayy0612

@sanjayy0612 sanjayy0612 commented Oct 4, 2026 •

Copy link
Copy Markdown

What type of PR is this?

/kind bug

What this PR does / why we need it:

The mock device plugin DaemonSet ignores mockDevicePlugin.image.pullSecrets and global.imagePullSecrets. Its image can therefore fail to pull from an authenticated private registry even when the chart values provide credentials.

Add a mock-plugin pull-secret helper using the existing common.images.pullSecrets renderer and include it in the DaemonSet Pod spec. Global and plugin secrets are both retained, and empty configuration continues to omit imagePullSecrets.

Which issue(s) this PR fixes:

Fixes #3167

Special notes for your reviewer:

  • Verified with Helm v3.21.4 and YAML parsing: empty, global-only, plugin-only, combined, and multiple-secret configurations. Compared each rendered Pod with the unmodified baseline: all other Pod fields remain unchanged. Also verified that a disabled mock plugin emits no DaemonSet.
  • Helm lint passed with default and configured mock-plugin values; default chart rendering, chart/app version equality, and git diff --check passed.
  • make verify_chart passed its lint, rendering, and version steps but could not run the Trivy scan because the local Docker daemon is unavailable.
  • make verify could not complete: Go dependency downloads failed with network errors on two attempts (github.com/urfave/cli/v2 and k8s.io/client-go). The resulting type-check errors were missing-dependency errors; no Go files are changed by this PR.
  • No live private-registry pull was tested. No accelerator allocation or isolation behavior is changed.
  • AI disclosure: Codex inspected the chart, generated the code change, performed validation, and drafted the issue, commit message, and PR description at the contributor's request.

Does this PR introduce a user-facing change?:

Yes. The mock device plugin Pod now includes configured global and mock-plugin image pull secrets.

Summary by CodeRabbit

  • New Features
    • Mock device-plugin images can now use configured image pull secrets, supporting deployments that pull images from authenticated registries.

Copilot AI balanced review requested due to automatic review settings October 4, 2026 15:06
@hami-robot hami-robot Bot added the kind/bug Something isn't working label Oct 4, 2026
@hami-robot
hami-robot Bot requested a review from lengrongfu October 4, 2026 15:06
@hami-robot

hami-robot Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sanjayy0612
Once this PR has been reviewed and has the lgtm label, please assign moezdil for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@hami-robot
hami-robot Bot requested a review from ouyangluwei163 October 4, 2026 15:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 16eb644b-3b2a-4017-bf88-57b9ea46c7b8
📥 Commits

Reviewing files that changed from the base of the PR and between 7ae1f5e and c25edad.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 942a50c5-c7e8-46ae-86f0-854f780b5400
📥 Commits

Reviewing files that changed from the base of the PR and between 43a8cff and 7ae1f5e.

📒 Files selected for processing (2)
  • charts/hami/templates/_helpers.tpl
  • charts/hami/templates/device-plugin/daemonsetmock.yaml

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The mock device plugin DaemonSet now renders image pull secrets through a new Helm helper that uses the mock device plugin image and global image settings.

Changes

Mock device plugin pull secrets

Layer / File(s) Summary
Render pull secrets in the mock DaemonSet
charts/hami/templates/_helpers.tpl, charts/hami/templates/device-plugin/daemonsetmock.yaml
A new helper passes the mock device plugin image and global settings to the shared pull-secret template. The mock DaemonSet pod specification uses this helper.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested labels: enhancement

Suggested reviewers: spencercjh

Merge Risk: ⚪ Minimal · up to 7ae1f

The mock DaemonSet now receives global and mock-specific pull secrets, while empty configuration continues to omit the field. No actionable merge risk remains.

Architecture Summary

Architecture risk: 🔵 Low · up to 7ae1f

The change affects 1 system.

Changed systems: charts

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — charts (service) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in charts/hami/templates/_helpers.tpl: Adds hami.mockDevicePlugin.imagePullSecrets, passing the mock device plugin image and global settings to the shared pull-secret template.
  • observed — Modified behavior in charts/hami/templates/device-plugin/daemonsetmock.yaml: The pod specification now includes image pull secrets rendered by hami.mockDevicePlugin.imagePullSecrets.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: adding missing image pull secrets for the mock device plugin.
Linked Issues check ✅ Passed Issue #3167 requires the mock-plugin Pod to render global and plugin image pull secrets and omit the field when configuration is empty. The new hami.mockDevicePlugin.imagePullSecrets helper passes `…
Out of Scope Changes check ✅ Passed The reviewed changes add the helper and its use in the mock-plugin DaemonSet. Both changes directly implement issue #3167. No unrelated changes appear in the reported PR scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
✨ Finishing Touches 💡 1
🧪 Generate unit tests (beta)
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

A rabbit checks the chart at night
And finds the secrets tucked in right
The mock pod reads the values through
Global and plugin secrets too
Then hops away beneath the moon

Comment @coderabbitai help to get the list of available commands.

@spencercjh spencercjh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

plz fix dco

Signed-off-by: sanjayy0612 <sanjayelango06@gmail.com>
@sanjayy0612
sanjayy0612 force-pushed the fix/mock-device-plugin-pull-secrets branch from 7ae1f5e to c25edad Compare October 4, 2026 15:44
@moezdil

moezdil commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

This is being closed because it does not comply with the contribution guidelines.

You need:

https://github.com/Project-HAMi/HAMi/blob/master/CONTRIBUTING.md#ai-assistance-notice

@moezdil moezdil closed this Oct 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement kind/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Helm chart ignores mock device plugin image pull secrets

4 participants