-
Notifications
You must be signed in to change notification settings - Fork 386
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
🌱 Skip upsynced resources in resource scheduling #2545
🌱 Skip upsynced resources in resource scheduling #2545
Conversation
73b6c1d
to
9b60665
Compare
@@ -47,6 +47,11 @@ func (c *Controller) reconcileResource(ctx context.Context, lclusterName logical | |||
logger := logging.WithObject(logging.WithReconciler(klog.Background(), ControllerName), obj).WithValues("groupVersionResource", gvr.String(), "logicalCluster", lclusterName.String()) | |||
logger.V(4).Info("reconciling resource") | |||
|
|||
if isUpSynced(obj.GetLabels()) { | |||
logger.Info("resource is in Upsync mode; ignoring") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
verbosity at least 3, probably more like 4.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
@@ -486,6 +486,17 @@ func (c *Controller) enqueueSyncTargetKey(syncTargetKey string) { | |||
} | |||
} | |||
|
|||
// isUpSynced returns true if the labels of the resource contain at least | |||
// one `ResourceState` label with the `Upsync` value. | |||
func isUpSynced(labels map[string]string) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as a special helper only used in one stop, please move it into the other file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
9b60665
to
a58e4ae
Compare
Signed-off-by: David Festal <dfestal@redhat.com>
a58e4ae
to
e7d5874
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sttts The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Summary
Resources upsynced from a downstream cluster, and thus having the
Upsync
label should not be taken in account by resource scheduling.Related issue(s)
No issue dedicated. Spotted while testing PR #2338