Graduate metrics/v1alpha1 to v1beta1 #51653

DirectXMan12 · 2017-08-30T20:55:51Z

This introduces v1beta1 of the resource metrics API, previously in alpha.
The v1alpha1 version remains for compatibility with the Heapster legacy version
of the resource metrics API, which is compatible with the v1alpha1 version. It also
renames the v1beta1 version to resource-metrics.metrics.k8s.io.

The HPA controller's REST clients (but not the legacy client) have been migrated as well.

Part of kubernetes/enhancements#118.

Migrate the metrics/v1alpha1 API to metrics/v1beta1.  The HorizontalPodAutoscaler
controller REST client now uses that version.  For v1beta1, the API is now known as
resource-metrics.metrics.k8s.io.

DirectXMan12 · 2017-08-30T20:56:28Z

cc @kubernetes/sig-instrumentation-api-reviews
@kubernetes/sig-autoscaling-api-reviews

cc @piosz

smarterclayton · 2017-08-30T23:49:34Z

Is there a feature issue to attach to this?

DirectXMan12 · 2017-08-31T14:01:16Z

Whoops, yep, will do -- it's part of kubernetes/enhancements#118 ("the master metrics API... needs to be stabilized").

DirectXMan12 · 2017-08-31T21:27:04Z

/retest

DirectXMan12 · 2017-08-31T21:27:31Z

let's see if that actually kicks the tests off -- it's been sitting here for a day, not doing anything

smarterclayton · 2017-08-31T22:52:36Z

So other than the name of the group I don't see any issues here with API. Will need sig approval and LGTM though.

piosz · 2017-09-01T12:33:51Z

staging/src/k8s.io/metrics/pkg/apis/metrics/register.go

@@ -22,7 +22,7 @@ import (
 )

 // GroupName is the group name use in this package
-const GroupName = "metrics"
+const GroupName = "resource-metrics.metrics.k8s.io"


Why not having just metrics.k8s.io. metrics.metrics sounds weird to me.

We already have custom-metrics.metrics.k8s.io, so it just seemed strange to use it as both a "namespace" for groups and a group itself. This would make it resource-metrics.metrics and custom-metrics.metrics, so they match.

can we have:
metrics.k8s.io for resource metrics
custom.metrics.k8s.io for custom metrics?

smarterclayton · 2017-09-01T14:50:40Z

Yeah those groups sound fine to me.

smarterclayton · 2017-09-01T14:51:15Z

We can get a release exception if that goes past code freeze since this would be new APIs, so if we generally agree on the shorter names we should do that for 1.8

smarterclayton · 2017-08-30T23:50:53Z

staging/src/k8s.io/metrics/pkg/apis/metrics/doc.go

 // +k8s:deepcopy-gen=package,register
-
+// +groupName=resource-metrics.metrics.k8s.io


How was group name decided? Is there a reason to stutter? "resource.metrics.k8s.io" is shorter

(this was my old comment that was not committed).

I'd prefer metrics.k8s.io and custom.metrics.k8s.io

piosz · 2017-09-01T14:54:56Z

@smarterclayton which group names sound fine for you?

smarterclayton · 2017-09-01T14:55:19Z

metrics.k8s.io for resource metrics
custom.metrics.k8s.io for custom metrics?

SGTM

piosz · 2017-09-01T15:01:27Z

@smarterclayton cool
@DirectXMan12 WDYT?

piosz · 2017-09-01T15:02:12Z

pkg/apis/batch/v1beta1/zz_generated.conversion.go

