Do not harcode namespace #40
Conversation
| @@ -15,7 +15,7 @@ webhooks: | |||
| - name: deployment.admission.stash.appscode.com | |||
| clientConfig: | |||
| service: | |||
| namespace: default | |||
| namespace: {{ .Release.Namespace }} | |||
There was a problem hiding this comment.
All changes made in this file are incorrect. The webhook server is registered as an extended apiserver and only accessed through the default/kubernetes service.
There was a problem hiding this comment.
ok, will revert
| @@ -15,7 +15,7 @@ webhooks: | |||
| - name: restic.admission.stash.appscode.com | |||
| clientConfig: | |||
| service: | |||
| namespace: default | |||
| namespace: {{ .Release.Namespace }} | |||
There was a problem hiding this comment.
All changes made in this file are incorrect. The webhook server is registered as an extended apiserver and only accessed through the default/kubernetes service.
There was a problem hiding this comment.
ok, will revert
| @@ -19,9 +19,9 @@ spec: | |||
| metadata: | |||
| labels: | |||
| {{- include "stash.labels" . | nindent 8 }} | |||
| {{- if or .Values.annotations (and .Values.criticalAddon (eq .Release.Namespace "kube-system")) }} | |||
| {{- if or .Values.annotations .Values.criticalAddon }} | |||
There was a problem hiding this comment.
critical addons were restricted to kube-system namespace until 1.17 .
kubernetes/kubernetes#60596 (comment)
So, you need to check for k8s version before relaxing this. Otherwise pods will not start in < 1.17 release.
There was a problem hiding this comment.
| @@ -99,7 +99,7 @@ apiVersion: rbac.authorization.k8s.io/v1 | |||
| kind: RoleBinding | |||
| metadata: | |||
| name: {{ template "stash.fullname" . }}-apiserver-extension-server-authentication-reader | |||
| namespace: kube-system | |||
| namespace: {{ .Release.Namespace }} | |||
There was a problem hiding this comment.
This is a Role binding and can only created in the namespace of the role which is kube-system. Please revert this change.
|
@tamalsaha PR updated |
|
I added a commit to force deployment to restart when tls.cert/tls.key are changed in the |
|
Can you please squash the commits and sign-off? |
Signed-off-by: Bruno Clermont <bruno@robotinfra.com>
|
@tamalsaha done BTW I had been running this for 1 week now |
|
Just added a minor change that convert operator resources into a value |
Signed-off-by: Bruno Clermont <bruno@robotinfra.com>
Signed-off-by: Bruno Clermont <bruno@robotinfra.com>
Signed-off-by: Bruno Clermont <bruno@robotinfra.com>
I just realized it wasn't that but an other fork, testing it |
tested |
|
something else not ok and that I should do with this PR? |
|
I had been using this in production on few clusters for > 3 weeks |
| @@ -7,6 +10,7 @@ metadata: | |||
| {{- include "stash.labels" . | nindent 4 }} | |||
| {{- if .Values.annotations }} | |||
| annotations: | |||
| checksum/apiregistration.yaml: {{ include (print $.Template.BasePath "/apiregistration.yaml") . | sha256sum }} | |||
There was a problem hiding this comment.
|
Thanks a lot for your work on this pr, @bclermont ! I really appreciate this. And sorry for the delay to merge the pr. |
Is there any reason why documentation use
kube-system?I rather keep different apps in separated namespaces.