Skip to content

Conversation

@ryanzhang-oss
Copy link
Contributor

@ryanzhang-oss ryanzhang-oss commented Aug 23, 2022

Description of your changes

  • Add the pending condition with a more suitable message
  • Reduce the event emission
  • Enhance the IT to include work status change
  • Fix a flaky e2e test
  • Fix one bug on handling work resources not found

Fixes #

I have:

  • Run make reviewable to ensure this PR is ready for review.

How has this code been tested

IT

Special notes for your reviewer

This is to fix #239

Copy link
Contributor

@Fei-Guo Fei-Guo left a comment

Choose a reason for hiding this comment

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

One Nit

case errors.Is(applyErr, ErrStillPendingManifest):
if appliedCond == nil || appliedCond.Status != metav1.ConditionUnknown {
klog.V(2).InfoS("Some selected resources are still waiting to be applied", "placement", placementRef)
r.Recorder.Event(placement, corev1.EventTypeWarning, "ResourceApplyPending", "Some selected resources are still waiting to be applied to the member cluster")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this necessary to send event for pending status?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how about appliedCond.Status == metav1.ConditionTrue? Which means it was applied but somehow now pending?

Copy link
Contributor

Choose a reason for hiding this comment

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

how about appliedCond.Status == metav1.ConditionTrue? Which means it was applied but somehow now pending?

That is ok. You may also need to rephrase the event msg to indicate something unusual happened.

@Fei-Guo Fei-Guo merged commit cd3bccb into Azure:main Aug 23, 2022
weng271190436 pushed a commit to weng271190436/fleet that referenced this pull request Nov 3, 2025
@weng271190436 weng271190436 mentioned this pull request Nov 3, 2025
1 task
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.

[BUG] The status message for a pending propagation is misleading

2 participants