syncCatalog: expose add-k8s-namespace-suffix flag #280
Conversation
values.yaml
Outdated
| # Namespace suffix is not added if 'annotationServiceName' is provided. | ||
| # NOTE: Setting this property to 'true' will result in deregistering of existing | ||
| # synced services in Consul and registering them with a new name. | ||
| addK8SNamespaceSuffix: false |
There was a problem hiding this comment.
I think we should set this to true by default. The current default has the major problem that a service with the same name in any namespace will be synced to the same Consul service.
We'd need to document the breaking change and tell people how to upgrade safely (set this value to false) but I think it's worth it to not put new users in a bad state.
There was a problem hiding this comment.
@lkysow After thinking about this more, I agree that this should be true by default and that it's worth making this a breaking change.
| @@ -94,6 +94,9 @@ spec: | |||
| {{- if .Values.syncCatalog.consulPrefix}} | |||
| -consul-service-prefix="{{ .Values.syncCatalog.consulPrefix}}" \ | |||
| {{- end}} | |||
| {{- if .Values.syncCatalog.addK8SNamespaceSuffix}} | |||
| -add-k8s-namespace-suffix="{{ .Values.syncCatalog.addK8SNamespaceSuffix}}" \ | |||
There was a problem hiding this comment.
This is a bool flag so can this be
{{- if .Values.syncCatalog.addK8SNamespaceSuffix}}
-add-k8s-namespace-suffix \
values.yaml
Outdated
| # Set this flag to true to avoid registering services with the same name | ||
| # but in different namespaces as instances for the same Consul service. | ||
| # Namespace suffix is not added if 'annotationServiceName' is provided. | ||
| # NOTE: Setting this property to 'true' will result in deregistering of existing |
There was a problem hiding this comment.
This is only a concern when someone is updating an existing Consul installation. This comment reads to me like it might happen if someone sets it to true the first time they install.
Also, given that we're now defaulting this to true, the wording is a little odd.
There was a problem hiding this comment.
True. I removed that note entirely. This comment will be more appropriate in the Changelog.
Co-Authored-By: Rebecca Zanzig <[email protected]>
Co-Authored-By: Rebecca Zanzig <[email protected]>
Depends on hashicorp/consul-k8s#160.