fix(OCPBUGS-105392): Add proxy hook for HyperShift CSI driver controller deployments - #594
Conversation
…ler deployments The HyperShift mode was missing a proxy hook in NewDefaultOperatorControllerConfig, leaving CSI driver controller containers without HTTP_PROXY, HTTPS_PROXY, and NO_PROXY when the management cluster requires a proxy for external access.
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: PoornimaSingour The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe HyperShift deployment hook list now includes Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/driver/common/operator/hooks.go`:
- Line 46: Update the flavour branching around AddDeploymentHookBuilders so the
HyperShift-only builders are registered exclusively when ClusterFlavour equals
generator.FlavourHyperShift. Keep generator.FlavourStandalone on the regular
path and handle any unsupported flavour explicitly instead of routing it through
the HyperShift branch.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 99f83c41-99a4-4326-8cd2-349cd7e081be
📒 Files selected for processing (2)
pkg/driver/common/operator/hooks.gopkg/driver/common/operator/hooks_test.go
|
/retest |
|
@PoornimaSingour: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Summary
withHyperShiftProxyhook to inject management cluster proxy env vars (HTTP_PROXY,HTTPS_PROXY,NO_PROXY) into CSI driver controller containers in HyperShift mode.NewDefaultOperatorControllerConfigfor the HyperShift branchProblem
In
NewDefaultOperatorControllerConfig(), the standalone branch registerswithClusterWideProxywhich reads proxy from the guest cluster'sobservedConfig. The HyperShift branch has no proxy hook at all, leaving CSI driver controller containers (csi-driver, csi-provisioner, csi-attacher, etc.) unable to reach cloud APIs when the management cluster requires a proxy.Fix
The new
withHyperShiftProxyhook reads proxy env vars fromos.Getenv()(management cluster proxy) and injects them into all containers. This follows the same pattern aswithHyperShiftControlPlaneImagesandwithHyperShiftRunAsUserin the same file. It is a no-op when no proxy is configured. Standalone mode is unaffected.This is step 3 of a 3-repo fix:
withHyperShiftProxyhook to inject proxy into CSI driver controller containersTest plan
go test -race ./pkg/driver/common/operator/... -vmake testpassesmake verifypassesJIRA
OCPBUGS-105391