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.

Sign up for GitHub

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

CreateNodeSelectorPods should respect parameter #37538

Merged
merged 1 commit into from
Dec 27, 2016
Merged

CreateNodeSelectorPods should respect parameter #37538

merged 1 commit into from
Dec 27, 2016

Conversation

resouer
Copy link
Contributor

@resouer resouer commented Nov 28, 2016

Fix (1): CreateNodeSelectorPods should respect parameter id.

The existing e2e does not break because it happened use "node-selector" as id, which is the same as the hard coded value.

Fix (2): The current CreateNodeSelectorPods does not use nodeSelector parameter, it hard coded a label instead.

The reason current e2e does not influenced because we happened use the same label: https://github.com/kubernetes/kubernetes/blob/master/test/e2e/cluster_size_autoscaling.go#L177

Found these bugs during testing #36238

This change is Reviewable

Update cluster_size_autoscaling.go
3777777 
CreateNodeSelectorPods respects nodeSelector
@resouer resouer added area/test kind/bug Categorizes issue or PR as related to a bug. labels Nov 28, 2016
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 28, 2016
@resouer resouer mentioned this pull request Nov 28, 2016
Closed
@resouer resouer changed the base branch from fix-e2e-respect-id to master November 28, 2016 06:53
@k8s-github-robot k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note-label-needed labels Nov 28, 2016
@fejta fejta assigned gmarek and wojtek-t and unassigned fejta Nov 28, 2016
@gmarek gmarek assigned mwielgus and unassigned gmarek and wojtek-t Nov 28, 2016
@resouer
Copy link
Contributor Author

resouer commented Dec 5, 2016

ping @mwielgus any thoughts on this fix?

@mwielgus
Copy link
Contributor

LGTM

@mwielgus mwielgus added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 27, 2016
@mwielgus
Copy link
Contributor

@k8s-bot test this

@k8s-github-robot k8s-github-robot added the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Dec 27, 2016
@mwielgus mwielgus added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. labels Dec 27, 2016
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit a1265e0 into kubernetes:master Dec 27, 2016
@resouer resouer deleted the fix-e2e-respect-id branch December 27, 2016 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@mwielgus mwielgus

Labels
area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@resouer @mwielgus @k8s-github-robot @fejta @gmarek @wojtek-t @k8s-ci-robot