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

Comment out NodeAffinity.RequiredDuringSchedulingRequiredDuringExecution #21057

Merged
merged 1 commit into from
Feb 15, 2016
Merged

Comment out NodeAffinity.RequiredDuringSchedulingRequiredDuringExecution #21057

merged 1 commit into from
Feb 15, 2016

Conversation

davidopp
Copy link
Member

because it's not yet implemented.

@wojtek-t to review

cc/ @bgrant0607 @kevin-wangzefeng

@k8s-github-robot k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 11, 2016
@k8s-github-robot
Copy link

Labelling this PR as size/M

@k8s-bot
Copy link

k8s-bot commented Feb 11, 2016

GCE e2e test build/test passed for commit 4d0eed1e53b1b0855f0e09a8112d6583645143bd.

@wojtek-t
Copy link
Member

@davidopp

hack/verify-gofmt.sh
!!! 'gofmt -s' needs to be run on the following files:
./plugin/pkg/scheduler/algorithm/predicates/predicates.go

Can you please fix?

@wojtek-t
Copy link
Member

Once fixed - LGTM

@kevin-wangzefeng
Copy link
Member

@davidopp Thanks for the fix, there are also tests that uses NodeAffinity.RequiredDuringSchedulingRequiredDuringExecution in validation_test.go, predicates_test.go, test/e2e/scheduler_predicates.go, shall we comment out or change them?

@davidopp
Copy link
Member Author

Thanks for noticing those, @kevin-wangzefeng . I should have used case-insensitive grep (or remembered that we actually have tests...)

Also ran 'gofmt -s'

@wojtek-t / @kevin-wangzefeng PTAL.

@k8s-github-robot
Copy link

Labelling this PR as size/L

@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 11, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 11, 2016

GCE e2e test build/test passed for commit ad921930a0324e48a70e2bb91cd1aa3938c9e563.

@wojtek-t
Copy link
Member

@davidopp - seems that there is still one issue:

=== RUN TestValidatePod
--- FAIL: TestValidatePod (0.00s)
validation_test.go:1786: expected success: [metadata.annotations: Invalid value: "scheduler.alpha.kubernetes.io/affinity": invalid character '/' looking for beginning of object key string]

in pkg/api/validation

Can you please take a look?

@k8s-bot
Copy link

k8s-bot commented Feb 12, 2016

GCE e2e test build/test passed for commit b51bfa5812f90cac88a76b1bbcfebea302309cd9.

@davidopp
Copy link
Member Author

@k8s-bot ok to test

@k8s-bot
Copy link

k8s-bot commented Feb 13, 2016

GCE e2e test build/test passed for commit b51bfa5812f90cac88a76b1bbcfebea302309cd9.

@k8s-bot
Copy link

k8s-bot commented Feb 13, 2016

GCE e2e build/test failed for commit b51bfa5812f90cac88a76b1bbcfebea302309cd9.

@k8s-github-robot
Copy link

@davidopp
You must link to the test flake issue which caused you to request this manual re-test.
Re-test requests should be in the form of: k8s-bot test this issue: #<number>
Here is the list of open test flakes.

@k8s-github-robot
Copy link

PR needs rebase

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 13, 2016
@davidopp
Copy link
Member Author

@k8s-bot test this issue: #21220

@k8s-bot
Copy link

k8s-bot commented Feb 14, 2016

GCE e2e build/test failed for commit b51bfa5812f90cac88a76b1bbcfebea302309cd9.

@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 14, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 14, 2016

GCE e2e test build/test passed for commit 3c05f9c5cd13f4dc4c163610aaca97611341ca24.

@davidopp davidopp added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 14, 2016
@davidopp
Copy link
Member Author

@k8s-bot test this issue: #IGNORE

@k8s-bot
Copy link

k8s-bot commented Feb 14, 2016

GCE e2e test build/test passed for commit 3c05f9c5cd13f4dc4c163610aaca97611341ca24.

@davidopp
Copy link
Member Author

@k8s-bot unit test this please issue: #IGNORE

@k8s-github-robot
Copy link

PR changed after LGTM, removing LGTM.

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 15, 2016
@davidopp davidopp added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 15, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 15, 2016

GCE e2e build/test failed for commit 66368ef.

@davidopp
Copy link
Member Author

@k8s-bot e2e test this issue: #19997

@k8s-bot
Copy link

k8s-bot commented Feb 15, 2016

GCE e2e test build/test passed for commit 66368ef.

@k8s-github-robot
Copy link

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

@k8s-bot
Copy link

k8s-bot commented Feb 15, 2016

GCE e2e test build/test passed for commit 66368ef.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

k8s-github-robot pushed a commit that referenced this pull request Feb 15, 2016
Merge pull request #21057 from davidopp/work1
4ff418c 
Auto commit by PR queue bot
@k8s-github-robot k8s-github-robot merged commit 4ff418c into kubernetes:master Feb 15, 2016
mikedanese
mikedanese reviewed Feb 16, 2016
View reviewed changes
pkg/api/deep_copy_generated.go
}
} else {
out.RequiredDuringSchedulingRequiredDuringExecution = nil
}
Copy link
Member

Choose a reason for hiding this comment

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

Did you delete this manually? I'm trying to debug some weirdness I'm noticing with ./hack/update-generated-deep-copies.sh

Copy link
Member Author

Choose a reason for hiding this comment

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

No I didn't edit any of the generated files manually.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, I opened #21338

openshift-publish-robot pushed a commit to openshift/kubernetes that referenced this pull request Sep 21, 2018
Merge pull request kubernetes#21057 from mfojtik/fix-component-instal…
f60e56e 
…l-race-comment

UPSTREAM: <drop>: comment out componentconfig registration for kube controller manager

Origin-commit: 8a1c021cbd254b7337020d3b3cc9565da6dc4122
@chendave chendave mentioned this pull request Sep 10, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@wojtek-t wojtek-t

Labels
kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@davidopp @k8s-github-robot @k8s-bot @wojtek-t @kevin-wangzefeng @mikedanese @k8s-teamcity-mesosphere @googlebot