Skip to content

add status check when diff workload#15

Merged
cheyang merged 1 commit intosgl-project:mainfrom
gujingit:bugfix/rbg-status
Sep 4, 2025
Merged

add status check when diff workload#15
cheyang merged 1 commit intosgl-project:mainfrom
gujingit:bugfix/rbg-status

Conversation

@gujingit
Copy link
Copy Markdown
Collaborator

@gujingit gujingit commented Sep 3, 2025

Ⅰ. 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

  • Format your code make fmt.
  • Add unit tests or integration tests.
  • Update the documentation related to the change.

@gujingit gujingit force-pushed the bugfix/rbg-status branch 2 times, most recently from 5107c86 to 04844bc Compare September 3, 2025 11:38
@gujingit gujingit changed the title [WIP] add status check when diff workload add status check when diff workload Sep 3, 2025
@gujingit gujingit requested a review from Copilot September 3, 2025 11:47
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 checkStatus parameter 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.

Comment thread go.mod
Comment thread test/e2e/framework/rbg_expect.go
Comment thread examples/basics/rbg-with-lws-workload.yaml Outdated
Copy link
Copy Markdown
Collaborator

@cheyang cheyang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

@cheyang cheyang merged commit 25713e8 into sgl-project:main Sep 4, 2025
3 checks passed
@gujingit gujingit deleted the bugfix/rbg-status branch September 23, 2025 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants