add status check when diff workload#15
Merged
cheyang merged 1 commit intosgl-project:mainfrom Sep 4, 2025
Merged
Conversation
5107c86 to
04844bc
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds status checking functionality when diffing workloads, specifically checking replicas and readyReplicas status fields to trigger reconciliation when these values change. This enables the controller to respond to status changes in LeaderWorkerSet, Deployment, and StatefulSet workloads.
- Adds a
checkStatusparameter to workload comparison functions to enable status checking - Updates workload equality checks to include status comparison when called from the controller predicate
- Enhances e2e test framework to verify RoleBasedGroup status readiness
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/reconciler/workload_reconciler.go | Updates workload equality calls to include status checking |
| pkg/reconciler/deploy_reconciler.go | Adds checkStatus parameter to deployment comparison function |
| pkg/reconciler/sts_reconciler.go | Adds checkStatus parameter to StatefulSet comparison function |
| pkg/reconciler/lws_reconciler.go | Adds checkStatus parameter to LeaderWorkerSet comparison function |
| internal/controller/workloads/rolebasedgroup_controller.go | Simplifies workload predicate logic |
| test/e2e/framework/rbg_expect.go | Adds RBG status readiness check to e2e tests |
| test/e2e/framework/rbgs_expect.go | Improves RBG set validation logic |
| pkg/discovery/injector_test.go | Updates test environment variable value |
| go.mod | Updates dependency versions |
| examples/basics/rbg-with-lws-workload.yaml | Adds example RBG with LeaderWorkerSet workload |
Comments suppressed due to low confidence (1)
go.mod:1
- Go version 1.24.1 does not exist. The latest Go version is 1.23.x. This should be updated to a valid Go version.
module sigs.k8s.io/rbgs
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
04844bc to
be077a7
Compare
cheyang
reviewed
Sep 4, 2025
be077a7 to
6e4df8d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ⅰ. Motivation
if lws/deploy/sts status.replicas or status.readyReplicas changed, enqueue item to reconcile
Ⅱ. Modifications
add status check when diff workload
Ⅳ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.
e2e tests add rbg.status ready check
Ⅴ. Describe how to verify it
e2e tests
VI. Special notes for reviews
add status check when diff workload
Checklist
make fmt.