search

cloudnative-pg / charts

CloudNativePG Helm Charts
Apache License 2.0
174 stars 82 forks source link

chore: add support for `hostNetwork` in chart #324

Closed marckhair closed 2 months ago

marckhair commented 3 months ago

When running in eks, the deployment is failing because of the webhook with the following error:

Internal error occurred: failed calling webhook "mcluster.cnpg.io": failed to call webhook: Address is not allowed

The fix would be to set hostNetwork true but it is not configurable in the chart.

In addition to that, the dnsPolicy needs to be set to ClusterFirstWithHostNet to give hostNetwork pods access to internal DNS.

mboutet commented 3 months ago

So it seems like https://github.com/cloudnative-pg/charts/pull/185 also partially addresses the same issue with running cloudnative-pg on EKS with a custom CNI. However, that other PR misses the dnsPolicy field.

That other PR has been opened for months without any attention being given to it from the maintainers. For such a simple change that would help a lot of folks, I'd expect maintainers to accept and merge this PR without excessive delays. I totally understand the open source aspect of such a project, but again, those two PRs can easily be reviewed in a few minutes and would bring a lot of value to some people.

Thank you for building cloudnative-pg, it is an incredibly useful operator!

marckhair commented 3 months ago

@gbartolini @leonardoce @mnencia @phisco @sxd We would greatly appreciate it if anyone could take a quick look at this PR

itay-grudev commented 3 months ago

@mboutet The problem is a lack of test suite, for both helm charts. We've made mistakes in the past, merging bugs. And the time requirements for merging with manual testing are increased.

I tried KUTTL a while ago, but mostly had issues with it. @phisco suggested I try Chainsaw instead, but before we have something like that merging will be slow.

I know it's 15 lines of code, but we've done damage with less.

marckhair commented 3 months ago

Tested default value | non default value combinations using helm template locally.

itay-grudev commented 3 months ago

Tested default value | non default value combinations using helm template locally.

The problem is not if they will render, but whether they will be accepted by K8s. I think a --dry-run is more appropriate if you don't want to install.

marckhair commented 3 months ago

Tested default value | non default value combinations using helm template locally.

The problem is not if they will render, but whether they will be accepted by K8s. I think a --dry-run is more appropriate if you don't want to install.

I passed the output of helm template to kubeconform in strict mode. Dry run is too complicated, it requires a cluster.