Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

Fix: coalesce.go warning while overriding the client affinity with a string value #854

Merged
merged 1 commit into from
Mar 9, 2021

Conversation

washtubs
Copy link
Contributor

@washtubs washtubs commented Mar 9, 2021

Create a local-values.yaml like the example shown in the values.yaml in the docs above client affinity:

client:
  affinity: |
    nodeAffinity:
      requiredDuringSchedulingIgnoredDuringExecution:
        nodeSelectorTerms:
        - matchExpressions:
          - key: node-role.kubernetes.io/master
            operator: DoesNotExist

If you run helm template -f ./local-values.yaml consul-test . | head, it will print a warning like so:

coalesce.go:196: warning: cannot overwrite table with non table for affinity (map[])

The end result is fine, but the warning is annoying. My understanding is when it's "coalescing" your local-values.yaml with the values.yaml it doesn't like it when a string is replacing an object.

Changes proposed in this PR:

  • Default client affinity to null instead of an empty map prevents the coalesce.go warning from being printed

How I've tested this PR:

How I expect reviewers to test this PR:

Checklist:

  • Bats tests added
  • CHANGELOG entry added (HashiCorp engineers only, community PRs should not add a changelog entry)

@hashicorp-cla
Copy link

hashicorp-cla commented Mar 9, 2021

CLA assistant check
All committers have signed the CLA.

@ndhanushkodi
Copy link
Contributor

@washtubs Can you sign the CLA? The change looks good to me.

Copy link
Contributor

@ndhanushkodi ndhanushkodi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good to me! On L754 we do the same with nodeSelector, where it's type is string and we default it to null, and seems reasonable to do the same here since it's not a map.

@ndhanushkodi ndhanushkodi requested review from a team, lkysow and ishustava and removed request for a team and ishustava March 9, 2021 21:22
@washtubs
Copy link
Contributor Author

washtubs commented Mar 9, 2021

Done!

@ndhanushkodi ndhanushkodi merged commit 0532464 into hashicorp:master Mar 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants