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

Fix kube vsphere.kerneltime #34997

Merged

Conversation

kerneltime
Copy link

@kerneltime kerneltime commented Oct 17, 2016

What this PR does / why we need it:
This fixes kube-up to correctly install and configure on vSphere and avoid panics when only a single ESX(hypervisor) is used instead of a cluster.

Which issue this PR fixes
fixes #34992
fixes #34847

Fix kube vsphere.kerneltime

Special notes for your reviewer:

We plan to cherry pick this into 1.4 release branch as well Ref: #34993


This change is Reviewable

Ritesh H Shukla added 2 commits October 17, 2016 23:40
Use ComputeResource instead of ClusterComputeResource when
initializing the vSphere Cloud Provider
@k8s-github-robot k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note-label-needed labels Oct 18, 2016
Copy link

@pdhamdhere pdhamdhere left a comment

Choose a reason for hiding this comment

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

LGTM.

@kerneltime
Copy link
Author

@zmerlynn zmerlynn added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Oct 18, 2016
@zmerlynn zmerlynn assigned zmerlynn and unassigned roberthbailey Oct 18, 2016
@zmerlynn
Copy link
Member

This doesn't look like the same set of changes you had in #34993. Were those changes already in another PR? If so, you'll need to cherry-pick them both (and you may want to note that on the other PR).

https://github.com/kubernetes/kubernetes/blob/master/docs/devel/cherry-picks.md

@k8s-cherrypick-bot
Copy link

Removing label cherrypick-candidate because no release milestone was set. This is an invalid state and thus this PR is not being considered for cherry-pick to any release branch. Please add an appropriate release milestone and then re-add the label.

@kerneltime
Copy link
Author

@zmerlynn Those changes are already part of the master. So with these 2 commits the master will have all the commits that were part of #34993
I will then cherry pick all the changes from master into release 1.4

A longer explanation, #34993 was a combination of 3 cherry-picks and 2 new commits. Thus, when targeting the change for master I only need 2 commits (here) since the 3 were already in (#31467)

@kerneltime
Copy link
Author

kerneltime commented Oct 18, 2016

Understood. This PR does need to be cherry picked. I will request the other PR (#31467) to be cherry picked as well. Thank you!

@kerneltime
Copy link
Author

@zmerlynn Do I comment on #31467 to have it cherry-picked or do I start a new PR? Sorry for the confusion.

@imkin
Copy link

imkin commented Oct 18, 2016

@kerneltime "cherrypick-candidate" and milestone 1.4 should suffice I believe. Once the tests pass adding a cherrypick-approved will tag this for cherrypicking into 1.4 release

@kerneltime
Copy link
Author

For #31467 and this PR I do not have ability to change labels or tag..

@roberthbailey
Copy link
Contributor

@kerneltime if you can't add labels, please request that your reviewer add them for you.

@zmerlynn zmerlynn added lgtm "Looks good to me", indicates that a PR is ready to be merged. retest-not-required labels Oct 18, 2016
@zmerlynn
Copy link
Member

@kerneltime: After this PR gets merged and cherrypick approved, you can propose the cherrypick yourself (see the link from #34997 (comment)). You can roll multiple PRs into one cherrypick, so in this case, you'd do e.g.:

hack/cherry_pick_pull.sh upstream/release-1.4 31467 34997

This PR should go in shortly. I labeled it as retest-not-required since we have no PR-builder coverage for vSphere (yet), so the submit queue should pick it up next pass.

@kerneltime
Copy link
Author

@zmerlynn Got it. Thank you!

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 01c31b3 into kubernetes:master Oct 18, 2016
@jessfraz jessfraz added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Oct 18, 2016
@kerneltime
Copy link
Author

Cherry pick PR #35059

@kerneltime kerneltime deleted the fix-kube-vsphere.kerneltime branch October 18, 2016 19:59
k8s-github-robot pushed a commit that referenced this pull request Oct 18, 2016
…34997-origin-release-1.4

Automatic merge from submit-queue

Automated cherry pick of #31467 #34997 

Cherry pick of  #31467 #34997 on release-1.4

#34997: Fix kube vsphere.kerneltime
#31467: Add support for vpshere cloud provider in kubeup

Ref: 34997 for discussion.
shyamjvs pushed a commit to shyamjvs/kubernetes that referenced this pull request Dec 1, 2016
…-of-#31467-kubernetes#34997-origin-release-1.4

Automatic merge from submit-queue

Automated cherry pick of kubernetes#31467 kubernetes#34997 

Cherry pick of  kubernetes#31467 kubernetes#34997 on release-1.4

kubernetes#34997: Fix kube vsphere.kerneltime
kubernetes#31467: Add support for vpshere cloud provider in kubeup

Ref: 34997 for discussion.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vSphere Cloud Provider panics in the absence of a cluster Kube-up for vsphere is broken due to a salt bug
9 participants