OCPBUGS-85337: set imagePullPolicy IfNotPresent on provider workloads#302
Conversation
openshift-ci-robot
added
the
jira/severity-moderate
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
openshift-ci-robot
added
the
jira/valid-reference
|
Skipping CI for Draft Pull Request. |
openshift-ci-robot
added
the
jira/invalid-bug
|
@damdo: This pull request references Jira Issue OCPBUGS-85337, which is invalid:
Comment 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. |
WalkthroughThis PR updates the cluster-capi-controllers manifest image pull policy and bumps the ChangesManifest and tool module update
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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: 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 `@openshift/tools/go.mod`:
- Around line 78-79: The go.mod entry for
github.com/openshift/cluster-capi-operator/manifests-gen still uses a local
absolute-path replace, which must be removed before merge. Update the module
dependency in go.mod to a pinned upstream require version instead of the
filesystem path, then refresh the module metadata and vendored contents as
needed so the dependency is portable for CI and other contributors.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0375059b-4454-4439-a184-fbcc31efbda6
⛔ Files ignored due to path filters (5)
openshift/tools/go.sumis excluded by!**/*.sumopenshift/tools/vendor/github.com/openshift/cluster-capi-operator/manifests-gen/LICENSEis excluded by!**/vendor/**openshift/tools/vendor/github.com/openshift/cluster-capi-operator/manifests-gen/kustomization.yamlis excluded by!**/vendor/**openshift/tools/vendor/github.com/openshift/cluster-capi-operator/manifests-gen/kustomize-values.yamlis excluded by!**/vendor/**openshift/tools/vendor/modules.txtis excluded by!**/vendor/**
📒 Files selected for processing (2)
openshift/capi-operator-manifests/default/manifests.yamlopenshift/tools/go.mod
|
|
||
| replace github.com/openshift/cluster-capi-operator/manifests-gen => /Users/ddonati/src/cluster-capi-operator/manifests-gen |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Remove local filesystem replace directive before merge.
This replace points to an absolute path (/Users/ddonati/...) on a specific developer machine, which will break builds for every other contributor and CI. Per the PR description, this is meant to vendor a fix pending merge of openshift/cluster-capi-operator#618 — once that PR merges, this should be replaced with a pinned upstream require version (with go mod tidy/vendor updated), not a local path replace.
🐛 Proposed fix
-replace github.com/openshift/cluster-capi-operator/manifests-gen => /Users/ddonati/src/cluster-capi-operator/manifests-gen
+// TODO: remove once github.com/openshift/cluster-capi-operator#618 merges and
+// the manifests-gen require is bumped to the released version.📝 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.
| replace github.com/openshift/cluster-capi-operator/manifests-gen => /Users/ddonati/src/cluster-capi-operator/manifests-gen | |
| // TODO: remove once github.com/openshift/cluster-capi-operator#618 merges and | |
| // the manifests-gen require is bumped to the released version. |
🤖 Prompt for 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.
In `@openshift/tools/go.mod` around lines 78 - 79, The go.mod entry for
github.com/openshift/cluster-capi-operator/manifests-gen still uses a local
absolute-path replace, which must be removed before merge. Update the module
dependency in go.mod to a pinned upstream require version instead of the
filesystem path, then refresh the module metadata and vendored contents as
needed so the dependency is portable for CI and other contributors.
damdo
force-pushed
the
OCPBUGS-85337-imagepullpolicy-ifnotpresent
branch
from
f0c5a2e to
993b05a
Compare
damdo
changed the title
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
mdbooth
left a comment
mdbooth
left a comment
There was a problem hiding this comment.
/approve
/hold for openshift/cluster-capi-operator#618 and revendor
| fieldPath: metadata.uid | ||
| image: registry.ci.openshift.org/openshift:cluster-capi-controllers | ||
| imagePullPolicy: Always | ||
| imagePullPolicy: IfNotPresent |
…gePullPolicy fix Vendor manifests-gen from openshift/cluster-capi-operator#618 which sets imagePullPolicy: IfNotPresent on all containers in all provider workloads via kustomize, fixing disconnected cluster upgrades using pinned image sets (OCPBUGS-85337).
Regenerated via `make ocp-manifests` after vendoring the manifests-gen fix. The capi-controller-manager Deployment container now has imagePullPolicy: IfNotPresent instead of Always, fixing disconnected cluster upgrades using pinned image sets (OCPBUGS-85337).
damdo
force-pushed
the
OCPBUGS-85337-imagepullpolicy-ifnotpresent
branch
from
993b05a to
86aabf0
Compare
openshift-ci-robot
added
the
jira/valid-bug
|
@damdo: This pull request references Jira Issue OCPBUGS-85337, which is valid. 3 validation(s) were run on this bug
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
removed
the
jira/invalid-bug
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
|
/retest |
|
@damdo: This PR was included in a payload test run from openshift/cluster-api-provider-aws#619 |
|
Tracking the testing of this here: https://redhat-internal.slack.com/archives/C05KZA3NVU6/p1783433131168649?thread_ts=1783088873.664179&cid=C05KZA3NVU6 |
|
/verified by @damdo I can see no |
openshift-ci-robot
added
the
verified
|
@damdo: 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. |
|
/hold |
|
/unhold |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
/retest-required |
|
/tide refresh |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: damdo, mdbooth 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 |
|
/test e2e-azure-capi-techpreview e2e-aws-serial-2of2 e2e-aws-ovn-techpreview e2e-aws-ovn |
|
/retest |
|
@damdo: all tests passed! 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. |
|
@damdo: Jira Issue Verification Checks: Jira Issue OCPBUGS-85337 Jira Issue OCPBUGS-85337 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 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. |
3 participants
Summary
manifests-genfix from OCPBUGS-85337: fix(manifests-gen): set imagePullPolicy IfNotPresent on provider workloads cluster-capi-operator#618 (via localgo.modreplace) and regenerates manifests.capi-controller-managerDeployment container changes fromimagePullPolicy: AlwaystoIfNotPresent.ImagePullBackOffbecause the kubelet tries to pull images that are already present on the node.Depends on: openshift/cluster-capi-operator#618
TODO
go.modreplace directive and re-vendormanifests-genfrom upstreamTest plan
capi-controller-managerpods no longer crash withImagePullBackOffcapi-controller-managerDeployment hasimagePullPolicy: IfNotPresentSummary by CodeRabbit
Bug Fixes
Chores