-
Notifications
You must be signed in to change notification settings - Fork 299
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
Vendor aws #111
Vendor aws #111
Conversation
This commit takes the code from k8s.io/staging/src/k8s.io/legacy-cloud-providers at 2e7996e3e271, and vendors it directly into legacy-cloud-providers. The reason for doing this is that k8s.io/staging/src/k8s.io/legacy-cloud-providers is actually a vendored version of k8s.io/legacy-cloud-providers, which lives on github as http://github.com/kubernetes/legacy-cloud-providers. These three levels of indirection make figuring out what's going on difficult. This moves all the relevant code into this repository, so that one commit can effect the result directly. This commit should not change the resulting functionality of the binary. Commit: commit 2e7996e3e2712684bc73f0dec0200d64eec7fe40 (HEAD, tag: v1.18.3) Author: Anago GCB <nobody@k8s.io> Date: Wed May 20 12:42:40 2020 +0000 Release commit for Kubernetes v1.18.3
The previous commit vendors the cloud provider repository 1:1 without any changes. This thins out the vendored bits to only the requisite code.
@sargun: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. 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. |
Welcome @sargun! |
Hi @sargun. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sargun The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This hoists the code under the module legacy-cloud-providers/ to ./aws. In turn it locks all the recursive dependencies to kubernetes v0.18.3.
/ok-to-test |
I appreciate this, but we're not ready to move the cloud provider code from legacy-cloud-providers quite yet. When we do, we will stop accepting commits there, but we only want one source of truth for now. |
@nckturner I'm curious why, and when? Depending on the answer to this, we will consider forking, or waiting for the change. Right now, it's fairly difficult for us to manage the current process. |
@sargun That's fair. ASAP, ideally. Maybe we can get your changes merged upstream into k/k, or look into @andrewsykim's V2 proposal. Thirdly, you're right, we could copy the code over here. I'd like to have the timeline concrete if we do that, and know exactly when we are deprecating the in-tree provider. I'll bring it up tomorrow in the provider-aws meeting. |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
It makes it so all the code is in one place.
Which issue(s) this PR fixes:
This addresses some of the concerns in #42 (comment).
Does this PR introduce a user-facing change?:
NONE