fix: change the expectedType of the kube-client reflector to match the listWatch #720

tariq1890 · 2019-04-13T20:15:47Z

Fixes #719

This PR adds what was missed in PR #652.

Add list/watch access to ClusterRole for CSR collector

…e listWatch

tariq1890 · 2019-04-14T06:53:56Z

This needs to be merged into the release-1.6 branch.

brancz · 2019-04-15T07:26:30Z

Do I understand correctly, that with this we're getting both appsv1 and backward compatible extensionsv1beta1 support back?

cc @mxinden

mxinden · 2019-04-15T09:40:41Z

This needs to be merged into the release-1.6 branch.

In that case this pull request needs to point to the release-1.6 branch, @tariq1890 . Once merged, we can merge release-1.6 back into master, preserving git commit hashes across branches.

tariq1890 · 2019-04-17T05:46:48Z

@brancz I'll have to test with a pod created with that api Group and see if it's captures and represented properly when kube state metric queries the pod. I haven't had much time on my hands off late. Will provide an update on this as soon as I get the bandwidth.

lilic

I tested this and it fixes the error for Deployments I was seeing as well. Thanks!

I don't think we need to downgrade the api support to appsv1beta1 for StatefulSets as according to the kube-state-metrics README only 1.10 onwards kubernetes versions are supported and StatefulSets were already out of beta in kubernetes 1.9 version.

Another thing: Would it make sense to adjust this?

brancz · 2019-04-18T07:07:13Z

I agree with @lilic let's just use v1apps for Deployments, StatefulSets and DaemonSets. It'll also be future proof.

tariq1890 · 2019-04-18T07:14:40Z

Do you want me to do that in this PR @brancz or in another one? Currently this PR is scoped to fixing #719. But that's just me saying that :).

brancz · 2019-04-18T07:18:39Z

The way I see it is this would be future proofing and fixing the bug :)

tariq1890 · 2019-04-18T08:02:44Z

I just tested a deployment manifest created with extensions/v1beta1 apiGroup with the kube-state-metrics code and I could see the metrics being exported successfully. I think it's safe to say that backwards compatibility with the deprecated extensions/v1beta1 apiGroup is preserved.

I will start implementing @lilic's suggestion now.

tariq1890 · 2019-04-18T08:19:13Z

internal/collector/statefulset.go

 				return &metric.Family{
-					Metrics: ms,
+					Metrics: []*metric.Metric{


This change is merely a simplification.

tariq1890 · 2019-04-18T08:59:47Z

CI passed and ready for review.

mxinden · 2019-04-18T09:23:47Z

Would you mind changing the target branch to release-1.6? See #720 (comment).

lilic

This worked for me 👍 Thanks!

lilic · 2019-04-18T09:24:55Z

vendor/modules.txt

 k8s.io/api/autoscaling/v2beta1
 k8s.io/api/batch/v1
 k8s.io/api/batch/v1beta1
 k8s.io/api/certificates/v1beta1
 k8s.io/api/core/v1
 k8s.io/api/extensions/v1beta1
 k8s.io/api/policy/v1beta1
+k8s.io/api/apps/v1beta1


Just curious why go mods does this change, moves the import in this file. The pkg is not used anymore in the code directly, guessing its used by one of the other deps e.g. client-go internally, but it makes me wonder if we can bump the version of client-go and in turn get rid of these dependencies. (But that might be going down the rabbit hole of solving deps, and probably falls outside of this PR :)

That makes sense. But yes, this definitely is out of scope for this PR. I believe we have k8s 1.14 depped into kube state metrics. We are just behind the latest by a patch release.

tariq1890 · 2019-04-18T14:32:33Z

@mxinden Done!

brancz · 2019-04-18T14:53:57Z

/lgtm

but giving @mxinden the last call

/hold

k8s-ci-robot · 2019-04-18T14:54:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brancz, tariq1890

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~~ [brancz]

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

mxinden · 2019-04-18T15:09:18Z

Thanks for the fix @tariq1890!

/hold cancel

PR #720 into master

AndrewDryga · 2020-05-19T10:07:08Z

This issue has a regression, v1.9.6:

"pkg/mod/k8s.io/client-go@v0.0.0-20191109102209-3c0d1af94be5/tools/cache/reflector.go:108: expected type *v1.MutatingWebhookConfiguration, but watch event object had type *v1beta1.MutatingWebhookConfiguration"

lizhifengones · 2023-08-02T07:35:54Z

expected type *v1.MutatingWebhookConfiguration, but watch event object had type *v1beta1.MutatingWebhookConfiguration"

Is there any response?

lizhifengones · 2023-08-02T07:46:20Z

expected type *v1.MutatingWebhookConfiguration, but watch event object had type *v1beta1.MutatingWebhookConfiguration"

Is there any response?

#1142

k8s-ci-robot and others added 2 commits April 10, 2019 00:22

Merge pull request kubernetes#718 from kubernetes/release-1.6

0f8266a

Add list/watch access to ClusterRole for CSR collector

fix: change the expectedType of the kube-client reflector to match th…

fd4d840

…e listWatch

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 13, 2019

k8s-ci-robot requested review from brancz and mxinden April 13, 2019 20:15

k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 13, 2019

tariq1890 mentioned this pull request Apr 13, 2019

expected type *v1beta1.Deployment, but watch event object had type *v1.Deployment #719

Closed

lilic reviewed Apr 17, 2019

View reviewed changes

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 18, 2019

tariq1890 commented Apr 18, 2019

View reviewed changes

tariq1890 force-pushed the fix_reflector branch from 0fcdde4 to a7357b7 Compare April 18, 2019 08:29

use appsv1 apigroup for StatefulSets and DaemonSets

6131a28

tariq1890 force-pushed the fix_reflector branch from a7357b7 to 6131a28 Compare April 18, 2019 08:51

lilic reviewed Apr 18, 2019

View reviewed changes

tariq1890 changed the base branch from master to release-1.6 April 18, 2019 14:32

k8s-ci-robot assigned brancz Apr 18, 2019

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

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 18, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 18, 2019

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

k8s-ci-robot merged commit ceffc8e into kubernetes:release-1.6 Apr 18, 2019

mxinden mentioned this pull request Apr 18, 2019

PR #720 into master #723

Merged

k8s-ci-robot added a commit that referenced this pull request Apr 18, 2019

Merge pull request #723 from kubernetes/release-1.6

5bcff06

PR #720 into master

tariq1890 deleted the fix_reflector branch April 18, 2019 16:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: change the expectedType of the kube-client reflector to match the listWatch #720

fix: change the expectedType of the kube-client reflector to match the listWatch #720

tariq1890 commented Apr 13, 2019 •

edited

Loading

tariq1890 commented Apr 14, 2019

brancz commented Apr 15, 2019

mxinden commented Apr 15, 2019

tariq1890 commented Apr 17, 2019

lilic left a comment

brancz commented Apr 18, 2019

tariq1890 commented Apr 18, 2019

brancz commented Apr 18, 2019

tariq1890 commented Apr 18, 2019

tariq1890 Apr 18, 2019

tariq1890 commented Apr 18, 2019

mxinden commented Apr 18, 2019

lilic left a comment

lilic Apr 18, 2019 •

edited

Loading

tariq1890 Apr 18, 2019

tariq1890 commented Apr 18, 2019

brancz commented Apr 18, 2019

k8s-ci-robot commented Apr 18, 2019

mxinden commented Apr 18, 2019

AndrewDryga commented May 19, 2020

lizhifengones commented Aug 2, 2023

lizhifengones commented Aug 2, 2023

fix: change the expectedType of the kube-client reflector to match the listWatch #720

fix: change the expectedType of the kube-client reflector to match the listWatch #720

Conversation

tariq1890 commented Apr 13, 2019 • edited Loading

tariq1890 commented Apr 14, 2019

brancz commented Apr 15, 2019

mxinden commented Apr 15, 2019

tariq1890 commented Apr 17, 2019

lilic left a comment

Choose a reason for hiding this comment

brancz commented Apr 18, 2019

tariq1890 commented Apr 18, 2019

brancz commented Apr 18, 2019

tariq1890 commented Apr 18, 2019

tariq1890 Apr 18, 2019

Choose a reason for hiding this comment

tariq1890 commented Apr 18, 2019

mxinden commented Apr 18, 2019

lilic left a comment

Choose a reason for hiding this comment

lilic Apr 18, 2019 • edited Loading

Choose a reason for hiding this comment

tariq1890 Apr 18, 2019

Choose a reason for hiding this comment

tariq1890 commented Apr 18, 2019

brancz commented Apr 18, 2019

k8s-ci-robot commented Apr 18, 2019

mxinden commented Apr 18, 2019

AndrewDryga commented May 19, 2020

lizhifengones commented Aug 2, 2023

lizhifengones commented Aug 2, 2023

tariq1890 commented Apr 13, 2019 •

edited

Loading

lilic Apr 18, 2019 •

edited

Loading