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

Jump to bottom

Bumped Metrics Server to v0.2.0 #52548

Merged
merged 1 commit into from Sep 16, 2017
Merged

Bumped Metrics Server to v0.2.0 #52548

merged 1 commit into from Sep 16, 2017

Conversation

piosz
Copy link
Member

@piosz piosz commented Sep 15, 2017 •
edited

ref kubernetes/enhancements#271

Release note:

Introduced Metrics Server in version v0.2.0. For more details see https://github.com/kubernetes-incubator/metrics-server/releases/tag/v0.2.0.

@piosz piosz added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Sep 15, 2017
@piosz piosz added this to the v1.8 milestone Sep 15, 2017
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 15, 2017
@piosz
Copy link
Member Author

piosz commented Sep 15, 2017

Testing the change locally also.

@piosz
Copy link
Member Author

piosz commented Sep 15, 2017

/test pull-kubernetes-e2e-gce-bazel

@piosz
Copy link
Member Author

piosz commented Sep 15, 2017

cc @kubernetes/sig-instrumentation-pr-reviews

@k8s-ci-robot k8s-ci-robot added the sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. label Sep 15, 2017
@DirectXMan12
Copy link
Contributor

/lgtm

(as long as it passes tests, and everything)

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

k8s-ci-robot commented Sep 15, 2017 •
edited

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

Test name Commit Details Rerun command
pull-kubernetes-e2e-gce-bazel c632649 link /test pull-kubernetes-e2e-gce-bazel

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.

@DirectXMan12 DirectXMan12 mentioned this pull request Sep 15, 2017
Closed
@piosz piosz unassigned luxas Sep 15, 2017
@piosz
Copy link
Member Author

piosz commented Sep 15, 2017

@eparis @roberthbailey could you please approve?

@piosz
Copy link
Member Author

piosz commented Sep 15, 2017

/test pull-kubernetes-unit

luxas
luxas reviewed Sep 15, 2017
View reviewed changes
Copy link
Member

@luxas luxas left a comment

Choose a reason for hiding this comment

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

Half-LGTM
A bit unsure about the deployment naming

cluster/addons/metrics-server/metrics-server-deployment.yaml
@@ -10,28 +10,31 @@ metadata:
apiVersion: extensions/v1beta1
kind: Deployment
metadata:
name: metrics-server
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, why do this? This a Deployment, not a ReplicationController...

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe it's to appease the arcane rules followed by the addon manager (it will only apply changes when it sees metadata changes, and it's easy to change the name).

Copy link
Member Author

Choose a reason for hiding this comment

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

TBH I don't know what is the current logic of addon-manager so I just followed Heapster pattern.

Copy link
Member

Choose a reason for hiding this comment

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

Ok

@roberthbailey
Copy link
Contributor

/lgtm
/approve

@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

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

Associated issue: 271

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

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

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 k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 15, 2017
@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 52488, 52548). If you want to cherry-pick this change to another branch, please follow the instructions here..

@k8s-github-robot k8s-github-robot merged commit 0f7aa67 into kubernetes:master Sep 16, 2017
@piosz piosz deleted the ms-bump branch September 16, 2017 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@luxas luxas luxas left review comments

@roberthbailey roberthbailey roberthbailey left review comments

Assignees

@DirectXMan12 DirectXMan12

@roberthbailey roberthbailey

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. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v1.8
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@piosz @DirectXMan12 @k8s-ci-robot @roberthbailey @k8s-github-robot @luxas