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

Drop cloud-controller-manager artifacts from k/k release #81029

Conversation

@dims
Copy link
Member

commented Aug 6, 2019

cloud-controller-manager was supposed to be a temporary artifact for making external cloud providers possible. But we end up shipping binaries/man pages etc. Let's stop doing that now in 1.16 so we can deprecate and remove this binary.

What type of PR is this?
/kind cleanup

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #81027

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

cloud-controller-manager binaries and docker images are no longer shipped with kubernetes releases.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:


@dims

This comment has been minimized.

Copy link
Member Author

commented Aug 6, 2019

/sig release
/sig cloud-provider

@dims

This comment has been minimized.

Copy link
Member Author

commented Aug 6, 2019

/priority important-soon

@dims

This comment has been minimized.

Copy link
Member Author

commented Aug 6, 2019

/test pull-kubernetes-integration

@BenTheElder

This comment has been minimized.

Copy link
Member

commented Aug 6, 2019

pod pending timeout 🙄 (prow)
/retest

@dims

This comment has been minimized.

Copy link
Member Author

commented Aug 6, 2019

@dims

This comment has been minimized.

Copy link
Member Author

commented Aug 6, 2019

@BenTheElder

This comment has been minimized.

Copy link
Member

commented Aug 6, 2019

build changes look good, should this also be removed from hyperkube?

@BenTheElder
Copy link
Member

left a comment

/approve
/lgtm
for build & hack

/hold
for discussion

@dims

This comment has been minimized.

Copy link
Member Author

commented Aug 7, 2019

/test pull-kubernetes-kubemark-e2e-gce-big

1 similar comment
@dims

This comment has been minimized.

Copy link
Member Author

commented Aug 7, 2019

/test pull-kubernetes-kubemark-e2e-gce-big

@thockin
Copy link
Member

left a comment

I guess I am OK with this, if we're confident that nobody is using this. Are we? How are we?

What about the code in cmd/... and maybe elsewhere?

@dims

This comment has been minimized.

Copy link
Member Author

commented Aug 7, 2019

@thockin the code in cmd/ can be used as a template for folks who are getting started to write their own stuff. I will remove the references in hyperkube later. I want this focused on just the artifacts that we ship.

@thockin

This comment has been minimized.

Copy link
Member

commented Aug 7, 2019

Should we move cmd to staging/.../cloud-controller-example or something?

Thanks!

/lgtm
/approve

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Aug 7, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BenTheElder, dims, thockin

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

@dims

This comment has been minimized.

Copy link
Member Author

commented Aug 8, 2019

/hold cancel

@k8s-ci-robot k8s-ci-robot merged commit 62f1c40 into kubernetes:master Aug 8, 2019

23 checks passed

cla/linuxfoundation dims authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Job succeeded.
Details
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-csi-serial Skipped.
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gce-iscsi Skipped.
pull-kubernetes-e2e-gce-iscsi-serial Skipped.
pull-kubernetes-e2e-gce-storage-slow Skipped.
pull-kubernetes-godeps 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-node-e2e-containerd Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details

@k8s-ci-robot k8s-ci-robot added this to the v1.16 milestone Aug 8, 2019

@andrewsykim

This comment has been minimized.

Copy link
Member

commented Aug 8, 2019

Should we move cmd to staging/.../cloud-controller-example or something?

I'd like to move cmd/cloud-controller-manager to staging, but the tricky thing is that it imports controllers in pkg/controllers which depends on a bunch of internal imports.

One thing we could do is remove internal deps in cloud specific controllers (service, route, cloud node lifecylce, etc) to somewhere like k8s.io/cloud-provider/controllers (in staging) and that would unblock us from moving cmd/cloud-controller-manager to staging as well. @thockin @dims are you opposed to that idea?

@dims

This comment has been minimized.

Copy link
Member Author

commented Aug 8, 2019

@andrewsykim I like that idea. please investigate that a bit more.

@andrewsykim

This comment has been minimized.

Copy link
Member

commented Aug 8, 2019

Sounds good, opened an issue here #81172 - will update that issue with next steps

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.