TT-7086 - feat(helm): Add hostNetwork option

[rdcwaldrop1]

Description

Add the option to turn host networking on. With the host networking option on it is likely users will also want to control port selection so as not to collide with other pods running with hostNetwork mode enabled. To accomodate port selection the ports have been parameterized in the template as well.

Related Issue

Resolves #532 - TT-7086

Test Coverage For This Change

Screenshots (if appropriate)

Types of changes

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

Checklist

• Make sure you are requesting to pull a topic/feature/bugfix branch (right side). If PRing from your fork, don't come from your master!
• Make sure you are making a pull request against our master branch (left side). Also, it would be best if you started your change off our latest master.
• Make sure you are updating CHANGELOG.md based on your changes.
• My change requires a change to the documentation.
  • If you've changed APIs, describe what needs to be updated in the documentation.
• I have updated the documentation accordingly.
• If you've changed API models, please update CRDs.
  • make manifests
  • make helm
• I have added tests to cover my changes.
• All new and existing tests passed.
• Check your code additions will not fail linting checks:
  • gofmt -s -w .
  • go vet ./...
  • golangci-lint run

[buraksekili]

@rdcwaldrop1 thank you for this great contribution. It looks great but we need to change the approach for this problem a bit. Let me briefly talk about how we generate helm manifests.

helm/templates/all file is created automatically with make helm command. Therefore, direct updates on the helm/templates/all file will be overwritten by the next make helm command, which will eventually override your contribution.

What we do for creating a helm manifest is to run make helm command. This command runs kustomize on config/helm and sends the output of kustomize to hack/helm/pre_helm.go file which replaces certain fields with their templating correspondence.

For example, in order to update helm/templates/all, it'd be better if hack/helm/pre_helm.go is updated based on your desired changes.

Let me be clear, assume you want to add an option to the helm chart to configure healthProbeBindAddress of controller, as you did here. In order to do that, you can update config/manager/controller_manager_config.yaml's healthProbeBindAddress to a certain variable, something like CONTROLLER_MANAGER_HEALTH_PROBE_BINDING_ADDR. Then, in the hack/helm/pre_helm.go script, you can add the following field to the struct

		{"CONTROLLER_MANAGER_HEALTH_PROBE_BINDING_ADDR", healthProbe},

to replace occurrences of CONTROLLER_MANAGER_HEALTH_PROBE_BINDING_ADDR with healthProbe, which can be defined as:

const healthProbe = `:{{ .Values.healthProbePort }}`

in the same pre_helm.go file. So that the kustomize output will contain

apiVersion: v1
data:
  controller_manager_config.yaml: |
    apiVersion: controller-runtime.sigs.k8s.io/v1alpha1
    kind: ControllerManagerConfig
    health:
      healthProbeBindAddress: CONTROLLER_MANAGER_HEALTH_PROBE_BINDING_ADDR
#rest of the file ...

and pre_helm.go script will replace all occurrences of CONTROLLER_MANAGER_HEALTH_PROBE_BINDING_ADDR with healthProbe constant which is defined as above :{{ .Values.healthProbePort }}

I hope I am clear about how to approach the solution. Please let me know if you need further help. I am happy to help. Again, thank you for coming up with this PR 🙏

[rdcwaldrop1]

Sorry, for the slow turnaround on this one @buraksekili. I believe I have made the changes you suggested in order to render the all.yaml manifest with the make helm command. Please let me know if there is anything else I need to add to this PR. If it looks good I'll mark it as ready for review. Thanks!

[buraksekili]

no worries @rdcwaldrop1 :) you can mark the PR whenever you feel it is ready for review - that's fine for me. Also, I skim through your changes and I see lots of changes in helm/crds/crds.yaml and most of the changes seem to be related to formatting.
I guess we have differences in kustomize version. What is your kustomize and go version? We use

$ kustomize version
{Version:kustomize/v4.4.1 GitCommit:b2d65ddc98e09187a8e38adc27c30bab078c1dbf BuildDate:2021-11-11T23:27:14Z GoOs:darwin GoArch:arm64}
$ go version
go version go1.17.5 darwin/arm64

[rdcwaldrop1]

Cool, I'll mark as ready for review and try regenerating with a matching version of kustomize.

[rdcwaldrop1]

I've bumped my tool versions and still seem to be getting the same formatting issues. Would it be best to just remove helm/crds/crds.yaml from the commit?

[buraksekili]

Would it be best to just remove helm/crds/crds.yaml from the commit?

i appreciate if you can. sorry for confusion.

[buraksekili]

hi @rdcwaldrop1, do you need some guidance on the PR? I am more than happy to help if you need it.

[rdcwaldrop1]

hi @rdcwaldrop1, do you need some guidance on the PR? I am more than happy to help if you need it.

Hi! In speaking with our solutions engineer he mentioned that this was something that was fixed in the next release. Maybe I misunderstood. If this is still something needed by the project I'm happy to finish this out.

[buraksekili]

hi again @rdcwaldrop1 , sorry for the back and forth. We, as a team, want to enable this feature on Tyk Operator.
Since you have already developed it, It'd be great if you can update the PR and we can go with your contribution. If you have no availability, please let me know. So that I can pick your work and continue working on it. Again, thank you for your contribution 🙏

[rdcwaldrop1]

hi again @rdcwaldrop1 , sorry for the back and forth. We, as a team, want to enable this feature on Tyk Operator. Since you have already developed it, It'd be great if you can update the PR and we can go with your contribution. If you have no availability, please let me know. So that I can pick your work and continue working on it. Again, thank you for your contribution 🙏

Thats fantastic! I can absolutely finish this out. Glad to have the opportunity. I'll make the updates mentioned above today.

[rdcwaldrop1]

Would it be best to just remove helm/crds/crds.yaml from the commit?

i appreciate if you can. sorry for confusion.

This file has been removed

[buraksekili]

LGTM!

[buraksekili]

thank you for your contribution @rdcwaldrop1! we really appreciate your time and effort.

Your changes will be available in v0.14.0 :)

[rdcwaldrop1]

thank you for your contribution @rdcwaldrop1! we really appreciate your time and effort.

Your changes will be available in v0.14.0 :)

Thanks! It was great working with you. I look forward to contributing more in the future.