OCPBUGS-99490: Apply some static assets earlier, before starting controllers#584
OCPBUGS-99490: Apply some static assets earlier, before starting controllers#584mpatlasov wants to merge 1 commit into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@mpatlasov: This pull request references Jira Issue OCPBUGS-99490, which is valid. 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. |
openshift-ci-robot
added
the
jira/valid-bug
|
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: Enterprise Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe operator configuration now declares prerequisite asset filenames, with the GCP PD driver providing service account and RBAC assets. A new helper applies these files with retries, error aggregation, logging, delays, and context cancellation. Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mpatlasov 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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/operator/prerequisites_test.go`:
- Line 110: Update the test setup at the fake clientset initialization to
replace the deprecated fakecore.NewSimpleClientset() constructor with
fakecore.NewClientset(), preserving the existing client behavior.
In `@pkg/operator/prerequisites.go`:
- Around line 61-63: Replace the unconditional time.Sleep retry backoff in the
prerequisite retry flow with a context-aware timer/select using the existing
context.Context, returning promptly when cancellation or timeout occurs while
preserving the delay before the next attempt when the context remains active.
In `@pkg/operator/starter.go`:
- Around line 271-274: Move the applyPrerequisites call in starterController
before the Precondition branch so it executes for both startup paths, including
configurations that define both Precondition and PrerequisiteAssets. Preserve
the existing error propagation and asset arguments.
🪄 Autofix (Beta)
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: Enterprise
Run ID: 2bf677af-2bd8-48cd-bd79-16b1fca0b40d
📒 Files selected for processing (5)
pkg/driver/gcp-pd/gcp_pd.gopkg/operator/config/config.gopkg/operator/prerequisites.gopkg/operator/prerequisites_test.gopkg/operator/starter.go
|
/payload-job periodic-ci-openshift-release-main-ci-5.0-e2e-gcp-ovn-techpreview |
|
@mpatlasov: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/bdd1d2a0-870f-11f1-8d71-c8ea5723ebf9-0 |
|
/payload-job periodic-ci-openshift-release-main-ci-5.0-e2e-gcp-ovn-techpreview |
|
@mpatlasov: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/9f1e9cb0-876b-11f1-91ae-bccd9c78cad4-0 |
CSIControllerSet.Run() starts all controllers as parallel goroutines with no ordering guarantee.
The Deployment controller can create pods before StaticResourcesController applies the assets for ClusterRole and ClusterRoleBinding, causing SCC admission to deny pod creation.
Such race-condition leads to events like:
```
message: 'Error creating: pods "gcp-pd-csi-driver-controller-6d9b8c6584-" is forbidden:
unable to validate against any security context constraint: provider "hostnetwork-v2":
Forbidden: not usable by user or serviceaccount'
```
The commit fixes the issue by applying all assets defined in OperatorConfig.PrerequisiteAssets before controllers start.
67852d3 to
d6b5c3b
Compare
|
Last force-push fixed three suggestions from @coderabbitai:
|
|
Tip For best results, initiate chat on the files or code changes.
|
|
/lgtm |
|
Scheduling required tests: Scheduling tests matching the |
|
/retest |
|
/verified by mpatlasov
|
openshift-ci-robot
added
the
verified
|
@mpatlasov: This PR has been marked as verified by 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. |
|
/retest-required |
|
/retest-required |
|
/test e2e-azure |
|
/test e2e-gcp-ovn-upgrade |
|
/test e2e-azure |
|
/test e2e-gcp-ovn-upgrade |
|
@mpatlasov: The following tests 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
https://redhat.atlassian.net/browse/OCPBUGS-99490
CSIControllerSet.Run() starts all controllers as parallel goroutines with no ordering guarantee. The Deployment controller can create pods before StaticResourcesController applies the assets for ClusterRole and ClusterRoleBinding, causing SCC admission to deny pod creation.
Such race-condition leads to events like:
The commit fixes the issue by applying all assets defined in OperatorConfig.PrerequisiteAssets before controllers start.