Skip to content

Conversation

@Emyrk
Copy link
Member

@Emyrk Emyrk commented Jul 29, 2021

What this does

We need this to get Kind to work for 1.21. It allows setting static nodeports

@Emyrk Emyrk marked this pull request as ready for review July 30, 2021 14:10
@Emyrk Emyrk requested review from jawnsy and kylecarbs July 30, 2021 14:10
@Emyrk Emyrk requested a review from kylecarbs August 2, 2021 19:36
# coderd.serviceNodePorts -- Allows manually setting static node ports for the coderd service.
# This is only helpful if static ports are required, and usually should be left alone.
# By default these are dynamically chosen.
serviceNodePorts:
Copy link

Choose a reason for hiding this comment

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

By adding these to the values file, these will be the default values applicable to all installs - is that what we want? What's the default behavior (e.g. if nodePort isn't specified)?

Copy link
Member Author

Choose a reason for hiding this comment

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

The values.yaml is the default?
The original default is that the nodePorts are selected dynamically. I thought values.yaml was an example config for generating the docs.

Copy link

Choose a reason for hiding this comment

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

the values file is the defaults that will be used if you don't specify any that override them, it's included in the Helm chart package (so if you do: helm install . coder then it'll use the values.yaml file unmodified). otherwise, if you specify --set, then it'll merge the default values file with the args you pass

Copy link
Member Author

Choose a reason for hiding this comment

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

TIL. So we can't document a field without explicitly setting a default?
I wonder if I can set it to null?

I prefer the default to be unset.

Copy link
Member Author

Choose a reason for hiding this comment

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

I could also make the value "0" effectively be "unset"

Copy link

Choose a reason for hiding this comment

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

Yeah I think null should work

Copy link

Choose a reason for hiding this comment

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

You can test by running make lint/kubernetes, this will generate a bunch of YAML files somewhere that you can examine

Copy link
Member Author

Choose a reason for hiding this comment

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

null works

@Emyrk Emyrk merged commit 349c105 into main Aug 3, 2021
@Emyrk Emyrk deleted the stevenmasley/kind_nodeport branch August 3, 2021 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants