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

Namespace status conditions #949

Merged

Conversation

wozniakjan
Copy link
Member

@wozniakjan wozniakjan commented Oct 14, 2019

What this PR does / why we need it:
Add metrics for namespace status conditions - kubernetes/community#4045, kubernetes/kubernetes#73405

NOTE: this requires kubernetes v1.16 among dependencies. There is a tracker which updates the dependencies #945 and this PR should wait and rebase when #945 merges. As a makeshift solution for testing, this uses fork of autoscaler https://github.com/wozniakjan/autoscaler with swapped go dep to go mod and includes necessary changes to bring kubernetes v1.16.1 as a dependency. I will try to upstream the changes to autoscaler to unblock dependency mismatch. 7a75a4a contains the dependency update mentioned in #945

Sample output:

$ k get ns
NAME              STATUS        AGE
default           Active        3d5h
kube-node-lease   Active        3d5h
kube-public       Active        3d5h
kube-system       Active        3d5h
test              Terminating   3d5h

$ curl localhost:8080/metrics | grep namespace_status
# HELP kube_namespace_status_phase kubernetes namespace status phase.
# TYPE kube_namespace_status_phase gauge
kube_namespace_status_phase{namespace="kube-node-lease",phase="Active"} 1
kube_namespace_status_phase{namespace="kube-node-lease",phase="Terminating"} 0
kube_namespace_status_phase{namespace="default",phase="Active"} 1
kube_namespace_status_phase{namespace="default",phase="Terminating"} 0
kube_namespace_status_phase{namespace="test",phase="Active"} 0
kube_namespace_status_phase{namespace="test",phase="Terminating"} 1
kube_namespace_status_phase{namespace="kube-system",phase="Active"} 1
kube_namespace_status_phase{namespace="kube-system",phase="Terminating"} 0
kube_namespace_status_phase{namespace="kube-public",phase="Active"} 1
kube_namespace_status_phase{namespace="kube-public",phase="Terminating"} 0
# HELP kube_namespace_status_condition The condition of a namespace.
# TYPE kube_namespace_status_condition gauge
kube_namespace_status_condition{namespace="test",condition="NamespaceDeletionDiscoveryFailure",status="true"} 1
kube_namespace_status_condition{namespace="test",condition="NamespaceDeletionDiscoveryFailure",status="false"} 0
kube_namespace_status_condition{namespace="test",condition="NamespaceDeletionDiscoveryFailure",status="unknown"} 0
kube_namespace_status_condition{namespace="test",condition="NamespaceDeletionGroupVersionParsingFailure",status="true"} 0
kube_namespace_status_condition{namespace="test",condition="NamespaceDeletionGroupVersionParsingFailure",status="false"} 1
kube_namespace_status_condition{namespace="test",condition="NamespaceDeletionGroupVersionParsingFailure",status="unknown"} 0
kube_namespace_status_condition{namespace="test",condition="NamespaceDeletionContentFailure",status="true"} 0
kube_namespace_status_condition{namespace="test",condition="NamespaceDeletionContentFailure",status="false"} 1
kube_namespace_status_condition{namespace="test",condition="NamespaceDeletionContentFailure",status="unknown"} 0

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 14, 2019
@wozniakjan wozniakjan changed the title Namespace status conditions [WIP] Namespace status conditions Oct 14, 2019
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 14, 2019
@tariq1890
Copy link
Contributor

Thanks @wozniakjan! I've also created this issue on the autoscaler repository - kubernetes/autoscaler#2447

Copy link
Member

@lilic lilic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR. Do you mind also adding this to the docs, thanks!

internal/store/namespace.go Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 25, 2019
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 25, 2019
@wozniakjan wozniakjan force-pushed the namespace_status_conditions branch 2 times, most recently from 9e5bd83 to e29f112 Compare October 25, 2019 10:19
@wozniakjan
Copy link
Member Author

wozniakjan commented Nov 1, 2019

@tariq1890, @brennerm the VPA has now updated the deps kubernetes/autoscaler#2486

In 7a75a4a I tried to update the go modules with updated VPA. This was the smallest set of changes allowed me to build the kube-state-metrics, any hints and pointers on how to make it even smaller so I don't pollute the go.mod file are greatly appreciated. The reason for the replace directives is VPA now comes with go.mod that comes with set of changes requiring the replacements kubernetes/autoscaler#2486, kubernetes/kubernetes#79384

@wozniakjan wozniakjan changed the title [WIP] Namespace status conditions Namespace status conditions Nov 1, 2019
@wozniakjan wozniakjan marked this pull request as ready for review November 1, 2019 11:37
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 1, 2019
Copy link
Member

@lilic lilic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good to me, are we ready to move forward with this PR?

cc @tariq1890 to re-review as well

go.mod Outdated Show resolved Hide resolved
- k8s.io/apimachinery to v1.16.2
- k8s.io/klog to v1.0.0
- k8s.io/autoscaler to current master
Copy link
Contributor

@tariq1890 tariq1890 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 15, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

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 k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 15, 2019
@k8s-ci-robot k8s-ci-robot merged commit 75a2d9d into kubernetes:master Nov 15, 2019
@tariq1890
Copy link
Contributor

@wozniakjan Thank you so much for your hard work with this! You are awesome :).

@wozniakjan
Copy link
Member Author

Thanks for valuable feedback and reviewing this to successful merge, always happy to help :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants