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

vSphere infrastructure provider template #25

Merged
merged 30 commits into from
Sep 5, 2024

Conversation

eaudetcobello
Copy link
Contributor

@eaudetcobello eaudetcobello commented Jul 10, 2024

Gaps:

  • insecure-flag: true issue and sha1/sha256 thumbprint needs to be investigated. TLDR, there's conflicting documentation where the provider says the thumbprint needs to be in sha256 format, but providing the thumbprint in sha256 causes a "thumbprint mismatch" error. I have resolved this by using a sha1 thumbprint and setting insecure-flag: true. Need to try only setting insecure-flag: true with sha256, or removing insecure-flag: true and use sha1.

NOTE Do not merge before testing with multi-node cluster.
NOTE Verify if L297-301, L319-323, L293-295 are needed for multi-node cluster before merging.

nodes are tainted with node.cloudprovider.kubernetes.io/uninitialized
because we set cloudProvider: external.

this prevents some crucial pods from being scheduled (for example
we need the kube-vip pod to start on the first node so it can
announce the control plane IP)
set insecure-flag="true" to get around a possible thumbprint
verification bug in vsphere-csi driver. Also update csi driver
versions to v3.3.0 which support k8s v1.28-v1.30.
create manifest at /capi/manifests so it's applied after bootstrap
@eaudetcobello eaudetcobello marked this pull request as ready for review July 16, 2024 14:12
The provider acts weird when it comes to checking the thumbprint. The provider suggests to use sha256 for the thumbprint, but kubectl explain VCluster says that thumbprint expects a sha1 hash. So it's clear there's some conflicting information in the documentation.

Some experimenting needs to be done in regards to removing the insecure-flag and using sha256 or sha1 for the thumbprint.

The error can be seen in vsphere-cpi pods, the logs complain about "thumbprint does not match".

Right now there is no error because of a combination of insecure-flag: true and using sha1. Not sure which one of these solves the issue, but it's solved for now.
Copy link
Contributor

@neoaggelos neoaggelos left a comment

Choose a reason for hiding this comment

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

Immaculate work getting it to work! Let's clean up the rough edges!

templates/vsphere/cluster-template.yaml Outdated Show resolved Hide resolved
templates/vsphere/cluster-template.yaml Outdated Show resolved Hide resolved
templates/vsphere/cluster-template.yaml Outdated Show resolved Hide resolved
templates/vsphere/cluster-template.yaml Outdated Show resolved Hide resolved
templates/vsphere/cluster-template.yaml Outdated Show resolved Hide resolved
templates/vsphere/cluster-template.yaml Outdated Show resolved Hide resolved
templates/vsphere/cluster-template.yaml Outdated Show resolved Hide resolved
templates/vsphere/template-variables.rc Outdated Show resolved Hide resolved
templates/vsphere/template-variables.rc Show resolved Hide resolved
templates/vsphere/template-variables.rc Outdated Show resolved Hide resolved
This reverts commit b3ce6ea281cc13a65dca4431a6e5a97a2355278b.
use kube-vip rbac manifests instead of mounting the admin.conf file
the secret can be enabled/disabled by changing VSPHERE_PROXY_DISABLE in clusterctl.yaml
Copy link
Contributor

@neoaggelos neoaggelos left a comment

Choose a reason for hiding this comment

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

Looks great, did another pass

templates/vsphere/template-variables.rc Outdated Show resolved Hide resolved
templates/vsphere/template-variables.rc Outdated Show resolved Hide resolved
templates/vsphere/template-variables.rc Outdated Show resolved Hide resolved
templates/vsphere/template-variables.rc Outdated Show resolved Hide resolved
templates/vsphere/template-variables.rc Outdated Show resolved Hide resolved
@eaudetcobello eaudetcobello requested a review from a team as a code owner August 5, 2024 13:37
Copy link
Contributor

@bschimke95 bschimke95 left a comment

Choose a reason for hiding this comment

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

LGTM, waiting for @neoaggelos final approval

templates/vsphere/template-variables.rc Outdated Show resolved Hide resolved
templates/vsphere/template-variables.rc Outdated Show resolved Hide resolved
templates/vsphere/cluster-template.yaml Outdated Show resolved Hide resolved
templates/vsphere/cluster-template.yaml Outdated Show resolved Hide resolved
templates/vsphere/cluster-template.yaml Outdated Show resolved Hide resolved
templates/vsphere/cluster-template.yaml Outdated Show resolved Hide resolved
@eaudetcobello eaudetcobello changed the title vsphere cluster template vSphere infrastructure provider template Aug 15, 2024
Copy link
Contributor

@bschimke95 bschimke95 left a comment

Choose a reason for hiding this comment

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

LGTM

@eaudetcobello eaudetcobello merged commit 4b50cc8 into main Sep 5, 2024
3 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants