Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Adds nodeSelector to deployment templates for mixed OS clusters #2692

Merged
merged 3 commits into from
Dec 17, 2019

Conversation

rparsonsbb
Copy link
Contributor

Adds an os nodeSelector to the two deployments required for the eksctl deployment of flux to work correctly.

@2opremio
Copy link
Contributor

2opremio commented Dec 17, 2019

@rparsonsbb thanks for the contribution 🌷

Can you please:

  1. Rebase on top of master
  2. Run make generate-deploy and commit the result
  3. Add the same nodeSelector change to chart/flux/templates/deployment.yaml

Thanks!

@rparsonsbb
Copy link
Contributor Author

Sure thing!

@2opremio
Copy link
Contributor

2opremio commented Dec 17, 2019

@rparsonsbb please clean up the history of your branch so that the PR only includes your commits (and no merge commits).

@rparsonsbb
Copy link
Contributor Author

Done!

@2opremio 2opremio added this to the 1.18.0 milestone Dec 17, 2019
@2opremio
Copy link
Contributor

@rparsonsbb Great. Did you test that it works when using extra nodeSelector and memcached.nodeSelector values?

@rparsonsbb
Copy link
Contributor Author

Yes, I generated templates and installed in our dev mixed os cluster with no additional nodeSelector values in the values.yaml to ensure that both flux and memcached deployments function and to make sure that the nodeSelector object was created properly then added additional values (kubernetes.io/hostname, kubernetes.io/arch) to validate adding more values.

Below works in long or short yaml notation

No additional values

/chart/flux/values.yaml

nodeSelector: {}
memcached.nodeSelector: {}

❯helm template ./ | grep -C 5 'nodeSelector'
...
          - --registry-trace=false
          resources:
            requests:
              cpu: 50m
              memory: 64Mi
      nodeSelector: 
        beta.kubernetes.io/os: linux
---
# Source: flux/templates/memcached.yaml
apiVersion: apps/v1
kind: Deployment
--
          {}
        securityContext:
          allowPrivilegeEscalation: false
          runAsGroup: 11211
          runAsUser: 11211
      nodeSelector:
        beta.kubernetes.io/os: linux

One additonal value

/chart/flux/values.yaml

nodeSelector: {kubernetes.io/host: bb-dev-01-linux-ng-1}
memcached.nodeSelector: {kubernetes.io/host: bb-dev-01-linux-ng-2}

❯helm template ./ | grep -C 5 'nodeSelector'
...
          - --registry-trace=false
          resources:
            requests:
              cpu: 50m
              memory: 64Mi
      nodeSelector: 
        beta.kubernetes.io/os: linux
        kubernetes.io/host: bb-dev-01-linux-ng-1
---
# Source: flux/templates/memcached.yaml
apiVersion: apps/v1
--
          {}
        securityContext:
          allowPrivilegeEscalation: false
          runAsGroup: 11211
          runAsUser: 11211
      nodeSelector:
        beta.kubernetes.io/os: linux
        kubernetes.io/host: bb-dev-01-linux-ng-2

Multiple additonal values

/chart/flux/values.yaml

nodeSelector: {kubernetes.io/host: bb-dev-01-linux-ng-1, disk: ssd}
memcached.nodeSelector: {kubernetes.io/host: bb-dev-01-linux-ng-2, disk: ssd}

❯helm template ./ | grep -C 5 'nodeSelector'
...
          - --registry-trace=false
          resources:
            requests:
              cpu: 50m
              memory: 64Mi
      nodeSelector: 
        beta.kubernetes.io/os: linux
        kubernetes.io/host: bb-dev-01-linux-ng-1
        disk: ssd
---
# Source: flux/templates/memcached.yaml
apiVersion: apps/v1
--
          {}
        securityContext:
          allowPrivilegeEscalation: false
          runAsGroup: 11211
          runAsUser: 11211
      nodeSelector:
        beta.kubernetes.io/os: linux
        kubernetes.io/host: bb-dev-01-linux-ng-2
        disk: ssd

@2opremio
Copy link
Contributor

Great, thanks!

@2opremio 2opremio merged commit 1a8d1aa into fluxcd:master Dec 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants