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

Moving the cloudprovider interface to staging. #68310

Merged
merged 1 commit into from Oct 5, 2018

Conversation

@cheftako
Member

cheftako commented Sep 6, 2018

What this PR does / why we need it:
Individual implementations are not yet being moved.
Fixed all dependencies which call the interface.
Fixed golint exceptions to reflect the move.
One of the first steps for https://github.com/kubernetes/community/blob/master/keps/sig-cloud-provider/0013-build-deploy-ccm.md

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes # KEP-0013

Special notes for your reviewer:

Release note:

Any external provider should be aware the cloud-provider interface should be imported from :-
cloudprovider "k8s.io/cloud-provider"
@cheftako

This comment has been minimized.

Show comment
Hide comment
@cheftako

cheftako Sep 19, 2018

Member

/test pull-kubernetes-e2e-gce-100-performance

Member

cheftako commented Sep 19, 2018

/test pull-kubernetes-e2e-gce-100-performance

@k8s-ci-robot k8s-ci-robot added size/XXL and removed size/XL labels Sep 20, 2018

@andrewsykim

This comment has been minimized.

Show comment
Hide comment
@andrewsykim
Member

andrewsykim commented Sep 27, 2018

@cheftako

This comment has been minimized.

Show comment
Hide comment
@cheftako

cheftako Sep 27, 2018

Member

/test pull-kubernetes-integration

Member

cheftako commented Sep 27, 2018

/test pull-kubernetes-integration

@cheftako

This comment has been minimized.

Show comment
Hide comment
@cheftako

cheftako Sep 27, 2018

Member

/test pull-kubernetes-e2e-gce-device-plugin-gpu

Member

cheftako commented Sep 27, 2018

/test pull-kubernetes-e2e-gce-device-plugin-gpu

@cheftako

This comment has been minimized.

Show comment
Hide comment
@cheftako

cheftako Sep 28, 2018

Member

/test pull-kubernetes-local-e2e-containerized

Member

cheftako commented Sep 28, 2018

/test pull-kubernetes-local-e2e-containerized

@cheftako

This comment has been minimized.

Show comment
Hide comment
@cheftako

cheftako Sep 28, 2018

Member

/test pull-kubernetes-integration

Member

cheftako commented Sep 28, 2018

/test pull-kubernetes-integration

@cheftako

This comment has been minimized.

Show comment
Hide comment
@cheftako

cheftako Sep 28, 2018

Member

/test pull-kubernetes-e2e-kops-aws

Member

cheftako commented Sep 28, 2018

/test pull-kubernetes-e2e-kops-aws

Moving the cloudprovider interface to staging.
Individual implementations are not yet being moved.
Fixed all dependencies which call the interface.
Fixed golint exceptions to reflect the move.
Added project info as per @dims and
https://github.com/kubernetes/kubernetes-template-project.
Added dims to the security contacts.
Fixed minor issues.
Added missing template files.
Copied ControllerClientBuilder interface to cp.
This allows us to break the only dependency on K8s/K8s.
Added TODO to ControllerClientBuilder.
Fixed GoDeps.
Factored in feedback from JustinSB.
@justinsb

This comment has been minimized.

Show comment
Hide comment
@justinsb

justinsb Oct 5, 2018

Member

Looks like you pulled the file mode changes out (thank you) and lost your lgtm because of it.

/lgtm

Member

justinsb commented Oct 5, 2018

Looks like you pulled the file mode changes out (thank you) and lost your lgtm because of it.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Oct 5, 2018

@dims

This comment has been minimized.

Show comment
Hide comment
@dims

dims Oct 5, 2018

Member

/test pull-kubernetes-local-e2e-containerized

Member

dims commented Oct 5, 2018

/test pull-kubernetes-local-e2e-containerized

@dims

This comment has been minimized.

Show comment
Hide comment
@dims

dims Oct 5, 2018

Member

/approve
/lgtm

Looks good! thanks @cheftako

Member

dims commented Oct 5, 2018

/approve
/lgtm

Looks good! thanks @cheftako

@dims

This comment has been minimized.

Show comment
Hide comment
@dims
Member

dims commented Oct 5, 2018

@@ -26,7 +26,7 @@ import (
"github.com/golang/glog"
// Cloud providers
"k8s.io/kubernetes/pkg/cloudprovider"
cloudprovider "k8s.io/cloud-provider"

This comment has been minimized.

@lavalamp

lavalamp Oct 5, 2018

Member

Is the "-" worth it? :)

@lavalamp

lavalamp Oct 5, 2018

Member

Is the "-" worth it? :)

## Compatibility
Cloud Providers are expected to keep the HEAD of their implementations in sync

This comment has been minimized.

@lavalamp

lavalamp Oct 5, 2018

Member

This doesn't sound like a reasonable request?

My (untested) goals for the apply repo look like this: https://github.com/kubernetes-sigs/structured-merge-diff/blob/master/RELEASE.md

@lavalamp

lavalamp Oct 5, 2018

Member

This doesn't sound like a reasonable request?

My (untested) goals for the apply repo look like this: https://github.com/kubernetes-sigs/structured-merge-diff/blob/master/RELEASE.md

## Things you should NOT do
1. Add an cloud provider specific code to this repo.

This comment has been minimized.

@lavalamp

lavalamp Oct 5, 2018

Member

s/an/any/

@lavalamp

lavalamp Oct 5, 2018

Member

s/an/any/

@lavalamp

This comment has been minimized.

Show comment
Hide comment
@lavalamp

lavalamp Oct 5, 2018

Member

/approve

I made some comments on the repo setup .md files; you can address in a followup. (Next time consider sending those first in a separate PR? Not that there should be a next time...)

Also I'll leave a hold in case you think it's worth removing the '-' from the package name; just unhold if not.

/hold

Member

lavalamp commented Oct 5, 2018

/approve

I made some comments on the repo setup .md files; you can address in a followup. (Next time consider sending those first in a separate PR? Not that there should be a next time...)

Also I'll leave a hold in case you think it's worth removing the '-' from the package name; just unhold if not.

/hold

@dims

This comment has been minimized.

Show comment
Hide comment
@dims

dims Oct 5, 2018

Member

/test pull-kubernetes-local-e2e-containerized

Member

dims commented Oct 5, 2018

/test pull-kubernetes-local-e2e-containerized

@dims

This comment has been minimized.

Show comment
Hide comment
@dims

dims Oct 5, 2018

Member

i am +1 to keeping the -

Member

dims commented Oct 5, 2018

i am +1 to keeping the -

@hogepodge

This comment has been minimized.

Show comment
Hide comment
@hogepodge

hogepodge Oct 5, 2018

Member

Also +1 on consistency. /lgtm

Member

hogepodge commented Oct 5, 2018

Also +1 on consistency. /lgtm

@saad-ali

This comment has been minimized.

Show comment
Hide comment
@saad-ali

saad-ali Oct 5, 2018

Member

/lgtm

Member

saad-ali commented Oct 5, 2018

/lgtm

@k8s-ci-robot

This comment has been minimized.

Show comment
Hide comment
@k8s-ci-robot

k8s-ci-robot Oct 5, 2018

Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cheftako, dims, hogepodge, justinsb, lavalamp, saad-ali

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Contributor

k8s-ci-robot commented Oct 5, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cheftako, dims, hogepodge, justinsb, lavalamp, saad-ali

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@lavalamp

This comment has been minimized.

Show comment
Hide comment
@lavalamp

lavalamp Oct 5, 2018

Member

OK in that case:

/hold cancel
/unhold

Member

lavalamp commented Oct 5, 2018

OK in that case:

/hold cancel
/unhold

@k8s-ci-robot

This comment has been minimized.

Show comment
Hide comment
@k8s-ci-robot

k8s-ci-robot Oct 5, 2018

Contributor

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

Test name Commit Details Rerun command
pull-kubernetes-local-e2e-containerized f3f46d5 link /test pull-kubernetes-local-e2e-containerized

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.

Contributor

k8s-ci-robot commented Oct 5, 2018

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

Test name Commit Details Rerun command
pull-kubernetes-local-e2e-containerized f3f46d5 link /test pull-kubernetes-local-e2e-containerized

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.

@k8s-ci-robot k8s-ci-robot merged commit 13705ac into kubernetes:master Oct 5, 2018

17 of 18 checks passed

pull-kubernetes-local-e2e-containerized Job failed.
Details
cla/linuxfoundation cheftako authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-e2e-kubeadm-gce Skipped
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
tide In merge pool.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment