Remove crd-install hook, as it break CRD updates

This is a problem that Helm is going to solve going forward, but for now if you use the crd-install hook, then you can only install CRDs, and not update them at any point during a chart lifecycle. Also, prior to Helm 2.12, if you installed chart with a crd-install hook that did not have one previously, it deleted the CRDs. Therefore, removing the crd-install hook, so that CRDs are again managed by the Helm charts. Added a `agones.crd.install` parameter, in case someone wants to subchart this chart, then can set this to false, and copy the Agones CRDs into their own charts to be included in the right place for their chart lifecycle. Also, since we have a `agones.crd` config section, moved `agones.enableHelmCleanupHooks` into `agones.crds.cleanupOnDelete` Unfortunately, with this back and forth on the crd-install hook, if you are using the Helm chart, you will need to do a full Agones `helm delete --purge` and cleanup any remaining CRDs to upgrade. More context on helm + crds: - helm/helm#4697 - istio/istio#9604 - istio/istio#7688 - helm/community#64 - helm/helm#4863 - helm/helm#4709
googleforgames · Dec 9, 2018 · 938e23e · 938e23e
1 parent 6efbe0b
commit 938e23e
Show file tree

Hide file tree

Showing 14 changed files with 30 additions and 122 deletions.
diff --git a/build/Makefile b/build/Makefile
@@ -175,13 +175,15 @@ push: push-controller-image push-agones-sdk-image push-ping-image
 install: ALWAYS_PULL_SIDECAR := true
 install: IMAGE_PULL_POLICY := "Always"
 install: PING_SERVICE_TYPE := "LoadBalancer"
+install: CRD_CLEANUP := true
 install: $(ensure-build-image) install-custom-pull-secret
 	$(DOCKER_RUN) \
 		helm upgrade --install --wait --namespace=agones-system\
 		--set agones.image.tag=$(VERSION),agones.image.registry=$(REGISTRY) \
 		--set agones.image.controller.pullPolicy=$(IMAGE_PULL_POLICY),agones.image.sdk.alwaysPull=$(ALWAYS_PULL_SIDECAR) \
 		--set agones.image.controller.pullSecret=$(IMAGE_PULL_SECRET) \
 		--set agones.ping.http.serviceType=$(PING_SERVICE_TYPE),agones.ping.udp.serviceType=$(PING_SERVICE_TYPE) \
+		--set agones.crds.cleanupOnDelete=$(CRD_CLEANUP) \
 		agones $(mount_path)/install/helm/agones/
 
 uninstall: $(ensure-build-image)
@@ -251,7 +253,8 @@ push-agones-sdk-image: $(ensure-build-image)
 gen-install: $(ensure-build-image)
 	docker run --rm $(common_mounts) $(DOCKER_RUN_ARGS) $(build_tag) bash -c \
 		'helm template --name=agones-manual --namespace agones-system $(mount_path)/install/helm/agones \
-		--set agones.controller.generateTLS=false --set agones.enableHelmCleanupHooks=false \
+		--set agones.controller.generateTLS=false \
+		--set agones.crds.cleanupOnDelete=false \
 		> $(mount_path)/install/yaml/install.yaml'
 
 # Generate the SDK gRPC server and client code

diff --git a/install/helm/README.md b/install/helm/README.md
@@ -84,6 +84,8 @@ The following tables lists the configurable parameters of the Agones chart and t
 | Parameter                                           | Description                                                                                     | Default                |
 | --------------------------------------------------- | ----------------------------------------------------------------------------------------------- | ---------------------- |
 | `agones.rbacEnabled`                                | Creates RBAC resources. Must be set for any cluster configured with RBAC                        | `true`                 |
+| `agones.crds.install`                               | Install the CRDs with this chart. Useful to disable if you want to subchart (since crd-install hook is broken), so you can copy the CRDs into your own chart. | `true` |
+| `agones.crds.cleanupOnDelete`                       | Run the pre-delete hook to delete all GameServers and their backing Pods when deleting the helm chart, so that all CRDs can be removed on chart deletion | `true`          |  
 | `agones.serviceaccount.controller`                  | Service account name for the controller                                                         | `agones-controller`    |
 | `agones.serviceaccount.sdk`                         | Service account name for the sdk                                                                | `agones-sdk`           |
 | `agones.image.registry`                             | Global image registry for all images                                                            | `gcr.io/agones-images` |

diff --git a/install/helm/agones/scripts/delete_crds.sh b/install/helm/agones/scripts/delete_crds.sh
diff --git a/install/helm/agones/templates/crds/fleet.yaml b/install/helm/agones/templates/crds/fleet.yaml
@@ -12,6 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+{{- if .Values.agones.crds.install }}
+
 apiVersion: apiextensions.k8s.io/v1beta1
 kind: CustomResourceDefinition
 metadata:
