Add nodeSelector and tolerations to hooks

[klamas1]

Description

Added nodeSelector and tolerations to hooks

Type of change

• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

How has this been tested?

helm upgrade --dry-run

Checklist

• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works

[guidoiaquinti]

Thank you for your contribution! Can you please add a test?

[klamas1]

I'm not sure I understand what is required.
The basic validity tests for migrate-job.yaml are run using the manifest
https://github.com/klamas1/charts-clickhouse/blob/main/charts/posthog/tests/migrate-job.yaml

[guidoiaquinti]

I'm not sure I understand what is required. The basic validity tests for migrate-job.yaml are run using the manifest https://github.com/klamas1/charts-clickhouse/blob/main/charts/posthog/tests/migrate-job.yaml

Sure, can we also test that by adding hooks.nodeSelector we end up with the template we expect? Thank you!

[klamas1]

I ran
helm upgrade posthog charts-clickhouse/charts/posthog --install --namespace services -f values/posthog/values.yaml --dry-run
With the following settings in values/posthog/values.yaml

migrate:
  enabled: true

hooks:
  tolerations:
    - key: dedicated
      operator: Equal
      value: analytics
      effect: NoExecute
  affinity:
    nodeAffinity:
      requiredDuringSchedulingIgnoredDuringExecution:
        nodeSelectorTerms:
          - matchExpressions:
              - key: node.deckhouse.io/group
                operator: In
                values:
                  - analytics
  nodeSelector:
    kubernetes.io/arch: amd64

I received the following valid manifest.
With a closer look, I realize that everything is all right :)

<...>
---
# Source: posthog/templates/migrate.job.yaml
apiVersion: batch/v1
kind: Job
metadata:
  name: posthog-migrate-2022-08-30-15-15-19
  labels:

    "app.kubernetes.io/name": "posthog"
    "app.kubernetes.io/instance": "posthog"
    "app.kubernetes.io/managed-by": "Helm"
    "helm.sh/chart": "posthog-26.6.0"
  annotations:

    "meta.helm.sh/release-name": "posthog"
    "meta.helm.sh/release-namespace": "services"
spec:
  template:
    metadata:
      name: posthog-migrate-2022-08-30-15-15-19
      annotations:
        checksum/secrets.yaml: 5676d787df5a31c3207525803a3eec3adb6a9d0a1bcc2fc4531c50950da1989c
      labels:
        app: posthog
        release: "posthog"
    spec:
      affinity:
        nodeAffinity:
          requiredDuringSchedulingIgnoredDuringExecution:
            nodeSelectorTerms:
            - matchExpressions:
              - key: node.deckhouse.io/group
                operator: In
                values:
                - analytics
      nodeSelector:
        kubernetes.io/arch: amd64
      tolerations:
        - effect: NoExecute
          key: dedicated
          operator: Equal
          value: analytics
      restartPolicy: Never
      containers:
<...>

[guidoiaquinti]

I ran helm upgrade posthog charts-clickhouse/charts/posthog --install --namespace services -f values/posthog/values.yaml --dry-run With the following settings in values/posthog/values.yaml

[...]

I received the following valid manifest. With a closer look, I realize that everything is all right :)

That's great to hear but unit tests are useful to validate this in code and make sure there are no unexpected regressions in future changes, etc.

[klamas1]

I don't understand what you're asking me to do %(

[klamas1]

The tests in this PR did not run. The message is

5 workflows awaiting approval
First-time contributors need a maintainer to approve running workflows. [Learn more.](https://docs.github.com/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks)

[guidoiaquinti]

I don't understand what you're asking me to do %(

I'm asking if you can add a unit test to validate your changes to add hooks.nodeSelector and hooks.tolerations. See as example this test for ClickHouse.

Let me know if something is not clear. Thank you! 🙇

[klamas1]

I did it

[klamas1]

The e2e test is broken(

[guidoiaquinti]

The e2e test is broken(

Yes, it's due to an upstream issue k3s-io/k3s#6052

[guidoiaquinti]

LGTM once CI is ✅. Thank you for your contribution! 🎉

[klamas1]

@guidoiaquinti, my commit was not included in the new tag 26.6.1, will they be included in the next one?

[guidoiaquinti]

Apologies but our CI bot had an issue publishing the release. I've triggered a new run that published 26.6.2. I've double checked and it contains your contribution.