Add metrics server as addon to k8s CI #1903

jsturtevant · 2021-12-02T21:47:19Z

What type of PR is this?
/kind feature

What this PR does / why we need it:
We would like to run HPA tests in upstream and the metrics server is required for HPA components: #1881

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1125

Special notes for your reviewer:
I've only added this to the CI Job that is need for now but did tried to do it in a way that would make it pretty easy to add to other jobs and tilt if we wanted.

Also using kustomize to pull in the metric server from the releases page on https://github.com/kubernetes-sigs/metrics-server/releases. This allows for easy updates and overriding settings if needed via kustomize. This could be a pattern we use for the other addons as well.

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

squashed commits
includes documentation
adds unit tests

Release note:

Add metrics server to CI jobs for upstream testing

jsturtevant · 2021-12-03T00:47:35Z

/hold

I've got it deploying but running into errors:

I1203 00:40:24.651567       1 server.go:188] "Failed probe" probe="metric-storage-ready" err="not metrics to serve"
E1203 00:40:29.522716       1 scraper.go:139] "Failed to scrape node" err="Get \"https://10.1.0.5:10250/stats/summary?only_cpu_and_memory=true\": x509: cannot validate certificate for 10.1.0.5 because it doesn't contain any IP SANs" node="capz-conf-bvvs4"
E1203 00:40:29.526596       1 scraper.go:139] "Failed to scrape node" err="Get \"https://10.1.0.4:10250/stats/summary?only_cpu_and_memory=true\": x509: cannot validate certificate for 10.1.0.4 because it doesn't contain any IP SANs" node="capz-conf-gtfr6"
E1203 00:40:29.527545       1 scraper.go:139] "Failed to scrape node" err="Get \"https://10.0.0.4:10250/stats/summary?only_cpu_and_memory=true\": x509: cannot validate certificate for 10.0.0.4 because it doesn't contain any IP SANs" node="capz-conf-5ghwmu-control-plane-9rlx5"

Apparently kubeadm doesn't set this up properly out of the box: https://kubernetes.io/docs/setup/production-environment/tools/kubeadm/troubleshooting-kubeadm/#cannot-use-the-metrics-server-securely-in-a-kubeadm-cluster

It looks like we are missing the ability to properly get signed certs:
https://kubernetes.io/docs/tasks/administer-cluster/kubeadm/kubeadm-certs/#kubelet-serving-certs

apiVersion: kubeadm.k8s.io/v1beta3
kind: ClusterConfiguration

apiVersion: kubelet.config.k8s.io/v1beta1
kind: KubeletConfiguration
serverTLSBootstrap: true

jsturtevant · 2021-12-03T00:51:37Z

looks like there has been some thought on how this works with capi: https://github.com/kubernetes-sigs/cluster-api/blob/58905fb5140a7844c3e24aa70d255fb8f9d0c491/docs/proposals/20210222-kubelet-authentication.md#changes-to-cluster-api-bootstrap-provider-kubeadm

jsturtevant · 2021-12-09T00:55:53Z

After following the directions in https://kubernetes.io/docs/tasks/administer-cluster/kubeadm/kubeadm-certs/#kubelet-serving-certs (note that ServerTLSBootstrap setting in kubelet config is actually the flag rotate-server-certificates in kubelet command line parameters

setting the field rotate-server-certificates on the kubelet join configurations for all the nodes did work but required manual intervention to approve the CSR before metric server would work. The cluster and the deployment still came up but with the same error as above before the manual approval.

As there is a proposal to address this long term in CAPI I will plan to turn on the metric server flag --kubelet-insecure-tls for the e2e tests with the plan to enable this customers longer term with the CAPI solution. It is possible to write a controller that would auto enable the CSR signing but would be not relevant once the CAPI work is in place.

Due to the way that kubeadm configures the kubelet with a self-signed certificate, the metric server can't verify the kubelet certs. There is a proposal in CAPI to manage this long term. For the e2e tests we disabling the tls verificiation. See kubernetes-sigs#1903 (comment) for more details.

jsturtevant · 2021-12-09T20:05:12Z

/test ls

k8s-ci-robot · 2021-12-09T20:05:13Z

@jsturtevant: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test pull-cluster-api-provider-azure-build
/test pull-cluster-api-provider-azure-e2e
/test pull-cluster-api-provider-azure-e2e-windows-dockershim
/test pull-cluster-api-provider-azure-test
/test pull-cluster-api-provider-azure-verify

The following commands are available to trigger optional jobs:

/test pull-cluster-api-provider-azure-apidiff
/test pull-cluster-api-provider-azure-apiversion-upgrade
/test pull-cluster-api-provider-azure-capi-e2e
/test pull-cluster-api-provider-azure-ci-entrypoint
/test pull-cluster-api-provider-azure-conformance
/test pull-cluster-api-provider-azure-conformance-with-ci-artifacts
/test pull-cluster-api-provider-azure-coverage
/test pull-cluster-api-provider-azure-e2e-exp
/test pull-cluster-api-provider-azure-e2e-full
/test pull-cluster-api-provider-azure-e2e-workload-upgrade-1-21-1-22
/test pull-cluster-api-provider-azure-e2e-workload-upgrade-1-22-latest-main
/test pull-cluster-api-provider-azure-upstream-windows-dockershim
/test pull-cluster-api-provider-azure-windows-containerd-upstream-with-ci-artifacts
/test pull-cluster-api-provider-azure-windows-containerd-upstream-with-ci-artifacts-serial-slow

Use /test all to run the following jobs that were automatically triggered:

pull-cluster-api-provider-azure-apidiff
pull-cluster-api-provider-azure-build
pull-cluster-api-provider-azure-ci-entrypoint
pull-cluster-api-provider-azure-coverage
pull-cluster-api-provider-azure-e2e
pull-cluster-api-provider-azure-e2e-exp
pull-cluster-api-provider-azure-e2e-windows-dockershim
pull-cluster-api-provider-azure-test
pull-cluster-api-provider-azure-verify

In response to this:

/test ls

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

jsturtevant · 2021-12-09T20:06:03Z

/test pull-cluster-api-provider-azure-conformance-with-ci-artifacts

jsturtevant · 2021-12-09T20:06:21Z

/test pull-cluster-api-provider-azure-windows-containerd-upstream-with-ci-artifacts

jsturtevant · 2021-12-10T00:33:26Z

The windows tests ran but have the flakes I am investigating in s separate workstream.

/test pull-cluster-api-provider-azure-windows-containerd-upstream-with-ci-artifacts

jsturtevant · 2021-12-10T19:04:50Z

/assign @CecileRobertMichon @devigned @mboersma

CecileRobertMichon · 2021-12-10T19:43:49Z

templates/addons/metrics-server/kustomization.yaml

+kind: Kustomization
+namespace: kube-system
+resources:
+  - https://github.com/kubernetes-sigs/metrics-server/releases/download/v0.5.2/components.yaml


this is awesome, we should use this model for more stuff (eg. cloud-provider) instead of maintaining addon specs in the repo

cool! I created a tracking issue with a few possible items: #1917

CecileRobertMichon

/lgtm

devigned

/lgtm

+1 to how you are pulling the metrics server resources

CecileRobertMichon · 2021-12-10T23:27:23Z

/approve

k8s-ci-robot · 2021-12-10T23:27:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [CecileRobertMichon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jsturtevant · 2021-12-10T23:32:58Z

/hold cancel

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 2, 2021

k8s-ci-robot requested review from alexeldeib and mboersma December 2, 2021 21:47

k8s-ci-robot added area/provider/azure Issues or PRs related to azure provider sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 2, 2021

jsturtevant force-pushed the add-metrics-server-to-ci branch 4 times, most recently from ab164e6 to 86c615a Compare December 3, 2021 00:25

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 3, 2021

jsturtevant force-pushed the add-metrics-server-to-ci branch from 86c615a to e8385ba Compare December 9, 2021 19:18

jsturtevant added 2 commits December 9, 2021 11:25

Add metric server as addon to k8s CI

7215edb

jsturtevant force-pushed the add-metrics-server-to-ci branch from e8385ba to 0093c77 Compare December 9, 2021 19:25

jsturtevant mentioned this pull request Dec 9, 2021

Support running serial/slow Windows jobs #1881

Merged

3 tasks

k8s-ci-robot assigned CecileRobertMichon Dec 10, 2021

k8s-ci-robot assigned devigned and mboersma Dec 10, 2021

CecileRobertMichon reviewed Dec 10, 2021

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 10, 2021

devigned approved these changes Dec 10, 2021

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 10, 2021

jsturtevant mentioned this pull request Dec 10, 2021

Use kustomize remote URL's to manage addons #1917

Closed

5 tasks

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 10, 2021

k8s-ci-robot merged commit 2b4a27e into kubernetes-sigs:main Dec 11, 2021

k8s-ci-robot added this to the v1.1 milestone Dec 11, 2021

jsturtevant mentioned this pull request Dec 11, 2021

Add Metrics server to clusters #1125

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add metrics server as addon to k8s CI #1903

Add metrics server as addon to k8s CI #1903

jsturtevant commented Dec 2, 2021

jsturtevant commented Dec 3, 2021

apiVersion: kubeadm.k8s.io/v1beta3
kind: ClusterConfiguration

jsturtevant commented Dec 3, 2021

jsturtevant commented Dec 9, 2021

jsturtevant commented Dec 9, 2021

k8s-ci-robot commented Dec 9, 2021

jsturtevant commented Dec 9, 2021

jsturtevant commented Dec 9, 2021

jsturtevant commented Dec 10, 2021

jsturtevant commented Dec 10, 2021

CecileRobertMichon Dec 10, 2021

jsturtevant Dec 10, 2021

CecileRobertMichon left a comment

devigned left a comment

CecileRobertMichon commented Dec 10, 2021

k8s-ci-robot commented Dec 10, 2021

jsturtevant commented Dec 10, 2021

Add metrics server as addon to k8s CI #1903

Add metrics server as addon to k8s CI #1903

Conversation

jsturtevant commented Dec 2, 2021

jsturtevant commented Dec 3, 2021

apiVersion: kubeadm.k8s.io/v1beta3 kind: ClusterConfiguration

jsturtevant commented Dec 3, 2021

jsturtevant commented Dec 9, 2021

jsturtevant commented Dec 9, 2021

k8s-ci-robot commented Dec 9, 2021

jsturtevant commented Dec 9, 2021

jsturtevant commented Dec 9, 2021

jsturtevant commented Dec 10, 2021

jsturtevant commented Dec 10, 2021

CecileRobertMichon Dec 10, 2021

Choose a reason for hiding this comment

jsturtevant Dec 10, 2021

Choose a reason for hiding this comment

CecileRobertMichon left a comment

Choose a reason for hiding this comment

devigned left a comment

Choose a reason for hiding this comment

CecileRobertMichon commented Dec 10, 2021

k8s-ci-robot commented Dec 10, 2021

jsturtevant commented Dec 10, 2021

apiVersion: kubeadm.k8s.io/v1beta3
kind: ClusterConfiguration