@@ -22,8 +24,6 @@ metadata:
     chart: {{ template "agones.chart" . }}
     release: {{ .Release.Name }}
     heritage: {{ .Release.Service }}
-  annotations:
-    "helm.sh/hook": "crd-install"
 spec:
   group: stable.agones.dev
   version: v1alpha1
@@ -59,3 +59,5 @@ spec:
                     - RollingUpdate
             template:
               {{- include "gameserver.validation" . | indent 14 }}
+
+{{- end }}
diff --git a/install/helm/agones/templates/crds/fleetallocation.yaml b/install/helm/agones/templates/crds/fleetallocation.yaml
@@ -12,6 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+{{- if .Values.agones.crds.install }}
+
 apiVersion: apiextensions.k8s.io/v1beta1
 kind: CustomResourceDefinition
 metadata:
@@ -22,8 +24,6 @@ metadata:
     chart: {{ template "agones.chart" . }}
     release: {{ .Release.Name }}
     heritage: {{ .Release.Service }}
-  annotations:
-    "helm.sh/hook": "crd-install"
 spec:
   group: stable.agones.dev
   version: v1alpha1
@@ -46,3 +46,5 @@ spec:
               minLength: 1
               maxLength: 63
               pattern: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?$"
+
+{{- end }}
diff --git a/install/helm/agones/templates/crds/fleetautoscaler.yaml b/install/helm/agones/templates/crds/fleetautoscaler.yaml
@@ -12,6 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+{{- if .Values.agones.crds.install }}
+
 apiVersion: apiextensions.k8s.io/v1beta1
 kind: CustomResourceDefinition
 metadata:
@@ -22,8 +24,6 @@ metadata:
     chart: {{ template "agones.chart" . }}
     release: {{ .Release.Name }}
     heritage: {{ .Release.Service }}
-  annotations:
-    "helm.sh/hook": "crd-install"
 spec:
   group: stable.agones.dev
   version: v1alpha1
@@ -65,3 +65,4 @@ spec:
                     maxReplicas:
                       type: integer
                       minimum: 1
+{{- end }}
diff --git a/install/helm/agones/templates/crds/gameserver.yaml b/install/helm/agones/templates/crds/gameserver.yaml
@@ -12,6 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+{{- if .Values.agones.crds.install }}
+
 apiVersion: apiextensions.k8s.io/v1beta1
 kind: CustomResourceDefinition
 metadata:
@@ -22,8 +24,6 @@ metadata:
     chart: {{ template "agones.chart" . }}
     release: {{ .Release.Name }}
     heritage: {{ .Release.Service }}
-  annotations:
-    "helm.sh/hook": "crd-install"
 spec:
   group: stable.agones.dev
   version: v1alpha1
@@ -37,3 +37,5 @@ spec:
   validation:
     openAPIV3Schema:
       {{- include "gameserver.validation" . | indent 6 }}
+
+{{- end }}
diff --git a/install/helm/agones/templates/crds/gameserverset.yaml b/install/helm/agones/templates/crds/gameserverset.yaml
@@ -12,6 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+{{- if .Values.agones.crds.install }}
+
 apiVersion: apiextensions.k8s.io/v1beta1
 kind: CustomResourceDefinition
 metadata:
@@ -22,8 +24,6 @@ metadata:
     chart: {{ template "agones.chart" . }}
     release: {{ .Release.Name }}
     heritage: {{ .Release.Service }}
-  annotations:
-    "helm.sh/hook": "crd-install"
 spec:
   group: stable.agones.dev
   version: v1alpha1
@@ -53,3 +53,4 @@ spec:
               - Distributed
             template:
               {{- include "gameserver.validation" . | indent 14 }}
+{{- end }}
diff --git a/install/helm/agones/templates/hooks/post_delete_hook.yaml b/install/helm/agones/templates/hooks/post_delete_hook.yaml
diff --git a/install/helm/agones/templates/hooks/pre_delete_hook.yaml b/install/helm/agones/templates/hooks/pre_delete_hook.yaml
@@ -1,4 +1,4 @@
-{{- if .Values.agones.enableHelmCleanupHooks }}
+{{- if .Values.agones.crds.cleanupOnDelete }}
 # Copyright 2018 Google Inc. All Rights Reserved.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");

diff --git a/install/helm/agones/templates/hooks/sa.yaml b/install/helm/agones/templates/hooks/sa.yaml
@@ -1,4 +1,4 @@
-{{- if .Values.agones.enableHelmCleanupHooks }}
+{{- if .Values.agones.crds.cleanupOnDelete }}
 # Copyright 2018 Google Inc. All Rights Reserved.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -44,9 +44,6 @@ metadata:
     "helm.sh/hook-weight": "-5"
     "helm.sh/hook-delete-policy": before-hook-creation
 rules:
