clusterloader2: Fix error of probes about metrics-server #2066

yangjunmyfm192085 · 2022-05-13T02:51:00Z

What type of PR is this?
/kind bug

What this PR does / why we need it:
When using InClusterAPIServerRequestLatency to measure the latency of metrics-server, need to add serviceMonitor and other information, and update the probe image
Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

"NONE"

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

"NONE"

yangjunmyfm192085 · 2022-05-13T02:54:41Z

Plan to use perf-test in kubernetes-sigs/metrics-server, which requires the probe resources, fix the bugs
/cc @wojtek-t @mborsz @mm4tt

yangjunmyfm192085 · 2022-05-13T04:54:06Z

.../pkg/measurement/common/probes/manifests/metricsServer/metrics-server-prober-deployment.yaml

    spec:
      containers:
        - name: prober
-          image: gcr.io/k8s-testimages/probes:v0.0.4
+          image: gcr.io/k8s-testimages/probes:v0.0.5


@mm4tt, I build the image using https://github.com/kubernetes/perf-tests/blob/master/util-images/probes/Dockerfile but don't have permission to push gcr.io/k8s-testimages/probes:v0.0.5.
I think I need help to push an image gcr.io/k8s-testimages/probes:v0.0.5 to registry

/cc @wojtek-t @mborsz @mm4tt

wojtek-t · 2022-05-16T09:07:01Z

.../pkg/measurement/common/probes/manifests/metricsServer/metrics-server-prober-deployment.yaml

@@ -4,27 +4,27 @@ metadata:
  namespace: probes
  name: metrics-server-prober
  labels:
-    probe: metrics-server-probe
+    probe: kube-client


Hmm - I actually likes the metrics-server-probe (or sth like that).

kube-client seems too generic too me.

Why are you changing this?

yeah, thanks for review. I have updated all suggestions

wojtek-t · 2022-05-16T09:07:43Z

...oader2/pkg/measurement/common/probes/manifests/metricsServer/metrics-server-prober-role.yaml

@@ -0,0 +1,14 @@
+---
+apiVersion: rbac.authorization.k8s.io/v1


What's the point of splitting into multiple files? Keeping whole BRA-related stuff in one file as it was actually is pretty convinient to use...

ok, I have I have rollbacked to use one file.

yangjunmyfm192085 · 2022-05-16T11:29:37Z

/test pull-perf-tests-clusterloader2

yangjunmyfm192085 · 2022-05-16T11:32:33Z

/retest

yangjunmyfm192085

/cc @wojtek-t I have updated all suggestions.

wojtek-t · 2022-05-17T09:30:43Z

...oader2/pkg/measurement/common/probes/manifests/metricsServer/metrics-server-prober-rbac.yaml

+apiVersion: v1
+kind: List
+items:
+- apiVersion: rbac.authorization.k8s.io/v1


I don't understand this...

k8s doesn't support list containing multiple types of objects...

I still don't understand why you're touching it - what didn't work with the original file?

The issue is here

perf-tests/clusterloader2/pkg/framework/framework.go

Line 275 in e6c9306

obj, err := templateProvider.TemplateToObject(filepath.Base(manifest), templateMapping)

According to the original file, only the first object can be parsed from the file, and the following cannot be parsed.

/cc @wojtek-t
In the original file, only one type of resource can be loaded, and the rest cannot be loaded.

and I observed that other manifest files also have only one resource.

OK - that makes sense.

k8s-ci-robot · 2022-05-19T07:26:17Z

@yangjunmyfm192085: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-perf-tests-clusterloader2-e2e-gce-scale-performance-manual	`d74cfe8`	link	true	`/test pull-perf-tests-clusterloader2-e2e-gce-scale-performance-manual`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

yangjunmyfm192085

/test pull-perf-tests-clusterloader2-e2e-gce-scale-performance-manual

k8s-ci-robot · 2022-05-19T12:01:20Z

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

/test pull-perf-tests-benchmark-kube-dns
/test pull-perf-tests-clusterloader2
/test pull-perf-tests-clusterloader2-kubemark
/test pull-perf-tests-util-images
/test pull-perf-tests-verify-all-python
/test pull-perf-tests-verify-dashboard
/test pull-perf-tests-verify-lint
/test pull-perf-tests-verify-test

The following commands are available to trigger optional jobs:

/test pull-perf-tests-100-adhoc
/test soak-tests-capz-windows-2019

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

pull-perf-tests-clusterloader2
pull-perf-tests-clusterloader2-kubemark
pull-perf-tests-verify-all-python
pull-perf-tests-verify-lint
pull-perf-tests-verify-test

In response to this:

/test pull-perf-tests-clusterloader2-e2e-gce-scale-performance-manual

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.

yangjunmyfm192085

/retest

yangjunmyfm192085 · 2022-05-19T14:24:02Z

Hi, @wojtek-t, Is this analysis correct?

wojtek-t · 2022-06-02T17:11:51Z

/assign @marseel

@marseel - can you please help with pushing the new image?

wojtek-t · 2022-06-02T17:12:10Z

/approve

But we need to push the image before lgtm-ing.

marseel · 2022-06-06T09:19:06Z

I've pushed 0.0.5, but please change also util-images/probes/Makefile version from 0.0.4 to 0.0.5

yangjunmyfm192085

/cc @marseel

yangjunmyfm192085 · 2022-06-09T06:26:26Z

I've pushed 0.0.5, but please change also util-images/probes/Makefile version from 0.0.4 to 0.0.5

Hi, @marseel I have applied all modifications, any other updates?

marseel · 2022-06-09T11:07:33Z

/lgtm

k8s-ci-robot · 2022-06-09T11:07:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: marseel, wojtek-t, yangjunmyfm192085

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:

~~clusterloader2/OWNERS~~ [marseel,wojtek-t]
~~util-images/probes/OWNERS~~ [marseel,wojtek-t]

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

k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 13, 2022

k8s-ci-robot requested review from mborsz and mm4tt May 13, 2022 02:51

yangjunmyfm192085 commented May 13, 2022

View reviewed changes

k8s-ci-robot requested a review from wojtek-t May 13, 2022 05:08

wojtek-t reviewed May 16, 2022

View reviewed changes

wojtek-t self-assigned this May 16, 2022

yangjunmyfm192085 force-pushed the fixprobebugs branch from b8ef2a6 to 2d467e6 Compare May 16, 2022 11:27

yangjunmyfm192085 force-pushed the fixprobebugs branch from 2d467e6 to d74cfe8 Compare May 16, 2022 11:31

yangjunmyfm192085 commented May 16, 2022

View reviewed changes

wojtek-t reviewed May 17, 2022

View reviewed changes

yangjunmyfm192085 commented May 19, 2022

View reviewed changes

k8s-ci-robot requested a review from wojtek-t May 30, 2022 08:09

k8s-ci-robot assigned marseel Jun 2, 2022

marseel removed their assignment Jun 6, 2022

clusterloader2: Fix error of probes about metrics-server

6a76603

yangjunmyfm192085 force-pushed the fixprobebugs branch from d74cfe8 to 6a76603 Compare June 6, 2022 12:44

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 6, 2022

yangjunmyfm192085 commented Jun 7, 2022

View reviewed changes

k8s-ci-robot assigned marseel Jun 9, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 9, 2022

k8s-ci-robot merged commit 47169b6 into kubernetes:master Jun 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

clusterloader2: Fix error of probes about metrics-server #2066

clusterloader2: Fix error of probes about metrics-server #2066

yangjunmyfm192085 commented May 13, 2022

yangjunmyfm192085 commented May 13, 2022 •

edited

Loading

yangjunmyfm192085 May 13, 2022 •

edited

Loading

yangjunmyfm192085 May 13, 2022

wojtek-t May 16, 2022

yangjunmyfm192085 May 16, 2022

wojtek-t May 16, 2022

yangjunmyfm192085 May 16, 2022

yangjunmyfm192085 commented May 16, 2022

yangjunmyfm192085 commented May 16, 2022

yangjunmyfm192085 left a comment

wojtek-t May 17, 2022

yangjunmyfm192085 May 17, 2022 •

edited

Loading

yangjunmyfm192085 May 30, 2022

yangjunmyfm192085 May 30, 2022 •

edited

Loading

wojtek-t Jun 2, 2022

k8s-ci-robot commented May 19, 2022

yangjunmyfm192085 left a comment

k8s-ci-robot commented May 19, 2022

yangjunmyfm192085 left a comment

yangjunmyfm192085 commented May 19, 2022

wojtek-t commented Jun 2, 2022

wojtek-t commented Jun 2, 2022

marseel commented Jun 6, 2022

yangjunmyfm192085 left a comment •

edited

Loading

yangjunmyfm192085 commented Jun 9, 2022

marseel commented Jun 9, 2022

k8s-ci-robot commented Jun 9, 2022

clusterloader2: Fix error of probes about metrics-server #2066

clusterloader2: Fix error of probes about metrics-server #2066

Conversation

yangjunmyfm192085 commented May 13, 2022

yangjunmyfm192085 commented May 13, 2022 • edited Loading

yangjunmyfm192085 May 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yangjunmyfm192085 commented May 16, 2022

yangjunmyfm192085 commented May 16, 2022

yangjunmyfm192085 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yangjunmyfm192085 May 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yangjunmyfm192085 May 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-ci-robot commented May 19, 2022

yangjunmyfm192085 left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented May 19, 2022

yangjunmyfm192085 left a comment

Choose a reason for hiding this comment

yangjunmyfm192085 commented May 19, 2022

wojtek-t commented Jun 2, 2022

wojtek-t commented Jun 2, 2022

marseel commented Jun 6, 2022

yangjunmyfm192085 left a comment • edited Loading

Choose a reason for hiding this comment

yangjunmyfm192085 commented Jun 9, 2022

marseel commented Jun 9, 2022

k8s-ci-robot commented Jun 9, 2022

yangjunmyfm192085 commented May 13, 2022 •

edited

Loading

yangjunmyfm192085 May 13, 2022 •

edited

Loading

yangjunmyfm192085 May 17, 2022 •

edited

Loading

yangjunmyfm192085 May 30, 2022 •

edited

Loading

yangjunmyfm192085 left a comment •

edited

Loading