-
Notifications
You must be signed in to change notification settings - Fork 14
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
Bug 2087042: Rebase cloud-provider-vsphere 15.06.2022 #20
Conversation
Fix staticcheck error by cluster-api dependency.
Remove extra key name in chart release config
In case if multiple VMs with the same hostname exists within the same scope, picking wrong VM is quite possible, which might lead to hard-diagnosable issues with Nodes initialization, or even to incorrect Node initialization. This commit replaces `FindByDnsName` call to `FindAllByDnsName`, `GetVMByDNSName` function now returns an error in case if multiple VMs with the same HostName was found.
…-hostnames Return an error if multiple VMs with the same hostname was found
…elease Update helm chart release
IP address typo in secrets examples
Wrong vCenter IP in kubernetes-on-vsphere-with-kubeadm.md secrets example
Signed-off-by: Xudong Liu <xudongliuharold@gmail.com>
…dd-pod-priority-class-name Patch priorityClassName: system-node-critical in CPI daemonset
Signed-off-by: Xudong Liu <xudongl@vmware.com>
…eprecate-pod-priority-class Deprecate pod priority annotation in cpi deployment
Signed-off-by: Chen Lin <linch@vmware.com>
…st-coverage Add script for test coverage pipeline
Signed-off-by: Chen Lin <linch@vmware.com>
…st-coverage Refactor unit-test and test-cover in MAKEFILE
This patch updates the cluster and node selector for the paravirtual mode for the CPI. Signed-off-by: Sagar Muchhal <muchhals@vmware.com>
Return error when VM's info returned from VC doesn't contains IP address
In the `Zones and Regions for Pod and Volume Placement - CPI` section, there was a link to a "zones/regions tutorial" that sent the user to a nonexistent Markdown file (https://github.com/kubernetes/cloud-provider-vsphere/blob/master/docs/book/tutorials/deploying_cpi_and_csi_with_multi_dc_vc_aka_zones.md). I have replaced that broken link with a link to the most similar tutorial I could find in the docs: [Deploying the vSphere CPI and CSI in a Multi-vCenter OR Multi-Datacenter Environment using Zones](https://cloud-provider-vsphere.sigs.k8s.io/tutorials/deploying_cpi_with_multi_dc_vc_aka_zones.html).
Fixed broken link
Signed-off-by: Xudong Liu <xudongl@vmware.com>
…maintainer Add xudong liu as cloud provider vsphere maintainer
Fix the error wrapping
@lobziik: This pull request references Bugzilla bug 2087042, which is valid. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
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. |
Added Openshift specific builds scripts, linter/tests/etc runners. Extended makefile with OCP specific targets. Upstream version of lint, and fmt pollutes go.mod and go.sum files, so, own versions of such scripts was introduced.
Generated by `go mod vendor`
@lobziik: This pull request references Bugzilla bug 2087042, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
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. |
1 similar comment
@lobziik: This pull request references Bugzilla bug 2087042, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
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. |
@lobziik following on from our discussion today, i think it would be nice to note in the description the commit you are adding and what it's fixing. |
/retest |
@elmiko - this required more work around than i expected, i will create a follow up pr with envtest update. Lets leave just rebase and revendoring here. |
ack, thanks for the update @lobziik |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this generally looks correct to me
/lgtm
/retest |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JoelSpeed 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 |
@lobziik: Some pull requests linked via external trackers have merged: The following pull requests linked via external trackers have not merged:
These pull request must merge or be unlinked from the Bugzilla bug in order for it to move to the next state. Once unlinked, request a bug refresh with Bugzilla bug 2087042 has not been moved to the MODIFIED state. In response to this:
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. |
@lobziik: The following test failed, say
Full PR test history. Your PR dashboard. 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. |
This PR rebases the cloud-provider-vsphere openshift patches on top of the kubernetes/cloud-provider-vsphere master branch after bump dependencies to kube v1.24.1 release.
There are several commits that we carry on top of the upstream cloud-provider-vsphere and the rebase process allows us to preserve those. Here is a description of the process I used to create this PR.
(replicated old @elmiko's process within openshift/kubernetes-autoscaler)
Process
First we need to identify the carry commits that we currently have, this is done against our previous rebase (or fork in this case) to catch new changes. Once identified we will drop commits which have merged upstream and only carry unique commits. (see below for the carried and dropped commits).
Identify carry commits:
where
556ede
is the last upstream commit in our fork. This is thelist of commits we will need to apply onto the new upstream version of the
cloud-provider-vsphere. ideally, some of these commits can be dropped.
After identifying the carry commits, the next step is to create the new commit-tree that will be used for the rebase and then cherry pick the carry commits into the new branch. The following commands cover these steps:
With the
merge-15-06-2022
branch in place, I cherry picked the carry commits which we should carry.Commits to carry:
These commits are integral to our CI platform, or are specific to the releases we create for OpenShift.
Changes in commits:
Commits above was squashed into a single commit:
1fc9db07 UPSTREAM: <carry>: OCP specific build scripts and Dockerfile
Commit with dependencies vendoring was added on top of all.