-- apiGroups: ["apiextensions.k8s.io"]
-  resources: ["customresourcedefinitions"]
-  verbs: ["delete"]
 - apiGroups: ["stable.agones.dev"]
   resources: ["fleets", "fleetallocations", "fleetautoscalers", "gameservers", "gameserversets"]
   verbs: ["delete", "list" ]

diff --git a/install/helm/agones/templates/hooks/scripts.yaml b/install/helm/agones/templates/hooks/scripts.yaml
@@ -1,4 +1,4 @@
-{{- if .Values.agones.enableHelmCleanupHooks }}
+{{- if .Values.agones.crds.cleanupOnDelete }}
 # Copyright 2018 Google Inc. All Rights Reserved.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -13,22 +13,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-apiVersion: v1
-kind: ConfigMap
-metadata:
-  name: delete-crds
-  labels:
-    app.kubernetes.io/managed-by: {{.Release.Service | quote }}
-    app.kubernetes.io/instance: {{.Release.Name | quote }}
-    helm.sh/chart: "{{.Chart.Name}}-{{.Chart.Version}}"
-  annotations:
-    "helm.sh/hook": pre-delete
-    "helm.sh/hook-weight": "-3"
-    "helm.sh/hook-delete-policy": before-hook-creation
-data:
-  delete_crds.sh: |
-{{ .Files.Get "scripts/delete_crds.sh" | indent 4 }}
----
 apiVersion: v1
 kind: ConfigMap
 metadata:

diff --git a/install/helm/agones/values.yaml b/install/helm/agones/values.yaml
@@ -16,7 +16,9 @@
 
 agones:
   rbacEnabled: true
-  enableHelmCleanupHooks: true
+  crds:
+    install: true
+    cleanupOnDelete: true
   serviceaccount:
     controller: agones-controller
     sdk: agones-sdk

diff --git a/install/yaml/install.yaml b/install/yaml/install.yaml
@@ -163,8 +163,6 @@ metadata:
     chart: agones-0.7.0-rc
     release: agones-manual
     heritage: Tiller
-  annotations:
-    "helm.sh/hook": "crd-install"
 spec:
   group: stable.agones.dev
   version: v1alpha1
@@ -303,7 +301,6 @@ spec:
                           type: integer
                           minimum: 1
                           maximum: 2147483648
-
 ---
 # Source: agones/templates/crds/fleetallocation.yaml
 # Copyright 2018 Google Inc. All Rights Reserved.
@@ -330,8 +327,6 @@ metadata:
     chart: agones-0.7.0-rc
     release: agones-manual
     heritage: Tiller
-  annotations:
-    "helm.sh/hook": "crd-install"
 spec:
   group: stable.agones.dev
   version: v1alpha1
@@ -354,7 +349,6 @@ spec:
               minLength: 1
               maxLength: 63
               pattern: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?$"
-
 ---
 # Source: agones/templates/crds/fleetautoscaler.yaml
 # Copyright 2018 Google Inc. All Rights Reserved.
@@ -381,8 +375,6 @@ metadata:
     chart: agones-0.7.0-rc
     release: agones-manual
     heritage: Tiller
-  annotations:
-    "helm.sh/hook": "crd-install"
 spec:
   group: stable.agones.dev
   version: v1alpha1
@@ -424,7 +416,6 @@ spec:
                     maxReplicas:
                       type: integer
                       minimum: 1
-
 ---
 # Source: agones/templates/crds/gameserver.yaml
 # Copyright 2018 Google Inc. All Rights Reserved.
@@ -451,8 +442,6 @@ metadata:
     chart: agones-0.7.0-rc
     release: agones-manual
     heritage: Tiller
-  annotations:
-    "helm.sh/hook": "crd-install"
 spec:
   group: stable.agones.dev
   version: v1alpha1
@@ -596,8 +585,6 @@ metadata:
     chart: agones-0.7.0-rc
     release: agones-manual
     heritage: Tiller
-  annotations:
-    "helm.sh/hook": "crd-install"
 spec:
   group: stable.agones.dev
   version: v1alpha1
@@ -730,7 +717,6 @@ spec:
                           type: integer
                           minimum: 1
                           maximum: 2147483648
-
 ---
 # Source: agones/templates/service.yaml
 # Copyright 2018 Google Inc. All Rights Reserved.
@@ -949,10 +935,6 @@ spec:
       targetPort: 8080
       protocol: UDP
   type: LoadBalancer
----
-# Source: agones/templates/hooks/post_delete_hook.yaml
-
-
 ---
 # Source: agones/templates/hooks/pre_delete_hook.yaml