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

Affinity rules enable by default (why is not disabled?) #21

Open
GRomR1 opened this issue Jul 31, 2022 · 4 comments
Open

Affinity rules enable by default (why is not disabled?) #21

GRomR1 opened this issue Jul 31, 2022 · 4 comments
Assignees
Labels
help wanted Extra attention is needed question Further information is requested
Milestone

Comments

@GRomR1
Copy link
Contributor

GRomR1 commented Jul 31, 2022

Is it possible don't add any affinity rules by default? I think is should be an option to enable them. By default they should be disabled. Am I right?

How to reproduce?

Create some values file

# ./samples/whoami.yml
deployments:
  whoami:
    containers:
    - name: whoami
      image: containous/whoami
      imageTag: v1.5.0

Run generate k8s manifests with this value file

helm template whoami --values ./samples/whoami.yml ./ > whoami.yaml

The generated result was here.

# Source: universal-chart/templates/deployment.yml
apiVersion: apps/v1
kind: Deployment
metadata:
  name: whoami-whoami
  namespace: default
  labels:
    app.kubernetes.io/name: whoami
    app.kubernetes.io/instance: whoami
    helm.sh/chart: universal-chart-2.1.0
    app.kubernetes.io/managed-by: Helm
  annotations:    
spec:
  replicas: 1
  selector:
    matchLabels:
      app.kubernetes.io/name: whoami
      app.kubernetes.io/instance: whoami
  template:
    metadata:
      labels:
        app.kubernetes.io/name: whoami
        app.kubernetes.io/instance: whoami
      annotations:
    spec:
      affinity:       ### < ---- i think this section should not be here by default, if is not explicitly defined
        nodeAffinity:
          {}
        podAffinity:
          preferredDuringSchedulingIgnoredDuringExecution:
          - podAffinityTerm:
              labelSelector:
                matchLabels:
                  app.kubernetes.io/name: whoami
                  app.kubernetes.io/instance: whoami
              namespaces:
              - "default"
              topologyKey: kubernetes.io/hostname
            weight: 1
        podAntiAffinity:
          preferredDuringSchedulingIgnoredDuringExecution:
          - podAffinityTerm:
              labelSelector:
                matchLabels:
                  app.kubernetes.io/name: whoami
                  app.kubernetes.io/instance: whoami
              namespaces:
              - "default"
              topologyKey: kubernetes.io/hostname
            weight: 1
      
      containers:
      - name: whoami
        image: containous/whoami:v1.5.0
        imagePullPolicy: IfNotPresent                
        volumeMounts:
        
          []
      volumes:      
        []
@GRomR1
Copy link
Contributor Author

GRomR1 commented Jul 31, 2022

I would like this code will be like here

      {{- if .affinity }}
      affinity: {{- include "helpers.tplvalues.render" ( dict "value" .affinity "context" $) | nindent 8 }}
      {{- else }}
      {{- if $general.enableAffinity | default false }}
      affinity:
        nodeAffinity: {{- include "helpers.affinities.nodes" (dict "type" $.Values.nodeAffinityPreset.type "key" $.Values.nodeAffinityPreset.key "values" $.Values.nodeAffinityPreset.values) | nindent 10 }}
        podAffinity: {{- include "helpers.affinities.pods" (dict "type" $.Values.podAffinityPreset "context" $) | nindent 10 }}
        podAntiAffinity: {{- include "helpers.affinities.pods" (dict "type" $.Values.podAntiAffinityPreset "context" $) | nindent 10 }}
      {{- end }}
      {{- end }}

Then values.yaml need to add this parameters

deploymentsGeneral:
  enableAffinity: true # or false

@randreev1321 randreev1321 added help wanted Extra attention is needed question Further information is requested labels Aug 1, 2022
@randreev1321 randreev1321 added this to the v2.2 milestone Aug 1, 2022
@randreev1321
Copy link
Collaborator

Hi @GRomR1.

Thanks for your issue. This is good point. We'll work on this idea in the next release.

To disable the creation of affinities now, set the following values in your values file:

podAffinityPreset: nil
podAntiAffinityPreset: nil

As a result, you will get an empty affinity block:

      ...
      affinity:
        nodeAffinity:
          
        podAffinity:
          
        podAntiAffinity:
      ...

@GRomR1
Copy link
Contributor Author

GRomR1 commented Aug 1, 2022

@randreev1321 many thanks!

Btw you create a great product, I wonder that no one made this before.

@randreev1321 randreev1321 modified the milestones: v2.2, v3.0 Feb 20, 2023
@randreev1321
Copy link
Collaborator

Hi, @GRomR1.

Thanks for provided Pull Requests, its great.

I made a bit updates:

  1. enableAffinity option for workloads general renamed to usePredefinedAffinity, this option still have false value by default.
  2. added usePredefinedAffinity option to generic section, with true value by default.

I made this because name usePredefinedAffinity more accurately captures the point of the variable. Also, because changing of default behavior requires update the major version, but I want to give users time and ability to prepare, and I want to have more updates in version 3.0.

However, you now have the opportunity to manage this behavior. Although the option is set to true by default at generic level, you can override this value at the workloads genereals level.

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

Successfully merging a pull request may close this issue.

2 participants