@@ -215,7 +214,8 @@ func Convert_batch_JobTemplate_To_v1beta1_JobTemplate(in *batch.JobTemplate, out

 func autoConvert_v1beta1_JobTemplateSpec_To_batch_JobTemplateSpec(in *v1beta1.JobTemplateSpec, out *batch.JobTemplateSpec, s conversion.Scope) error {
 	out.ObjectMeta = in.ObjectMeta
-	if err := batch_v1.Convert_v1_JobSpec_To_batch_JobSpec(&in.Spec, &out.Spec, s); err != nil {
+	// TODO: Inefficient conversion - can we improve it?


Is this change intended? Looks weird.

no, it's not. It's weird that the conversion-gen changed it...

piosz · 2017-09-01T15:03:35Z

pkg/generated/openapi/BUILD

@@ -59,6 +59,7 @@ openapi_library(
        "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1beta1",
        "k8s.io/metrics/pkg/apis/custom_metrics/v1alpha1",
        "k8s.io/metrics/pkg/apis/metrics/v1alpha1",


shouldn't this be removed?

piosz · 2017-09-01T15:03:42Z

staging/src/k8s.io/code-generator/cmd/go-to-protobuf/protobuf/cmd.go

@@ -98,6 +98,7 @@ func New() *Generator {
 			`k8s.io/api/networking/v1`,
 			`k8s.io/kubernetes/federation/apis/federation/v1beta1`,
 			`k8s.io/metrics/pkg/apis/metrics/v1alpha1`,


smarterclayton · 2017-09-01T21:04:51Z

staging/src/k8s.io/metrics/pkg/apis/metrics/v1beta1/register.go

+)
+
+// GroupName is the group name use in this package
+const GroupName = "metrics"


metrics.k8s.io?

smarterclayton · 2017-09-01T21:06:11Z

/approve

Names custom.metrics.k8s.io and metrics.k8s.io. The other APIs you are promoting look fine.

sjenning · 2017-09-02T02:40:55Z

/retest

sjenning · 2017-09-02T02:45:06Z

Wait... when did kops drop off the required list?! I'm not complaining; it was blocking the world. Just didn't notice before I started the retest.

liggitt · 2017-09-02T02:47:57Z

It is non-blocking while they work out why it wasn't running tests using the test code from the PR being tested

Automatic merge from submit-queue (batch tested with PRs 49727, 51792) Introducing metrics-server ref kubernetes/enhancements#271 There is still some work blocked on problems with repo synchronization: - migrate to `v1beta1` introduced in #51653 - bump deps to HEAD Will do it in a follow up PRs once the issue is resolved. ```release-note Introduced Metrics Server ```

This commit graduates them resource metrics API from v1alpha1 to v1beta1.

This commit updates the REST metrics client to use metrics/v1beta1. The legacy client still uses metrics/v1alpha1.

This commit renames metrics to metrics.k8s.io for the v1beta1 version, to give it a properly namespaced name which mirrors custom.metrics.k8s.io.

k8s-ci-robot · 2017-09-05T21:02:53Z

@DirectXMan12: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-e2e-kops-aws	`e1a22e8`	link	`/test pull-kubernetes-e2e-kops-aws`

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.

smarterclayton · 2017-09-05T21:53:47Z

@piosz is there an active release exception covering this yet?

This is API approved

piosz · 2017-09-06T08:57:33Z

@smarterclayton I don't know how the exception process works but my feeling is that it was reviewed and approved before the code freeze.

piosz · 2017-09-06T08:57:54Z

/lgtm

k8s-github-robot · 2017-09-06T08:58:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: DirectXMan12, piosz, smarterclayton

Associated issue: 118

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

Needs approval from an approver in each of these OWNERS Files:

~~cmd/kube-controller-manager/OWNERS~~ [smarterclayton]
~~hack/OWNERS~~ [smarterclayton]
~~pkg/OWNERS~~ [smarterclayton]
~~staging/OWNERS~~ [smarterclayton]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-github-robot · 2017-09-06T09:55:11Z

Automatic merge from submit-queue (batch tested with PRs 51603, 51653)

piosz · 2017-09-06T10:11:34Z

Ups. I unintentionally triggered merge. We can consider reverting this if people disagree with my #51653 (comment).

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 30, 2017

k8s-github-robot assigned liggitt and smarterclayton Aug 30, 2017

k8s-github-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Aug 30, 2017

DirectXMan12 force-pushed the versions/metrics-v2beta1 branch from b6221aa to 5b8f083 Compare August 31, 2017 21:54

DirectXMan12 force-pushed the versions/metrics-v2beta1 branch from 5b8f083 to 2451b45 Compare September 1, 2017 03:22

This was referenced Sep 1, 2017

Added deployment files kubernetes-sigs/metrics-server#4

Merged

Introducing metrics-server #51792

Merged

piosz reviewed Sep 1, 2017

View reviewed changes

smarterclayton reviewed Sep 1, 2017

View reviewed changes

piosz self-assigned this Sep 1, 2017

piosz reviewed Sep 1, 2017

View reviewed changes

smarterclayton reviewed Sep 1, 2017

View reviewed changes

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 1, 2017

DirectXMan12 force-pushed the versions/metrics-v2beta1 branch from bc506b3 to c571aac Compare September 1, 2017 23:54

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 3, 2017

DirectXMan12 added 3 commits September 5, 2017 16:09

Graduate metrics/v1alpha1 to v1beta1

580b28e

This commit graduates them resource metrics API from v1alpha1 to v1beta1.

Update HPA REST metrics client to metrics/v1beta1

86f24ea

This commit updates the REST metrics client to use metrics/v1beta1. The legacy client still uses metrics/v1alpha1.

Rename metrics to metrics.k8s.io

e1a22e8

This commit renames metrics to metrics.k8s.io for the v1beta1 version, to give it a properly namespaced name which mirrors custom.metrics.k8s.io.

DirectXMan12 force-pushed the versions/metrics-v2beta1 branch from c571aac to e1a22e8 Compare September 5, 2017 20:12

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 5, 2017

piosz added this to the v1.8 milestone Sep 6, 2017

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 6, 2017

k8s-github-robot merged commit 0076f02 into kubernetes:master Sep 6, 2017

piosz unassigned liggitt Sep 6, 2017

DirectXMan12 deleted the versions/metrics-v2beta1 branch September 6, 2017 13:09

This was referenced Sep 8, 2017

Metrics Server (for resource metrics API) kubernetes/enhancements#271

Closed

Publish updates of staging repos #51643

Closed

karataliu mentioned this pull request Dec 15, 2017

Kubernetes Horizontal Pod Autoscaler does not work for v1.8+ cluster Azure/acs-engine#1936

Closed

zdzaz mentioned this pull request May 22, 2020

API Server path update to Kubernetes v1.8 luxas/kubeadm-workshop#48

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Graduate metrics/v1alpha1 to v1beta1 #51653

Graduate metrics/v1alpha1 to v1beta1 #51653

DirectXMan12 commented Aug 30, 2017 •

edited

Loading

DirectXMan12 commented Aug 30, 2017

smarterclayton commented Aug 30, 2017

DirectXMan12 commented Aug 31, 2017

DirectXMan12 commented Aug 31, 2017

DirectXMan12 commented Aug 31, 2017

smarterclayton commented Aug 31, 2017

piosz Sep 1, 2017

DirectXMan12 Sep 1, 2017

piosz Sep 1, 2017

smarterclayton commented Sep 1, 2017

smarterclayton commented Sep 1, 2017

smarterclayton Aug 30, 2017

smarterclayton Sep 1, 2017

piosz commented Sep 1, 2017

smarterclayton commented Sep 1, 2017

piosz commented Sep 1, 2017

piosz Sep 1, 2017

DirectXMan12 Sep 1, 2017

piosz Sep 1, 2017

piosz Sep 1, 2017

smarterclayton Sep 1, 2017

smarterclayton commented Sep 1, 2017

sjenning commented Sep 2, 2017

sjenning commented Sep 2, 2017

liggitt commented Sep 2, 2017

k8s-ci-robot commented Sep 5, 2017 •

edited

Loading

smarterclayton commented Sep 5, 2017

piosz commented Sep 6, 2017

piosz commented Sep 6, 2017

k8s-github-robot commented Sep 6, 2017

k8s-github-robot commented Sep 6, 2017

piosz commented Sep 6, 2017

		// +k8s:deepcopy-gen=package,register

		// +groupName=resource-metrics.metrics.k8s.io

Graduate metrics/v1alpha1 to v1beta1 #51653

Graduate metrics/v1alpha1 to v1beta1 #51653

Conversation

DirectXMan12 commented Aug 30, 2017 • edited Loading

DirectXMan12 commented Aug 30, 2017

smarterclayton commented Aug 30, 2017

DirectXMan12 commented Aug 31, 2017

DirectXMan12 commented Aug 31, 2017

DirectXMan12 commented Aug 31, 2017

smarterclayton commented Aug 31, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smarterclayton commented Sep 1, 2017

smarterclayton commented Sep 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

piosz commented Sep 1, 2017

smarterclayton commented Sep 1, 2017

piosz commented Sep 1, 2017

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

smarterclayton commented Sep 1, 2017

sjenning commented Sep 2, 2017

sjenning commented Sep 2, 2017

liggitt commented Sep 2, 2017

k8s-ci-robot commented Sep 5, 2017 • edited Loading

smarterclayton commented Sep 5, 2017

piosz commented Sep 6, 2017

piosz commented Sep 6, 2017

k8s-github-robot commented Sep 6, 2017

k8s-github-robot commented Sep 6, 2017

piosz commented Sep 6, 2017

DirectXMan12 commented Aug 30, 2017 •

edited

Loading

k8s-ci-robot commented Sep 5, 2017 •

edited

Loading