feat: refactor cmpd's role definition #8416
Conversation
cjc7373
force-pushed
the
feature/refactor-role
branch
from
b18c91c
to
deaf5f1
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8416 +/- ##
==========================================
- Coverage 61.22% 61.15% -0.07%
==========================================
Files 351 351
Lines 41854 41828 -26
==========================================
- Hits 25624 25581 -43
- Misses 13906 13921 +15
- Partials 2324 2326 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
cjc7373
requested review from
free6om,
a team,
sophon-zt,
wangyelei,
leon-inf and
Y-Rookie
as code owners
cjc7373
force-pushed
the
feature/refactor-role
branch
from
752aefa
to
90b8f22
Compare
| // | ||
| // +kubebuilder:default=0 | ||
| // +optional | ||
| UpdatePriority int `json:"updatePriority"` |
| // | ||
| // +kubebuilder:default=false | ||
| // +optional | ||
| SwitchoverBeforeUpdate bool `json:"switchoverBeforeUpdate"` |
There was a problem hiding this comment.
maybe SwitchoverBeforeMaintenance or NeedSwitchover
There was a problem hiding this comment.
How about remove this option and decide within the switchover action whether a switchover is necessary based on the role of target pod?
| // | ||
| // +kubebuilder:default=false | ||
| // +optional | ||
| ParticipatesInQuorum bool `json:"participatesInQuorum"` |
cjc7373
force-pushed
the
feature/refactor-role
branch
from
98bfd03
to
b0e9c70
Compare
cjc7373
force-pushed
the
feature/refactor-role
branch
from
ea9ddbd
to
12ffec8
Compare
| @@ -232,7 +232,16 @@ func (r *RestoreManager) DoPostReady(comp *component.SynthesizedComponent, | |||
| backupObj *dpv1alpha1.Backup) error { | |||
| jobActionLabels := constant.GetCompLabels(r.Cluster.Name, comp.Name) | |||
| if len(comp.Roles) > 0 { | |||
| jobActionLabels[instanceset.AccessModeLabelKey] = string(appsv1alpha1.ReadWrite) | |||
| // HACK: assume the role with highest priority to be writable | |||
There was a problem hiding this comment.
It's better to let the data-protection define a policy to select target pods explicitly.
|
|
||
| // getTargetRole returns the role on which the switchover is performed | ||
| // FIXME: the assumption that only one role supports switchover may change in the future | ||
| func getTargetRoleName(roles []appsv1.ReplicaRole) (string, error) { |
There was a problem hiding this comment.
We don't need to infer the target pods, just let the user specify the specific roles or pods.
| return "", fmt.Errorf("more than one role defined as leader: %s,%s", targetRole, role.Name) | ||
| } | ||
| targetRole = role.Name | ||
| // HACK: assume the role with highest priority to be leader |
There was a problem hiding this comment.
All of the legacy codes can be removed.
| @@ -291,6 +293,9 @@ type InstanceSetStatus struct { | |||
|
|
|||
| // Indicates whether it is required for the InstanceSet to have at least one primary instance ready. | |||
| // | |||
| // Deprecated: since instanceset no longer checks a "primary" role when doing ready check, this | |||
There was a problem hiding this comment.
Remove the field directly.
fixes #8377