Skip to content

🐛 fix work generation from received cloudevents. - #175

Merged
openshift-merge-bot[bot] merged 1 commit into
open-cluster-management-io:mainfrom
morvencao:br_fix_work_agent_generation
Dec 9, 2025
Merged

🐛 fix work generation from received cloudevents.#175
openshift-merge-bot[bot] merged 1 commit into
open-cluster-management-io:mainfrom
morvencao:br_fix_work_agent_generation

Conversation

@morvencao

@morvencao morvencao commented Dec 9, 2025

Copy link
Copy Markdown
Member

Summary

This PR attempts to fix the work generation decoded from received cloudevents, now the work generation will be always set from cloudevents resourceversion extension.

Related issue(s)

Fixes #

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced generation field handling in manifest bundle processing to ensure consistent behavior.

✏️ Tip: You can customize this high-level summary in your review settings.

Signed-off-by: Morven Cao <lcao@redhat.com>
@openshift-ci
openshift-ci Bot requested review from deads2k and qiujian16 December 9, 2025 02:51
@coderabbitai

coderabbitai Bot commented Dec 9, 2025

Copy link
Copy Markdown

Walkthrough

This PR modifies the manifest bundle codec to always assign the Generation field in the Decode function instead of conditionally assigning it only when zero. Additionally, a blank line is added to the import block for formatting consistency.

Changes

Cohort / File(s) Summary
Manifest bundle codec update
pkg/cloudevents/clients/work/agent/codec/manifestbundle.go
Added blank line in import block after fmt import. Changed Decode function to always set metaObj.Generation to int64(resourceVersion), removing the previous conditional assignment that only set it when zero.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • The logic change should be reviewed to verify that always assigning Generation is the intended behavior and doesn't unintentionally override previously preserved Generation values.

Possibly related PRs

Suggested labels

approved, lgtm

Suggested reviewers

  • qiujian16
  • deads2k

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly indicates a bug fix for work generation from CloudEvents, which matches the main change in the code that fixes how metaObj.Generation is set.
Description check ✅ Passed The PR description includes a summary explaining the fix and follows the template structure, though the 'Related issue(s)' section is incomplete with only 'Fixes #' placeholder.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c3b3f06 and d1bc05b.

📒 Files selected for processing (1)
  • pkg/cloudevents/clients/work/agent/codec/manifestbundle.go (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: integration
  • GitHub Check: unit
  • GitHub Check: verify
🔇 Additional comments (2)
pkg/cloudevents/clients/work/agent/codec/manifestbundle.go (2)

3-8: Import block blank line is style-only and acceptable

The extra blank line in the import block just affects grouping/formatting and has no behavioral impact. It’s fine to keep as-is or let goimports normalize it according to your repo’s conventions.


144-147: Unconditionally deriving Generation from CloudEvent resourceVersion matches the intended semantics

Always setting metaObj.Generation = int64(resourceVersion) ensures the decoded ManifestWork generation is consistently derived from the CloudEvent’s resourceVersion, instead of sometimes reusing a potentially stale value from the serialized ObjectMeta. This aligns with the comment that the event resourceVersion semantically tracks spec changes and should drive generation, and it avoids mismatches between the work meta extension and the event’s sequence.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@qiujian16

Copy link
Copy Markdown
Member

/approve
/lgtm

@openshift-ci

openshift-ci Bot commented Dec 9, 2025

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: morvencao, qiujian16

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved label Dec 9, 2025
@openshift-merge-bot
openshift-merge-bot Bot merged commit 62521c9 into open-cluster-management-io:main Dec 9, 2025
12 checks passed
@morvencao
morvencao deleted the br_fix_work_agent_generation branch December 9, 2025 03:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants