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

[Client-gen] Replace client with clientset in kubelet and pkg/admission #20452

Merged

Conversation

caesarxuchao
Copy link
Member

@caesarxuchao caesarxuchao commented Feb 2, 2016

Depends on #20427.


This change is Reviewable

@caesarxuchao caesarxuchao added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Feb 2, 2016
@caesarxuchao caesarxuchao self-assigned this Feb 2, 2016
@k8s-github-robot
Copy link

Labelling this PR as size/XXL

@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/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 2, 2016
@caesarxuchao caesarxuchao assigned lavalamp and unassigned caesarxuchao Feb 2, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 2, 2016

GCE e2e build/test failed for commit 3d25469fd708f43ac546d9b5b68fef6d22a4b7e8.

@caesarxuchao caesarxuchao changed the title Replace client with clientset in kubelet Replace client with clientset in kubelet and pkg/admission Feb 2, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 2, 2016

GCE e2e build/test failed for commit 5f7481706c8eaa246da042e4942c3a310b72a0d9.

@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 2, 2016
@caesarxuchao caesarxuchao force-pushed the replace-client-kubelet branch from 5f74817 to 0b50135 Compare February 2, 2016 08:05
@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 2, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 2, 2016

GCE e2e test build/test passed for commit 0b50135109f37a962115544e4eeb96c8842e18d8.

@caesarxuchao
Copy link
Member Author

A flake:

I0202 08:27:59.890733    7948 server.go:177] Will report 172.17.0.3 as public IP address.
I0202 08:27:59.890818    7948 plugins.go:71] No cloud provider specified.
F0202 08:27:59.890917    7948 server.go:250] Invalid server address: group extensions has not been registered
!!! [0202 08:30:01] Timed out waiting for apiserver:  to answer at http://127.0.0.1:8080/healthz; tried 120 waiting 1 between each
!!! Error in ./hack/test-update-storage-objects.sh:48

https://console.developers.google.com/storage/browser/kubernetes-jenkins/pr-logs/pull/20452/kubernetes-pull-test-unit-integration/11988/?pli=1

@k8s-github-robot
Copy link

@caesarxuchao an issue is required for any manual rebuild. Expecting comment of the form 'github issue: #'
Open test flakes

@k8s-bot
Copy link

k8s-bot commented Feb 2, 2016

GCE e2e test build/test passed for commit 0b50135109f37a962115544e4eeb96c8842e18d8.

@caesarxuchao caesarxuchao reopened this Feb 2, 2016
@caesarxuchao caesarxuchao mentioned this pull request Feb 2, 2016
@caesarxuchao
Copy link
Member Author

@k8s-bot unit test this please. github issue: #20506

}
clientset.ExtensionsClient, err = extensions_unversioned.NewForConfig(c)
if err != nil {
return nil, err
return &clientset, err
Copy link
Member Author

Choose a reason for hiding this comment

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

We let caller of this function to decide if to fail on error, because it's possible that Extensions are disabled through KUBE_API_VERSIONS, but the user still wants to use other groups' clients of the clientset.

@k8s-bot
Copy link

k8s-bot commented Feb 2, 2016

GCE e2e build/test failed for commit bde0336b38cf78fd69db96387b9bb414045fabe5.

@caesarxuchao
Copy link
Member Author

Not sure if e2e is a flake:

Kubernetes e2e suite.Cluster level logging using Elasticsearch should check that logs from pods on all nodes are ingested into Elasticsearch

@k8s-github-robot
Copy link

@caesarxuchao you must link to the test flake issue when requesting a manual test.
Expecting comment of the form '@k8s-bot test this github issue: #'
Open test flakes

@k8s-bot
Copy link

k8s-bot commented Feb 2, 2016

GCE e2e build/test failed for commit bde0336b38cf78fd69db96387b9bb414045fabe5.

@k8s-bot
Copy link

k8s-bot commented Feb 2, 2016

GCE e2e test build/test passed for commit bde0336b38cf78fd69db96387b9bb414045fabe5.

@caesarxuchao
Copy link
Member Author

@lavalamp all tests passed, it's ready for review. Thanks!

@caesarxuchao caesarxuchao added priority/backlog Higher priority than priority/awaiting-more-evidence. and removed priority/backlog Higher priority than priority/awaiting-more-evidence. labels Feb 3, 2016
@lavalamp
Copy link
Member

lavalamp commented Feb 3, 2016

Just some nits.

@caesarxuchao caesarxuchao force-pushed the replace-client-kubelet branch from bde0336 to cddd7b5 Compare February 3, 2016 04:29
@caesarxuchao
Copy link
Member Author

I've addressed all the comments and squashed the commits. Self applying the LGTM. Thanks.

@caesarxuchao caesarxuchao added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Feb 3, 2016
@caesarxuchao
Copy link
Member Author

Increase the priority to see if it gets merged more quickly...

@caesarxuchao caesarxuchao added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 3, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 3, 2016

GCE e2e test build/test passed for commit cddd7b5.

@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 3, 2016

GCE e2e test build/test passed for commit cddd7b5.

@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 3, 2016

GCE e2e test build/test passed for commit cddd7b5.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

k8s-github-robot pushed a commit that referenced this pull request Feb 3, 2016
@k8s-github-robot k8s-github-robot merged commit 843c11e into kubernetes:master Feb 3, 2016
@@ -173,7 +175,7 @@ type SchedulerServer struct {
conntrackTCPTimeoutEstablished int

executable string // path to the binary running this service
client *client.Client
client *clientset.Clientset
Copy link
Contributor

Choose a reason for hiding this comment

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

this change broke event sending from our scheduler: #20783

@caesarxuchao caesarxuchao changed the title Replace client with clientset in kubelet and pkg/admission [Client-gen] Replace client with clientset in kubelet and pkg/admission Aug 31, 2016
openshift-publish-robot pushed a commit to openshift/kubernetes that referenced this pull request Aug 8, 2018
UPSTREAM 64896: kubectl: wait for all errors and successes on podEviction

Origin-commit: 55f97bfa79891e6dd5a3aaa41b794842328f6e31
openshift-publish-robot pushed a commit to openshift/kubernetes that referenced this pull request Jan 29, 2019
UPSTREAM 64896: kubectl: wait for all errors and successes on podEviction

Origin-commit: 55f97bfa79891e6dd5a3aaa41b794842328f6e31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants