Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NET-11297] Purge on disable #4378

Merged
merged 11 commits into from
Oct 21, 2024
Merged

[NET-11297] Purge on disable #4378

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
2cc3fe3
purge services on disable of sync catalog
jm96441n Oct 1, 2024
ca3df7c
gofumpt
jm96441n Oct 1, 2024
0ecfe35
update tests for sync catalog
jm96441n Oct 1, 2024
b667860
Added changelog
jm96441n Oct 10, 2024
0fd8e01
create separate jobs to handle deregistering services when disabling …
jm96441n Oct 10, 2024
9f491d4
Check error
jm96441n Oct 17, 2024
b1f29a1
Merge branch 'main' into purge-on-disable
jm96441n Oct 21, 2024
bde9f05
fix tests
jm96441n Oct 21, 2024
681eee1
rename field in values file, remove references to pod security policy
jm96441n Oct 21, 2024
8dd18f4
remove psp
jm96441n Oct 21, 2024
6a2b63d
added bats testing
jm96441n Oct 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/4378.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:enhancement
catalog-sync: Added field to helm chart to purge all services registered with catalog-sync from consul on disabling of catalog-sync.
```
123 changes: 123 additions & 0 deletions charts/consul/templates/sync-catalog-cleanup-on-uninstall-job.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
{{- if .Values.syncCatalog.purgeServicesOnDisable }}
apiVersion: batch/v1
kind: Job
metadata:
name: {{ template "consul.fullname" . }}-sync-catalog-cleanup-on-uninstall
namespace: {{ .Release.Namespace }}
labels:
app: {{ template "consul.name" . }}
chart: {{ template "consul.chart" . }}
heritage: {{ .Release.Service }}
release: {{ .Release.Name }}
component: sync-catalog-cleanup
{{- if .Values.global.extraLabels }}
{{- toYaml .Values.global.extraLabels | nindent 4 }}
{{- end }}
annotations:
"helm.sh/hook": pre-delete
"helm.sh/hook-weight": "0"
"helm.sh/hook-delete-policy": hook-succeeded,hook-failed
spec:
template:
metadata:
name: {{ template "consul.fullname" . }}-sync-catalog-cleanup-on-uninstall
labels:
app: {{ template "consul.name" . }}
chart: {{ template "consul.chart" . }}
release: {{ .Release.Name }}
component: sync-catalog-cleanup
{{- if .Values.global.extraLabels }}
{{- toYaml .Values.global.extraLabels | nindent 8 }}
{{- end }}
annotations:
"consul.hashicorp.com/connect-inject": "false"
"consul.hashicorp.com/mesh-inject": "false"
spec:
restartPolicy: Never
serviceAccountName: {{ template "consul.fullname" . }}-sync-catalog-cleanup
containers:
- name: sync-catalog-cleanup-job
image: "{{ default .Values.global.imageK8S .Values.syncCatalog.image }}"
{{ template "consul.imagePullPolicy" . }}
{{- include "consul.restrictedSecurityContext" . | nindent 8 }}
env:
{{- include "consul.consulK8sConsulServerEnvVars" . | nindent 8 }}
{{- if .Values.global.acls.manageSystemACLs }}
- name: CONSUL_LOGIN_AUTH_METHOD
{{- if and .Values.global.federation.enabled .Values.global.federation.primaryDatacenter .Values.global.enableConsulNamespaces }}
value: {{ template "consul.fullname" . }}-k8s-component-auth-method-{{ .Values.global.datacenter }}
{{- else }}
value: {{ template "consul.fullname" . }}-k8s-component-auth-method
{{- end }}
- name: CONSUL_LOGIN_DATACENTER
{{- if and .Values.global.federation.enabled .Values.global.federation.primaryDatacenter .Values.global.enableConsulNamespaces }}
value: {{ .Values.global.federation.primaryDatacenter }}
{{- else }}
value: {{ .Values.global.datacenter }}
{{- end }}
- name: CONSUL_LOGIN_META
value: "component=sync-catalog,pod=$(NAMESPACE)/$(POD_NAME)"
{{- end }}
- name: NAMESPACE
valueFrom:
fieldRef:
fieldPath: metadata.namespace
{{- if (and .Values.syncCatalog.aclSyncToken.secretName .Values.syncCatalog.aclSyncToken.secretKey) }}
- name: CONSUL_ACL_TOKEN
valueFrom:
secretKeyRef:
name: {{ .Values.syncCatalog.aclSyncToken.secretName }}
key: {{ .Values.syncCatalog.aclSyncToken.secretKey }}
{{- end }}
volumeMounts:
{{- if .Values.global.tls.enabled }}
{{- if not (or (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) .Values.global.secretsBackend.vault.enabled) }}
- name: consul-ca-cert
mountPath: /consul/tls/ca
readOnly: true
{{- end }}
{{- end }}
command:
- "/bin/sh"
- "-ec"
- |
exec consul-k8s-control-plane sync-catalog \
-log-level={{ default .Values.global.logLevel .Values.syncCatalog.logLevel }} \
-log-json={{ .Values.global.logJSON }} \
-k8s-default-sync={{ .Values.syncCatalog.default }} \
{{- if .Values.global.adminPartitions.enabled }}
-partition={{ .Values.global.adminPartitions.name }} \
{{- end }}
{{- if .Values.syncCatalog.metrics.enabled | default .Values.global.metrics.enabled }}
-enable-metrics \
{{- end }}
{{- if .Values.syncCatalog.metrics.path }}
-metrics-path={{ .Values.syncCatalog.metrics.path }} \
{{- end }}
{{- if .Values.syncCatalog.metrics.port }}
-metrics-port={{ .Values.syncCatalog.metrics.port }} \
{{- end }}
-prometheus-retention-time={{ .Values.global.metrics.agentMetricsRetentionTime }} \
{{- if .Release.IsUpgrade }}
jm96441n marked this conversation as resolved.
Show resolved Hide resolved
-purge-k8s-services-from-node
{{- end }}
{{- if .Values.global.acls.tolerations }}
tolerations:
{{ tpl .Values.global.acls.tolerations . | indent 8 | trim }}
{{- end }}
volumes:
{{- if .Values.global.tls.enabled }}
{{- if not (or (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) .Values.global.secretsBackend.vault.enabled) }}
- name: consul-ca-cert
secret:
{{- if .Values.global.tls.caCert.secretName }}
secretName: {{ .Values.global.tls.caCert.secretName }}
{{- else }}
secretName: {{ template "consul.fullname" . }}-ca-cert
{{- end }}
items:
- key: {{ default "tls.crt" .Values.global.tls.caCert.secretKey }}
path: tls.crt
{{- end }}
{{- end }}
{{- end }}
124 changes: 124 additions & 0 deletions charts/consul/templates/sync-catalog-cleanup-on-upgrade-job.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
{{- $syncCatalogEnabled := (or (and (ne (.Values.syncCatalog.enabled | toString) "-") .Values.syncCatalog.enabled) (and (eq (.Values.syncCatalog.enabled | toString) "-") .Values.global.enabled)) }}
{{- if and (not $syncCatalogEnabled) .Values.syncCatalog.purgeServicesOnDisable }}
apiVersion: batch/v1
kind: Job
metadata:
name: {{ template "consul.fullname" . }}-sync-catalog-cleanup-on-upgrade
namespace: {{ .Release.Namespace }}
labels:
app: {{ template "consul.name" . }}
chart: {{ template "consul.chart" . }}
heritage: {{ .Release.Service }}
release: {{ .Release.Name }}
component: sync-catalog-cleanup
{{- if .Values.global.extraLabels }}
{{- toYaml .Values.global.extraLabels | nindent 4 }}
{{- end }}
annotations:
"helm.sh/hook": pre-upgrade
"helm.sh/hook-weight": "0"
"helm.sh/hook-delete-policy": hook-succeeded,hook-failed
spec:
template:
metadata:
name: {{ template "consul.fullname" . }}-sync-catalog-cleanup-on-upgrade
labels:
app: {{ template "consul.name" . }}
chart: {{ template "consul.chart" . }}
release: {{ .Release.Name }}
component: sync-catalog-cleanup
{{- if .Values.global.extraLabels }}
{{- toYaml .Values.global.extraLabels | nindent 8 }}
{{- end }}
annotations:
"consul.hashicorp.com/connect-inject": "false"
"consul.hashicorp.com/mesh-inject": "false"
spec:
restartPolicy: Never
serviceAccountName: {{ template "consul.fullname" . }}-sync-catalog-cleanup
containers:
- name: sync-catalog-cleanup-job
image: "{{ default .Values.global.imageK8S .Values.syncCatalog.image }}"
{{ template "consul.imagePullPolicy" . }}
{{- include "consul.restrictedSecurityContext" . | nindent 8 }}
env:
{{- include "consul.consulK8sConsulServerEnvVars" . | nindent 8 }}
{{- if .Values.global.acls.manageSystemACLs }}
- name: CONSUL_LOGIN_AUTH_METHOD
{{- if and .Values.global.federation.enabled .Values.global.federation.primaryDatacenter .Values.global.enableConsulNamespaces }}
value: {{ template "consul.fullname" . }}-k8s-component-auth-method-{{ .Values.global.datacenter }}
{{- else }}
value: {{ template "consul.fullname" . }}-k8s-component-auth-method
{{- end }}
- name: CONSUL_LOGIN_DATACENTER
{{- if and .Values.global.federation.enabled .Values.global.federation.primaryDatacenter .Values.global.enableConsulNamespaces }}
value: {{ .Values.global.federation.primaryDatacenter }}
{{- else }}
value: {{ .Values.global.datacenter }}
{{- end }}
- name: CONSUL_LOGIN_META
value: "component=sync-catalog,pod=$(NAMESPACE)/$(POD_NAME)"
{{- end }}
- name: NAMESPACE
valueFrom:
fieldRef:
fieldPath: metadata.namespace
{{- if (and .Values.syncCatalog.aclSyncToken.secretName .Values.syncCatalog.aclSyncToken.secretKey) }}
- name: CONSUL_ACL_TOKEN
valueFrom:
secretKeyRef:
name: {{ .Values.syncCatalog.aclSyncToken.secretName }}
key: {{ .Values.syncCatalog.aclSyncToken.secretKey }}
{{- end }}
volumeMounts:
{{- if .Values.global.tls.enabled }}
{{- if not (or (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) .Values.global.secretsBackend.vault.enabled) }}
- name: consul-ca-cert
mountPath: /consul/tls/ca
readOnly: true
{{- end }}
{{- end }}
command:
- "/bin/sh"
- "-ec"
- |
exec consul-k8s-control-plane sync-catalog \
-log-level={{ default .Values.global.logLevel .Values.syncCatalog.logLevel }} \
-log-json={{ .Values.global.logJSON }} \
-k8s-default-sync={{ .Values.syncCatalog.default }} \
{{- if .Values.global.adminPartitions.enabled }}
-partition={{ .Values.global.adminPartitions.name }} \
{{- end }}
{{- if .Values.syncCatalog.metrics.enabled | default .Values.global.metrics.enabled }}
-enable-metrics \
{{- end }}
{{- if .Values.syncCatalog.metrics.path }}
-metrics-path={{ .Values.syncCatalog.metrics.path }} \
{{- end }}
{{- if .Values.syncCatalog.metrics.port }}
-metrics-port={{ .Values.syncCatalog.metrics.port }} \
{{- end }}
-prometheus-retention-time={{ .Values.global.metrics.agentMetricsRetentionTime }} \
{{- if .Release.IsUpgrade }}
jm96441n marked this conversation as resolved.
Show resolved Hide resolved
-purge-k8s-services-from-node
{{- end }}
{{- if .Values.global.acls.tolerations }}
tolerations:
{{ tpl .Values.global.acls.tolerations . | indent 8 | trim }}
{{- end }}
volumes:
{{- if .Values.global.tls.enabled }}
{{- if not (or (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots) .Values.global.secretsBackend.vault.enabled) }}
- name: consul-ca-cert
secret:
{{- if .Values.global.tls.caCert.secretName }}
secretName: {{ .Values.global.tls.caCert.secretName }}
{{- else }}
secretName: {{ template "consul.fullname" . }}-ca-cert
{{- end }}
items:
- key: {{ default "tls.crt" .Values.global.tls.caCert.secretKey }}
path: tls.crt
{{- end }}
{{- end }}
{{- end }}
32 changes: 32 additions & 0 deletions charts/consul/templates/sync-catalog-cleanup-podsecuritypolicy.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{{- if and .Values.syncCatalog.purgeServicesOnDisable .Values.global.enablePodSecurityPolicies }}
jm96441n marked this conversation as resolved.
Show resolved Hide resolved
apiVersion: policy/v1beta1
kind: PodSecurityPolicy
metadata:
name: {{ template "consul.fullname" . }}-sync-catalog-cleanup
namespace: {{ .Release.Namespace }}
labels:
app: {{ template "consul.name" . }}
chart: {{ template "consul.chart" . }}
heritage: {{ .Release.Service }}
release: {{ .Release.Name }}
component: sync-catalog-cleanup
spec:
privileged: false
allowPrivilegeEscalation: false
# This is redundant with non-root + disallow privilege escalation,
# but we can provide it for defense in depth.
requiredDropCapabilities:
- ALL
hostNetwork: false
hostIPC: false
hostPID: false
runAsUser:
rule: 'RunAsAny'
seLinux:
rule: 'RunAsAny'
supplementalGroups:
rule: 'RunAsAny'
fsGroup:
rule: 'RunAsAny'
readOnlyRootFilesystem: false
{{- end }}
19 changes: 19 additions & 0 deletions charts/consul/templates/sync-catalog-cleanup-serviceaccount.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{{- if .Values.syncCatalog.purgeServicesOnDisable }}
apiVersion: v1
kind: ServiceAccount
metadata:
name: {{ template "consul.fullname" . }}-sync-catalog-cleanup
namespace: {{ .Release.Namespace }}
labels:
app: {{ template "consul.name" . }}
chart: {{ template "consul.chart" . }}
heritage: {{ .Release.Service }}
release: {{ .Release.Name }}
component: sync-catalog-cleanup
{{- with .Values.global.imagePullSecrets }}
imagePullSecrets:
{{- range . }}
- name: {{ .name }}
{{- end }}
{{- end }}
{{- end }}
3 changes: 2 additions & 1 deletion charts/consul/templates/sync-catalog-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{{- if (or (and (ne (.Values.syncCatalog.enabled | toString) "-") .Values.syncCatalog.enabled) (and (eq (.Values.syncCatalog.enabled | toString) "-") .Values.global.enabled)) }}
{{- $syncCatalogEnabled := (or (and (ne (.Values.syncCatalog.enabled | toString) "-") .Values.syncCatalog.enabled) (and (eq (.Values.syncCatalog.enabled | toString) "-") .Values.global.enabled)) }}
{{- if $syncCatalogEnabled }}
{{- template "consul.reservedNamesFailer" (list .Values.syncCatalog.consulNamespaces.consulDestinationNamespace "syncCatalog.consulNamespaces.consulDestinationNamespace") }}
Copy link
Member

Choose a reason for hiding this comment

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

❤️

{{ template "consul.validateRequiredCloudSecretsExist" . }}
{{ template "consul.validateCloudSecretKeys" . }}
Expand Down
4 changes: 4 additions & 0 deletions charts/consul/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2090,6 +2090,10 @@ syncCatalog:
# global.enabled.
enabled: false

# True if you want to deregister all services in this cluster from consul that have been registered by the catalog sync when the `enabled` flag is set to false.
# @type: boolean
purgeServicesOnDisable: false
Copy link
Member Author

Choose a reason for hiding this comment

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

this needs a better name

Copy link
Member

Choose a reason for hiding this comment

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

Also, as we discussed before, do we need to add more fields here to allow users to control the purging in more granular ways?

Copy link
Member Author

Choose a reason for hiding this comment

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

we don't, it's not really a requirement and since we're doing this on a disable of the catalog-sync we don't want to potentially leave behind services that might no longer exist. The main requirement is "remove services registered by catalog sync when catalog sync is disabled"

Copy link
Member Author

Choose a reason for hiding this comment

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

this still needs a better name

Copy link
Member

Choose a reason for hiding this comment

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

Just brain dumping other possibilities:

  • cleanUpServicesOnDisable
  • cleanUpOnDisable (might be my vote thus far 🤷‍♂️ )
  • deregisterServicesOnDisable
  • deregisterOnDisable

It feels like the name needs to be some version of ..OnDisable because putting disable as a prefix makes it seem like you are disabling something additional.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah I don't like the prefix disable I was thinking maybe cleanupNodeOnRemoval since the Disable can be a bit of misnomer because this runs on uninstall of consul and when you disable syncCatalog whereas the removal can mean "removal of sync catalog" or "removal of consul"


# The name of the Docker image (including any tag) for consul-k8s-control-plane
# to run the sync program.
# @type: string
Expand Down
Loading
Loading