feat: When a resource is created for the first time, it should also be considred as a modification. #4583
Conversation
karmada-bot
added
the
kind/feature
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
karmada-bot
added
the
size/M
|
/cc @RainbowMango @chaosi-zju PTAL. |
What do you mean |
First time creating to k8s |
…e considered as a modification. Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
chaunceyjiang
force-pushed
the
activationPreference
branch
from
af5550a
to
0028f79
Compare
karmada-bot
added
size/S
| @@ -69,7 +69,7 @@ func (d *ResourceDetector) propagateResource(object *unstructured.Unstructured, | |||
| } | |||
| d.RemoveWaiting(objectKey) | |||
| metrics.ObserveFindMatchedPolicyLatency(start) | |||
| return d.ApplyPolicy(object, objectKey, resourceChangeByKarmada, propagationPolicy) | |||
| return d.ApplyPolicy(object, objectKey, false, propagationPolicy) | |||
There was a problem hiding this comment.
My core idea is: Whether PP is Lazy or immediate, it should be immediate when the resources have not yet been bound by PP.
There was a problem hiding this comment.
I believe Lazy should only take effect after the PP is bound with resources.
There was a problem hiding this comment.
does this PR want to resolve the problem #4577 (comment) you memtioned here?
There was a problem hiding this comment.
does this PR want to resolve the problem #4577 (comment) you memtioned here?
No, these are two different issues.
-
The issue in Introduced a lazy activation preference to PropagationPolicy/ClusterPropagationPolicy #4577 (comment) is that using a single PP to select two resources with different effects.
-
This PR wants to introduce "Lazy should only take effect after the PP is bound with resources."
There was a problem hiding this comment.
Assuming that a resource has been distributed according to PolicyA.
Then PolicyA deleted, or changed resourceSelector and no longer match to the resource.
The detecor will remove the policy label in resource immediately.
This will also triggle resoure updated.
In this case, resouce will look for a new policy (PolicyB), if PolicyB is Lazy, the binding should be unchanged.
The related pods of the reosource running just fine, we shouldn't break it just because policy change.
There was a problem hiding this comment.
In this case, resouce will look for a new policy, if this policy is Lazy, the binding should be unchanged.
Good question, your question introduces a new thought, whether the binding of PP and resources is Lazy or Immediate.
There was a problem hiding this comment.
Lazy should only take effect after the PP is bound with resources.
This principle is what I want too, and I couldn't agree more.
However, there is an ambiguity in our understanding of buond. I understand bound to mean that a resource is bound when it is labelled with a policy, whereas you think that a resource is bound when it is refreshed with a binding following a policy.
There was a problem hiding this comment.
your question introduces a new thought, whether the binding of PP and resources is Lazy or Immediate.
So the core of my proposal is The impact of Policy changes on the bondage relationship between resources and Policy is always immediate, but whether the resource needs to synchronize the latest placement to the binding immediately is determined by this field.
Pay attention to the bondage relationship between resources and Policy is always immediate, when a resource is labelled with a policy, just finish label, it finished the bound.
after bound, there's only ever one definitive policy for this resource, if this policy is lazy, don't change the resource's binding.
This would be the clearest behaviour.
There was a problem hiding this comment.
Pay attention to the bondage relationship between resources and Policy is always immediate, when a resource is labelled with a policy, just finish label, it finished the bound.
I got you.
However, most resources, once created, will not be changed.
As i said:
then resources like GatewayClass, IPPool can never be propagated to member clusters. Because once they are created, they will never be modified.
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #4583 +/- ##
==========================================
+ Coverage 51.69% 51.71% +0.01%
==========================================
Files 247 247
Lines 24418 24418
==========================================
+ Hits 12624 12627 +3
+ Misses 11106 11104 -2
+ Partials 688 687 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
After discussions in the community, it is currently considered appropriate to introduce a new type of Lazy. |
What type of PR is this?
/kind feature
What this PR does / why we need it:
If the first creation is not considered as a modification, then resources like GatewayClass, IPPool can never be propagated to member clusters. Because once they are created, they will never be modified.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: