Sync TiDB service in control-loop #1242
Conversation
Signed-off-by: Aylei <[email protected]>
|
/run-e2e-in-kind |
| @@ -122,7 +122,7 @@ func (tmm *tidbMemberManager) syncTiDBHeadlessServiceForTidbCluster(tc *v1alpha1 | |||
| if !equal { | |||
| svc := *oldSvc | |||
| svc.Spec = newSvc.Spec | |||
| err = SetServiceLastAppliedConfigAnnotation(newSvc) | |||
| oldSvc := oldSvcTmp.DeepCopy() | ||
|
|
||
| // Adopt orphaned service by simply overriding | ||
| if metav1.GetControllerOf(oldSvc) == nil { |
There was a problem hiding this comment.
Adopting orphaned objects is a systematic problem that should be considered at project level in a separate issue, this PR adopts the TiDB service only.
| if err != nil { | ||
| return err | ||
| } | ||
| annoEqual := isSubMapOf(newSvc.Annotations, oldSvc.Annotations) |
There was a problem hiding this comment.
Annotation directly added by user will be retained.
|
/run-e2e-in-kind |
| Spec: corev1.ServiceSpec{ | ||
| Type: svcSpec.Type, | ||
| Ports: ports, | ||
| ExternalTrafficPolicy: svcSpec.ExternalTrafficPolicy, |
There was a problem hiding this comment.
For api server, this should have validation and defaulting.
| @@ -93,7 +93,11 @@ func (tmm *tidbMemberManager) Sync(tc *v1alpha1.TidbCluster) error { | |||
| } | |||
|
|
|||
| // Sync Tidb StatefulSet | |||
| return tmm.syncTiDBStatefulSetForTidbCluster(tc) | |||
| if err := tmm.syncTiDBStatefulSetForTidbCluster(tc); err != nil { | |||
| return err | |||
There was a problem hiding this comment.
I'm a bit worried that if something goes wrong with sts synchronization, then the service will not be synchronized too.
There was a problem hiding this comment.
Yes, such dangers are everywhere in today's tidb-controller-manager because of synchronized reconciling, this should be a systematic improvement that I'd like to collect efforts in a dedicated ticket
Signed-off-by: Aylei <[email protected]>
|
/run-e2e-in-kind |
1 similar comment
|
/run-e2e-in-kind |
Co-Authored-By: weekface <[email protected]>
Co-Authored-By: weekface <[email protected]>
Signed-off-by: Aylei <[email protected]>
|
/run-e2e-in-kind |
1 similar comment
|
/run-e2e-in-kind |
Signed-off-by: Aylei <[email protected]>
|
/run-e2e-in-kind |
|
/run-e2e-in-kind |
1 similar comment
|
/run-e2e-in-kind |
|
/run-e2e-test |
|
/run-e2e-test |
Signed-off-by: Aylei [email protected]
What problem does this PR solve?
close #1190
What is changed and how does it work?
Add syncTiDBService in tidb_member_manager
Check List
Tests
Does this PR introduce a user-facing change?: