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 |
📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 |
|
/lgtm |
|
Scheduling required tests: |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jsafrane, PoornimaSingour 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 |
openshift-ci
Bot
added
the
approved
jsafrane
changed the title
openshift-ci-robot
added
jira/valid-reference
|
@PoornimaSingour: This pull request references Jira Issue OCPBUGS-105392, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/test hypershift-e2e-aks |
|
End-to-End Fix Validation Summary Bug : aws-ebs-csi-driver-controller deployment missing proxy environment variables (HTTP_PROXY, HTTPS_PROXY, NO_PROXY) on proxy-required management clusters, causing PVC provisioning failures due to STS timeout when reaching AWS APIs. Fix — 3 PRs across 3 repos PR 1: openshift/hypershift#9256 (CPO)
PR 2: openshift/cluster-storage-operator#723 (CSO)
PR 3: #594 (csi-operator)
Test Environment
Custom Images Built and Deployed
How Custom Images Were Applied
Proxy Chain Validation Step 1: CPO sets proxy on CSO Step 2: CSO sets proxy on aws-ebs-csi-driver-operator Step 3: aws-ebs-csi-driver-operator sets proxy on aws-ebs-csi-driver-controller (all 12 containers) PVC Provisioning Test (end-to-end proof) Guest cluster nodes PVC status — Bound PV details — gp3 EBS volume provisioned Pod status — Running This proves the aws-ebs-csi-driver-controller can now reach AWS STS through the proxy to assume the IAM role and call EC2 CreateVolume successfully. |
|
/retest |
|
/retest hypershift-e2e-aks |
|
/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. |
4 participants
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