Skip to content
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

[Feature] After configuring TabletPool Affinity, the Zone configuration will become invalid #649

Open
chhan-coupang opened this issue Dec 24, 2024 · 5 comments

Comments

@chhan-coupang
Copy link

Our Vitess will configure the following affinity, but this will cause the Zone to no longer take effect, causing the zone to not be as expected.

          tabletPools:
          - affinity:
              nodeAffinity:
                requiredDuringSchedulingIgnoredDuringExecution:
                  nodeSelectorTerms:
                  - matchExpressions:
                    - key: alpha.eksctl.io/nodegroup-name
                      operator: In
                      values:
                      - vitess-20
            cell: ap2d

The relevant code implementation is that after specifying any affinity, the default rule will no longer take effect.
Can we open nodeSelector or other methods/parameters to make custom affinity and zone configuration take effect at the same time?

if spec.Zone != "" {
// Limit to a specific zone.
obj.Spec.Affinity.NodeAffinity = &corev1.NodeAffinity{
RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{
NodeSelectorTerms: []corev1.NodeSelectorTerm{
{
MatchExpressions: []corev1.NodeSelectorRequirement{
{
Key: k8s.ZoneFailureDomainLabel,
Operator: corev1.NodeSelectorOpIn,
Values: []string{spec.Zone},
},
},
},
},
},
}
}
}

Reference: #604

@chhan-coupang
Copy link
Author

vttablet can be replaced by the following method, but be careful to manually set different zones.
Others like vtgate/vtctld/vtorc cannot be configured in this way because they do not distinguish zones on the vitescluster object.

   affinity:
     nodeAffinity:
       requiredDuringSchedulingIgnoredDuringExecution:
         nodeSelectorTerms:
         - matchExpressions:
           - key: alpha.eksctl.io/nodegroup-name
             operator: In
             values:
             - vitess-test-bcd
           - key: topology.kubernetes.io/zone
             operator: In
             values:
             - ap-northeast-2b

@mattlord
Copy link
Collaborator

@chhan-coupang I'm not sure what this issue is (it's not clearly reflected in the title, labels, or description). Is this a bug report? If so, it's unclear to me (no test case and I don't know what the results are). Is this a feature request (for #604)? Something else?

Thanks!

@mattlord
Copy link
Collaborator

I wonder if in your case this or similar would address it? The current input and expected/desired output is not clear so I cannot say:

diff --git a/pkg/operator/vttablet/pod.go b/pkg/operator/vttablet/pod.go
index 55a0f37..dc98b18 100644
--- a/pkg/operator/vttablet/pod.go
+++ b/pkg/operator/vttablet/pod.go
@@ -359,20 +359,34 @@ func UpdatePod(obj *corev1.Pod, spec *Spec) {
 		}
 		if spec.Zone != "" {
 			// Limit to a specific zone.
-			obj.Spec.Affinity.NodeAffinity = &corev1.NodeAffinity{
-				RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{
-					NodeSelectorTerms: []corev1.NodeSelectorTerm{
-						{
-							MatchExpressions: []corev1.NodeSelectorRequirement{
-								{
-									Key:      k8s.ZoneFailureDomainLabel,
-									Operator: corev1.NodeSelectorOpIn,
-									Values:   []string{spec.Zone},
+			zoneNodeSelector := corev1.NodeSelectorTerm{
+				MatchExpressions: []corev1.NodeSelectorRequirement{
+					{
+						Key:      k8s.ZoneFailureDomainLabel,
+						Operator: corev1.NodeSelectorOpIn,
+						Values:   []string{spec.Zone},
+					},
+				},
+			}
+			if obj.Spec.Affinity.NodeAffinity == nil || obj.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution == nil {
+				obj.Spec.Affinity.NodeAffinity = &corev1.NodeAffinity{
+					RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{
+						NodeSelectorTerms: []corev1.NodeSelectorTerm{
+							{
+								MatchExpressions: []corev1.NodeSelectorRequirement{
+									{
+										Key:      k8s.ZoneFailureDomainLabel,
+										Operator: corev1.NodeSelectorOpIn,
+										Values:   []string{spec.Zone},
+									},
 								},
 							},
 						},
 					},
-				},
+				}
+			} else {
+				obj.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms = append(obj.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms,
+					zoneNodeSelector)
 			}
 		}
 	}

@chhan-coupang
Copy link
Author

Thank you for your reply!
I want to automatically merge the custom Affinity with the zone rules.
Instead of having to choose one or the other.
The alternative I use can only solve the problem of vttablet, but cannot solve other components (vtgate, vtctld, vtorc)

@chhan-coupang
Copy link
Author

This is a feature request. There are two suggested implementations:

  1. Open nodeSelector to customize node labels so that it can take effect at the same time as zone.
  2. Merge zone configuration and customized Affinity so that they can take effect at the same time.

@chhan-coupang chhan-coupang changed the title After configuring TabletPool Affinity, the Zone configuration will become invalid [Feature] After configuring TabletPool Affinity, the Zone configuration will become invalid Dec 27, 2024
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

No branches or pull requests

2 participants