-
Notifications
You must be signed in to change notification settings - Fork 779
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
Upgrading metrics-server to v0.3.2 #600
Conversation
@@ -99,51 +99,22 @@ spec: | |||
serviceAccountName: metrics-server | |||
containers: | |||
- name: metrics-server | |||
image: k8s.gcr.io/metrics-server-$ARCH:v0.2.1 | |||
image: k8s.gcr.io/metrics-server-$ARCH:v0.3.3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not see the v0.3.3 available for arm https://console.cloud.google.com/gcr/images/google-containers/GLOBAL/metrics-server-arm?gcrImageListsize=30 . Should we go for v0.3.2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested it on amd64, so didn't realize this. Let me change this to v0.3.2
please
@@ -219,7 +181,7 @@ metadata: | |||
rbac.authorization.k8s.io/aggregate-to-admin: "true" | |||
rules: | |||
- apiGroups: ["metrics.k8s.io"] | |||
resources: ["pods", "nodes"] | |||
resources: ["pods"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We got this part 11 days ago from @giner: 8920c10
@junaid-ali, @giner do we need the "nodes" here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@giner, do we need this when we have it here: https://github.com/kubernetes-incubator/metrics-server/blob/v0.3.3/deploy/1.8%2B/resource-reader.yaml#L11
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me microk8s.kubectl top nodes
is working:
$ microk8s.kubectl top nodes
NAME CPU(cores) CPU% MEMORY(bytes) MEMORY%
junaid-hp-z240-tower-workstation 828m 20% 20263Mi 63%
Here are the cluster roles:
$ microk8s.kubectl -n kube-system get clusterrole system:aggregated-metrics-reader -o yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
annotations:
kubectl.kubernetes.io/last-applied-configuration: |
{"apiVersion":"rbac.authorization.k8s.io/v1","kind":"ClusterRole","metadata":{"annotations":{},"labels":{"rbac.authorization.k8s.io/aggregate-to-admin":"true","rbac.authorization.k8s.io/aggregate-to-edit":"true","rbac.authorization.k8s.io/aggregate-to-view":"true"},"name":"system:aggregated-metrics-reader"},"rules":[{"apiGroups":["metrics.k8s.io"],"resources":["pods"],"verbs":["get","list","watch"]}]}
creationTimestamp: "2019-08-16T14:27:53Z"
labels:
rbac.authorization.k8s.io/aggregate-to-admin: "true"
rbac.authorization.k8s.io/aggregate-to-edit: "true"
rbac.authorization.k8s.io/aggregate-to-view: "true"
name: system:aggregated-metrics-reader
resourceVersion: "3115824"
selfLink: /apis/rbac.authorization.k8s.io/v1/clusterroles/system%3Aaggregated-metrics-reader
uid: 25e0931b-1543-432a-802d-cc3465e84304
rules:
- apiGroups:
- metrics.k8s.io
resources:
- pods
verbs:
- get
- list
- watch
$ microk8s.kubectl -n kube-system get clusterrole system:metrics-server -o yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
annotations:
kubectl.kubernetes.io/last-applied-configuration: |
{"apiVersion":"rbac.authorization.k8s.io/v1","kind":"ClusterRole","metadata":{"annotations":{},"labels":{"addonmanager.kubernetes.io/mode":"Reconcile","kubernetes.io/cluster-service":"true"},"name":"system:metrics-server"},"rules":[{"apiGroups":[""],"resources":["pods","nodes","nodes/stats"],"verbs":["get","list","watch"]}]}
creationTimestamp: "2019-08-16T14:27:53Z"
labels:
addonmanager.kubernetes.io/mode: Reconcile
kubernetes.io/cluster-service: "true"
name: system:metrics-server
resourceVersion: "3115821"
selfLink: /apis/rbac.authorization.k8s.io/v1/clusterroles/system%3Ametrics-server
uid: d7b0d0bb-f68a-41eb-ac6c-7685c4d1af2e
rules:
- apiGroups:
- ""
resources:
- pods
- nodes
- nodes/stats
verbs:
- get
- list
- watch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, these are different.
system:metrics-server
role is used to access kubernetes API by metrics-server
system:aggregated-metrics-reader
is used to access metrics-server API itself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is rbac enabled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, will add nodes
in resources again
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is rbac enabled?
Wow, it was disabled.
Yes, we do kubernetes-sigs/metrics-server#297 |
We need to also handle the upgrade case of MicroK8s. When this PR gets merged the new Also, upgraded MicroK8s deployment will need to get the kubelet arguments updated and the service restart. The script that run during an upgrade is the configure one found in https://github.com/ubuntu/microk8s/blob/master/snap/hooks/configure. We will need an if statement sayin "if the new arguments are missing from kubelet, then add them", similar to this one https://github.com/ubuntu/microk8s/blob/master/snap/hooks/configure#L135 Thank you for being on top of this PR @junaid-ali. |
@ktsakalozos apologies, couldn't follow the PR for a few days. I have started tested it again. When we enable rbac, I caught a few missing things (we have to add flags to apiserver for API aggregation, https://kubernetes.io/docs/tasks/access-kubernetes-api/configure-aggregation-layer/#enable-kubernetes-apiserver-flags). Also there is one issue still left, |
@junaid-ali, enabling aggregation layer is merged to master and will be in the snap with version 1.15.3. |
Is there any way to get this merged? 0.2.1 is quite old now and newer dashboards in Grafana are not working for me. I've tried installing it via the helm chart instead of enabling it via microk8s but that does not seem to work. |
I just realized I never finished this PR 😅 Hopefully I will work on this over the weekend to wrap this up if no one else is working on it |
That would be great. Let me know if and how I can help with this. Everything is working well with microk8s so far except this in my setup. |
* Updated Cluster Role Link: https://github.com/kubernetes-incubator/metrics-server/tree/master/deploy/1.8%2B * Added flags `authentication-token-webhook` and `authorization-mode` to kubelet since `--anonymous-auth` flag is `false` Links: - https://kubernetes.io/docs/reference/command-line-tools-reference/kubelet-authentication-authorization/#kubelet-authentication - https://kubernetes.io/docs/reference/command-line-tools-reference/kubelet-authentication-authorization/#kubelet-authorization - kubernetes/kops#7200 (comment) Signed-off-by: Junaid Ali <[email protected]>
v0.3.3 is not available for arm Signed-off-by: Junaid Ali <[email protected]>
I have this issue where as soon as I enable
|
The following error in api server logs (
|
The following error on kubelet:
|
@ktsakalozos any idea why this error could happen? |
Although, metrics server is running fine:
|
@junaid-ali there is an RBAC denial in the logs you pasted. Is this PR ready for a second review round? |
The addition of the authentication-token-webhook and authorization-mode flags are also needed for prometheus kubelet metrics retrieval. It would really help to get this merged as soon as possible. |
I still need to address your comment at #600 (comment) |
@drewboswell enabling authentication webhook causes
Still need to debug from where this |
Do you have any idea when this pr will be done or do you suggest I try to get the helm chart working instead? |
@junaid-ali im sorry i also did a PR on upgrading metrics server. Didn't realize that there is one. |
@balchua I couldn't really work on it due to to other commitments. Please go ahead with your PR, check the comments here if that can help you in any way. |
@junaid-ali Thanks for letting me know. In that case, will be closing this PR in favor of this #1201 |
Updated Cluster Role
Link: https://github.com/kubernetes-incubator/metrics-server/tree/master/deploy/1.8%2B
Added flags
authentication-token-webhook
andauthorization-mode
to kubelet since--anonymous-auth
flag isfalse
Links:
Signed-off-by: Junaid Ali [email protected]