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.

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

Cannot add serviceAccount to Prometheus object #444

Closed
farcaller opened this issue Jun 23, 2017 · 8 comments
Closed

Cannot add serviceAccount to Prometheus object #444

farcaller opened this issue Jun 23, 2017 · 8 comments

Comments

@farcaller
Copy link

What did you do?
Tried to enable rbac, following up after erros in prometheus pod console by adding:

serviceAccountName: prometheus

to my

apiVersion: monitoring.coreos.com/v1alpha1
kind: Prometheus

object.

What did you expect to see?
Prometheus is restarted and works as expected with no errors.

What did you see instead? Under which circumstances?
Prometheus isn't restarted. Prometheus opreator is stuck.

Environment

  • Kubernetes version information:
Client Version: version.Info{Major:"1", Minor:"6", GitVersion:"v1.6.6", GitCommit:"7fa1c1756d8bc963f1a389f4a6937dc71f08ada2", GitTreeState:"clean", BuildDate:"2017-06-16T20:46:19Z", GoVersion:"go1.8.3", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"6", GitVersion:"v1.6.6", GitCommit:"7fa1c1756d8bc963f1a389f4a6937dc71f08ada2", GitTreeState:"clean", BuildDate:"2017-06-16T18:21:54Z", GoVersion:"go1.7.6", Compiler:"gc", Platform:"linux/amd64"}
  • Kubernetes cluster kind:

kubeadm

  • Manifests:

n/a

  • Prometheus Operator Logs:
E0623 13:43:33.026580       1 operator.go:521] Sync "kube-prometheus/prometheus" failed: updating statefulset failed: StatefulSet.apps "prometheus-prometheus" is invalid: spec: Forbidden: updates to statefulset spec for fields other than 'replicas' and 'containers' are forbidden.
@brancz
Copy link
Contributor

brancz commented Jun 28, 2017

Unfortunately modifying fields of the PodTemplate is an illegal action on a StatefulSet. This is going to change in the future, so I'm hesitant to make modifications which we would have to revert again soon. In more critical parts we have logic that prevents this part of the StatefulSet to be reconciled.

If you think we shouldn't error in this case, I can offer to add the ServiceAccountName in here, which will prevent it from being reconciled, however, it does not end up being changed, it would just prevent the Prometheus Operator from erroring.

Wdyt?

@farcaller
Copy link
Author

Is re-creating Prometheus StatefulSet an option?

@brancz
Copy link
Contributor

brancz commented Jun 28, 2017

That's exactly what I don't want to build into the codebase today. From what I can tell, StatefulSet upgrades are being released for 1.7. And 1.7 release is targeted for today 🎉

@farcaller
Copy link
Author

Ah great! Let's keep it open for a day, until I'll roll out 1.7 and would confirm that the new setup is WAI?

@fabxc
Copy link
Contributor

fabxc commented Jul 3, 2017

@farcaller any update?

@farcaller
Copy link
Author

Yep, sorry, just did an upgrade to 1.7 and had to fight a few other breakages.

So, if I go a transition from no serviceAccountName to serviceAccountName: prometheus, it propagates to StatefulSet, but stateful set won't do anything to the pod and prometheus is still broken.

I'm not sure if that's a StatefulSet bug in k8s or an expected behaviour, but from my prometheus-operator user standpoint, I see it as a prometheus-operator bug (i.e. it didn't take the manual churn of killing the old pod for me).

@fabxc
Copy link
Contributor

fabxc commented Jul 4, 2017

Expected behavior. Stateful sets currently do not do any upgrading at this point. We do it instead in the operator but only on a small selection of changes we know we can do safely.
In this case, you could upgrade by killing the pods one by one – they should be automatically restarted with the correct service account.

@brancz
Copy link
Contributor

brancz commented Jul 24, 2017

I agree that this is something that the Prometheus Operator needs to handle, however, it's part of a larger story of rolling out any changes to the StatefulSet and its PodTemplate. Therefore closing this in favor of #49 as it captures the higher level problem that we want to tackle. We'll track this further there. Thanks for reporting and following up @farcaller. Greatly appreciated!

@brancz brancz closed this as completed Jul 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants