USHIFT-6778: rebase-release-4.22-4.22.0-0.nightly-2026-04-01-151631_amd64-2026-04-01_arm64-2026-04-02#6456
Conversation
|
@eslutsky: This pull request references USHIFT-6778 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.22.0" version, but no target version was set. 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. |
WalkthroughUpdated Kubernetes from v1.35.2 to v1.35.3, bumped OpenShift nightly release versions across multiple architectures, refreshed container image digests in release manifests and kustomization files, and applied upstream Kubernetes dependency updates including feature gate and controller logic adjustments. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.4)level=error msg="Running error: context loading failed: failed to load packages: failed to load packages: failed to load with go/packages: err: exit status 1: stderr: go: inconsistent vendoring in :\n\tgithub.com/apparentlymart/go-cidr@v1.1.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/coreos/go-systemd@v0.0.0-20190321100706-95778dfbb74e: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/google/go-cmp@v0.7.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/miekg/dns@v1.1.63: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/openshift/api@v0.0.0-20260309155933-45fd88d185dd: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/openshift/build-machinery-go@v0.0.0-20251023084048-5d77c1a5e5af: is explicitly required in go.mod, but not marked as explicit ... [truncated 29518 characters] ... belet: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/metrics: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/mount-utils: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/pod-security-admission: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/sample-apiserver: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/sample-cli-plugin: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/sample-controller: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\n\tTo ignore the vendor directory, use -mod=readonly or -mod=mod.\n\tTo sync the vendor directory, run:\n\t\tgo mod vendor\n" 🔧 Trivy (0.69.3)Trivy execution failed: Unknown error Comment |
Signed-off-by: Evgeny Slutsky <eslutsky@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
deps/github.com/openshift/kubernetes/pkg/controller/devicetainteviction/device_taint_eviction.go (1)
1285-1307:⚠️ Potential issue | 🟠 MajorMultiple workers can race on status update timing.
Queueing the rule-status item before
handlePodsdoesn't guarantee it executes first whenRunuses more than one worker. After the mutex releases, concurrent workers can dequeue items in any order, so the "status before pods" behavior is not deterministic. The test forcesRun(..., 1)specifically to avoid this race, as documented in the test comment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deps/github.com/openshift/kubernetes/pkg/controller/devicetainteviction/device_taint_eviction.go` around lines 1285 - 1307, The current approach enqueues a status-update work item (workqueue.Add(workItemForRule(newRule))) expecting it to run before pod-eviction handling, but multiple workers (Run with >1) can dequeue in any order and race with handlePods; to fix, perform the immediate status update synchronously instead of relying on the queue: call the controller's status sync path directly while still holding the relevant mutex (e.g., invoke the rule status update helper used by workers or a new tc.syncRuleStatus(tc, newRule) function) before releasing the lock and before adding the eviction work item, then keep the workqueue.Add(workItemForRule(newRule)) only for subsequent async processing; this ensures status is updated deterministically prior to handlePods regardless of Run worker count and avoids relying on workqueue ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@deps/github.com/openshift/kubernetes/cmd/kubeadm/app/util/etcd/etcd.go`:
- Around line 536-539: After promoting a learner to a voting member (flow
involving AddMemberAsLearner -> MemberPromote), the client’s cached endpoints
(c.Endpoints) are not refreshed so WaitForClusterAvailable can skip the new
member; call c.Sync() immediately after MemberPromote completes to refresh
endpoints before any availability/health checks (e.g., before invoking
WaitForClusterAvailable), and handle/log any Sync() error so the join sequence
fails fast if endpoint refresh fails.
In
`@deps/github.com/openshift/kubernetes/pkg/controller/devicetainteviction/device_taint_eviction.go`:
- Around line 1493-1498: The debounce loop scheduling a delayed status update
can miss the initial transition to EvictionInProgress=True; change the logic in
the eviction.reason loop (where workqueue.AddAfter(workItemForRule(reason.rule),
ruleStatusPeriod) is called) to trigger an immediate status update via
maybeUpdateRuleStatus for the rule when countPendingPods for that rule is > 0
(i.e., first pending pod) and the rule's status is not already
EvictionInProgress, then still schedule the delayed workItemForRule(reason.rule)
for subsequent updates; use the existing functions maybeUpdateRuleStatus,
countPendingPods and workItemForRule to detect and perform the immediate update
before adding the delayed work.
---
Outside diff comments:
In
`@deps/github.com/openshift/kubernetes/pkg/controller/devicetainteviction/device_taint_eviction.go`:
- Around line 1285-1307: The current approach enqueues a status-update work item
(workqueue.Add(workItemForRule(newRule))) expecting it to run before
pod-eviction handling, but multiple workers (Run with >1) can dequeue in any
order and race with handlePods; to fix, perform the immediate status update
synchronously instead of relying on the queue: call the controller's status sync
path directly while still holding the relevant mutex (e.g., invoke the rule
status update helper used by workers or a new tc.syncRuleStatus(tc, newRule)
function) before releasing the lock and before adding the eviction work item,
then keep the workqueue.Add(workItemForRule(newRule)) only for subsequent async
processing; this ensures status is updated deterministically prior to handlePods
regardless of Run worker count and avoids relying on workqueue ordering.
🪄 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), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 75dc4b3c-c996-41e9-9827-89d3b7eee4af
⛔ Files ignored due to path filters (21)
deps/github.com/openshift/kubernetes/staging/src/k8s.io/sample-apiserver/pkg/generated/clientset/versioned/fake/clientset_generated.gois excluded by!**/generated/**deps/github.com/openshift/kubernetes/staging/src/k8s.io/sample-controller/pkg/generated/clientset/versioned/fake/clientset_generated.gois excluded by!**/generated/**etcd/go.sumis excluded by!**/*.sumetcd/vendor/github.com/openshift/api/config/v1/types.gois excluded by!**/vendor/**etcd/vendor/github.com/openshift/api/config/v1/types_authentication.gois excluded by!**/vendor/**etcd/vendor/github.com/openshift/api/config/v1/types_dns.gois excluded by!**/vendor/**etcd/vendor/github.com/openshift/api/config/v1/types_infrastructure.gois excluded by!**/vendor/**etcd/vendor/github.com/openshift/api/config/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**etcd/vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**etcd/vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**etcd/vendor/github.com/openshift/api/operator/v1/types_csi_cluster_driver.gois excluded by!**/vendor/**etcd/vendor/github.com/openshift/api/operator/v1/types_ingress.gois excluded by!**/vendor/**etcd/vendor/github.com/openshift/api/operator/v1/types_network.gois excluded by!**/vendor/**etcd/vendor/github.com/openshift/api/operator/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**etcd/vendor/github.com/openshift/api/operator/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**etcd/vendor/github.com/openshift/api/operator/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**etcd/vendor/modules.txtis excluded by!**/vendor/**vendor/k8s.io/client-go/kubernetes/fake/clientset_generated.gois excluded by!vendor/**,!**/vendor/**vendor/k8s.io/kubernetes/pkg/controller/devicetainteviction/device_taint_eviction.gois excluded by!vendor/**,!**/vendor/**vendor/k8s.io/kubernetes/pkg/features/kube_features.gois excluded by!vendor/**,!**/vendor/**vendor/modules.txtis excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (45)
Makefile.kube_git.varMakefile.version.aarch64.varMakefile.version.x86_64.varassets/components/multus/kustomization.aarch64.yamlassets/components/multus/release-multus-aarch64.jsonassets/components/multus/release-multus-x86_64.jsonassets/optional/operator-lifecycle-manager/kustomization.aarch64.yamlassets/optional/operator-lifecycle-manager/kustomization.x86_64.yamlassets/optional/operator-lifecycle-manager/release-olm-aarch64.jsonassets/optional/operator-lifecycle-manager/release-olm-x86_64.jsonassets/release/release-aarch64.jsonassets/release/release-x86_64.jsondeps/github.com/openshift/kubernetes/CHANGELOG/CHANGELOG-1.35.mddeps/github.com/openshift/kubernetes/build/common.shdeps/github.com/openshift/kubernetes/build/dependencies.yamldeps/github.com/openshift/kubernetes/cmd/kubeadm/app/cmd/phases/reset/unmount_linux.godeps/github.com/openshift/kubernetes/cmd/kubeadm/app/util/etcd/etcd.godeps/github.com/openshift/kubernetes/openshift-hack/cmd/k8s-tests-ext/provider.godeps/github.com/openshift/kubernetes/openshift-hack/images/hyperkube/Dockerfile.rheldeps/github.com/openshift/kubernetes/pkg/controller/devicetainteviction/device_taint_eviction.godeps/github.com/openshift/kubernetes/pkg/controller/devicetainteviction/device_taint_eviction_test.godeps/github.com/openshift/kubernetes/pkg/features/kube_features.godeps/github.com/openshift/kubernetes/staging/src/k8s.io/apiextensions-apiserver/examples/client-go/pkg/client/clientset/versioned/fake/clientset_generated.godeps/github.com/openshift/kubernetes/staging/src/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/fake/clientset_generated.godeps/github.com/openshift/kubernetes/staging/src/k8s.io/apiextensions-apiserver/test/integration/finalization_test.godeps/github.com/openshift/kubernetes/staging/src/k8s.io/client-go/kubernetes/fake/clientset_generated.godeps/github.com/openshift/kubernetes/staging/src/k8s.io/code-generator/cmd/client-gen/generators/fake/generator_fake_for_clientset.godeps/github.com/openshift/kubernetes/staging/src/k8s.io/code-generator/examples/HyphenGroup/clientset/versioned/fake/clientset_generated.godeps/github.com/openshift/kubernetes/staging/src/k8s.io/code-generator/examples/MixedCase/clientset/versioned/fake/clientset_generated.godeps/github.com/openshift/kubernetes/staging/src/k8s.io/code-generator/examples/apiserver/clientset/versioned/fake/clientset_generated.godeps/github.com/openshift/kubernetes/staging/src/k8s.io/code-generator/examples/crd/clientset/versioned/fake/clientset_generated.godeps/github.com/openshift/kubernetes/staging/src/k8s.io/code-generator/examples/single/clientset/versioned/fake/clientset_generated.godeps/github.com/openshift/kubernetes/staging/src/k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/fake/clientset_generated.godeps/github.com/openshift/kubernetes/staging/src/k8s.io/metrics/pkg/client/clientset/versioned/fake/clientset_generated.godeps/github.com/openshift/kubernetes/test/compatibility_lifecycle/reference/versioned_feature_list.yamldeps/github.com/openshift/kubernetes/test/e2e/node/pods.godeps/github.com/openshift/kubernetes/test/integration/dra/binding_conditions_test.goetcd/go.modgo.modpackaging/crio.conf.d/10-microshift_amd64.confpackaging/crio.conf.d/10-microshift_arm64.confscripts/auto-rebase/assets.yamlscripts/auto-rebase/changelog.txtscripts/auto-rebase/commits.txtscripts/auto-rebase/last_rebase.sh
💤 Files with no reviewable changes (4)
- deps/github.com/openshift/kubernetes/staging/src/k8s.io/metrics/pkg/client/clientset/versioned/fake/clientset_generated.go
- deps/github.com/openshift/kubernetes/build/dependencies.yaml
- deps/github.com/openshift/kubernetes/staging/src/k8s.io/code-generator/examples/apiserver/clientset/versioned/fake/clientset_generated.go
- deps/github.com/openshift/kubernetes/staging/src/k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/fake/clientset_generated.go
.../github.com/openshift/kubernetes/pkg/controller/devicetainteviction/device_taint_eviction.go
Show resolved
Hide resolved
|
/retest-required |
|
@eslutsky: 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. |
|
/lgtm |
|
@ggiguash: 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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eslutsky, ggiguash 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 |
No description provided.