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

Add support for admission controller based on namespace node selectors. #24980

Merged
merged 2 commits into from
Oct 15, 2016
Merged

Add support for admission controller based on namespace node selectors. #24980

merged 2 commits into from
Oct 15, 2016

Conversation

aveshagarwal
Copy link
Member

@aveshagarwal aveshagarwal commented Apr 29, 2016 •
edited by k8s-oncall
Loading

This work is to upstream openshift's project node selectors based admission controller.

Fixes #17151

This change is Reviewable

@aveshagarwal
Copy link
Member Author

@kubernetes/rh-cluster-infra

@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Apr 29, 2016
@aveshagarwal aveshagarwal force-pushed the master-project-node-selector branch from c6482cb to ab15a87 Compare April 29, 2016 14:49
@aveshagarwal aveshagarwal mentioned this pull request Apr 29, 2016
Closed
@aveshagarwal aveshagarwal force-pushed the master-project-node-selector branch 4 times, most recently from 5dc8819 to ad81af8 Compare May 2, 2016 18:21
@aveshagarwal
Copy link
Member Author

Added test cases for namespace cahce and labels.

DirectXMan12
DirectXMan12 reviewed May 2, 2016
View reviewed changes
pkg/labels/selector.go
}

// Merge combines given maps
// Note: It doesn't not check for any conflicts between the maps
Copy link
Contributor

@DirectXMan12 DirectXMan12 May 2, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Nit: You should note that the second map wins when conflicts do occur.

Copy link
Member Author

Choose a reason for hiding this comment

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

Here it is just to find out if a conflict occurs (true or false) that means if a same key exists, then their values are same or not. Not sure what do you mean by win here?

Copy link
Member Author

@aveshagarwal aveshagarwal May 2, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

@DirectXMan12 nvm I was thinking about conflicts.

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

@googlebot googlebot added cla: no and removed cla: yes labels May 2, 2016
@aveshagarwal aveshagarwal force-pushed the master-project-node-selector branch from 096e4f7 to 1635272 Compare May 2, 2016 20:45
@googlebot
Copy link

CLAs look good, thanks!

@googlebot googlebot added cla: yes and removed cla: no labels May 2, 2016
@aveshagarwal
Copy link
Member Author

@DirectXMan12 thanks. Updated.

@aveshagarwal aveshagarwal force-pushed the master-project-node-selector branch 4 times, most recently from b29c374 to ec9962f Compare May 3, 2016 19:50
@k8s-github-robot k8s-github-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 3, 2016
@aveshagarwal aveshagarwal force-pushed the master-project-node-selector branch from ec9962f to adb2fe9 Compare May 3, 2016 21:50
@aveshagarwal aveshagarwal force-pushed the master-project-node-selector branch from 137379f to 6fec17b Compare September 30, 2016 20:59
@aveshagarwal
Copy link
Member Author

@k8s-bot kubemark e2e test this

@aveshagarwal
Copy link
Member Author

@derekwaynecarr updated it. PTAL.

@k8s-ci-robot
Copy link
Contributor

Jenkins Kubemark GCE e2e failed for commit 6fec17bbc4031c2b8e0a9a0e57f96e76d22fdc39. Full PR test history.

The magic incantation to run this job again is @k8s-bot kubemark e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@aveshagarwal
Copy link
Member Author

The following errors dont seem related to a flake.
@k8s-bot kubemark e2e test this

@aveshagarwal
Copy link
Member Author

@derekwaynecarr @smarterclayton Would appreciate if you could tell me if anything else is needed to get it in?

@davidopp
Copy link
Member

Per our discussion at the SIG meeting today, this doesn't need an LGTM from me, feel free to merge once the appropriate RedHat people are happy with it.

@derekwaynecarr
Copy link
Member

Reviewing this now.

@derekwaynecarr derekwaynecarr added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 12, 2016
@derekwaynecarr
Copy link
Member

LGTM

@aveshagarwal
Copy link
Member Author

@k8s-bot test this issue: #IGNORE

@k8s-ci-robot
Copy link
Contributor

Jenkins unit/integration failed for commit 6fec17bbc4031c2b8e0a9a0e57f96e76d22fdc39. Full PR test history.

The magic incantation to run this job again is @k8s-bot unit test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@aveshagarwal
Copy link
Member Author

the pr #32555 merged and changed the signature of admission.NewPluginInitializer which is now breaking the build of this admission controller. Will fix it soon.

Avesh Agarwal added 2 commits October 13, 2016 13:14
Add support for admission controller based on namespace node selectors.
b902f85 
This work is to upstream openshift's project node selectors based admission controller.

Addresses kubernetes#17151.
Added new unit tests.
5bb2cb8
@aveshagarwal aveshagarwal force-pushed the master-project-node-selector branch from 6fec17b to 5bb2cb8 Compare October 13, 2016 17:15
@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 13, 2016
@aveshagarwal
Copy link
Member Author

@derekwaynecarr this pr needs LGTM again.

@derekwaynecarr derekwaynecarr added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 14, 2016
@k8s-github-robot
Copy link

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

@k8s-ci-robot
Copy link
Contributor

Jenkins GKE smoke e2e failed for commit 5bb2cb8. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 19848dd into kubernetes:master Oct 15, 2016
@aveshagarwal aveshagarwal deleted the master-project-node-selector branch November 7, 2016 14:29
@resouer resouer mentioned this pull request Dec 20, 2017
Closed
@sanposhiho sanposhiho mentioned this pull request Nov 7, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smarterclayton smarterclayton smarterclayton left review comments

@derekwaynecarr derekwaynecarr derekwaynecarr approved these changes

Assignees

@derekwaynecarr derekwaynecarr

Labels
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. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
v1.5
Development

Successfully merging this pull request may close these issues.