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

k6.spec.separate should append podAntiAffinity to Affinity and not overwrite it #97

Open
jdheyburn opened this issue Jan 31, 2022 · 2 comments
Labels
evaluation needed help wanted Extra attention is needed UX

Comments

@jdheyburn
Copy link

jdheyburn commented Jan 31, 2022

Hello

I've been using k6.spec.separate = true to define a podAntiAffinity with other k6 runners, however I'd like to add my own affinity using k6.spec.runner.affinity, but this logic in the operator overwrites it:

if k6.Spec.Separate {
    job.Spec.Template.Spec.Affinity = newAntiAffinity()
}

Because of this, I have to set k8.spec.separate = false. While I can replicate the antiAffinity in my custom affinity block, it would be better UX to have it appended to a podAntiAffinity if there is one defined already.

Thanks
Joe

@yorugac
Copy link
Collaborator

yorugac commented Feb 3, 2022

Hi @jdheyburn thanks for the issue!

I must ask what do you mean by

appended to a podAntiAffinity

?
The way separate is defined now is the "hard" anti-affinity requirement RequiredDuringSchedulingIgnoredDuringExecution:

func newAntiAffinity() *corev1.Affinity {
return &corev1.Affinity{
PodAntiAffinity: &corev1.PodAntiAffinity{
RequiredDuringSchedulingIgnoredDuringExecution: []corev1.PodAffinityTerm{
{
LabelSelector: &metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: "app",
Operator: "In",
Values: []string{
"k6",
},
},
},
},
TopologyKey: "kubernetes.io/hostname",
},
},
},
}
}

But if one uses the override with manually set podAntiAffinity like you described, how to reliably add additional conditions there? Meaning, there is more than one way to do that (like hard vs soft, what if this label is already there, etc.) which implies we'll need more than one configuration option just to be clear on what exactly should be done. That seems like quite an overhead 🤔

The way I see it, separate is a shortcut option for quick anti-affinity in setups that don't care about anything more complex. For other setups, podAffinity and podAntiAffinity provide as much flexibility as possible. So these two ways to configure are kind of opposites.

I definitely understand the desire to improve UX here but I don't yet see how to combine it in a way that will:

  1. not over-complicate configuration
  2. be straight-forward to implement

Suggestions are welcome 🙂

@yorugac yorugac added evaluation needed help wanted Extra attention is needed UX labels Feb 3, 2022
@yorugac
Copy link
Collaborator

yorugac commented Jan 26, 2023

Potential solution: switch to topology spread constraints instead of anti-affinity in implementation of separate option. Then anti-affinity can be passed as usual, without overwrites.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
evaluation needed help wanted Extra attention is needed UX
Projects
None yet
Development

No branches or pull requests

2 participants