From 58e9cba9b1e4cefc15d0772c2835813f0db7a58a Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Mon, 17 Mar 2025 09:27:10 +0100 Subject: [PATCH 1/4] vspherecluster: implement Ready, FailureDomainsReady, ClusterModulesReady and VCenterAvailable v1beta2 conditions for govmomi --- .golangci.yml | 4 - apis/v1beta1/vspherecluster_types.go | 74 ++++++++- ...ture.cluster.x-k8s.io_vsphereclusters.yaml | 2 +- controllers/clustermodule_reconciler.go | 18 +++ controllers/vspherecluster_reconciler.go | 141 ++++++++++++++++-- pkg/context/cluster_context.go | 16 -- pkg/session/error.go | 40 ----- pkg/session/session.go | 4 +- 8 files changed, 226 insertions(+), 73 deletions(-) delete mode 100644 pkg/session/error.go diff --git a/.golangci.yml b/.golangci.yml index d714cb5b72..c742366a1a 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -214,10 +214,6 @@ issues: - linters: - staticcheck text: "SA1019: \"sigs.k8s.io/cluster-api-provider-vsphere/apis/(v1alpha3|v1alpha4)\" is deprecated: This package will be removed in one of the next releases." - # Deprecations for Cluster API's predicates.ClusterUnpausedAndInfrastructureReady - - linters: - - staticcheck - text: "SA1019: predicates.ClusterUnpausedAndInfrastructureReady is deprecated: This predicate is deprecated and will be removed in a future version, use ClusterPausedTransitionsOrInfrastructureReady instead." - linters: - revive text: "exported: exported method .*\\.(Reconcile|SetupWithManager|SetupWebhookWithManager) should have comment or be unexported" diff --git a/apis/v1beta1/vspherecluster_types.go b/apis/v1beta1/vspherecluster_types.go index ddf7df673a..2a363f62e5 100644 --- a/apis/v1beta1/vspherecluster_types.go +++ b/apis/v1beta1/vspherecluster_types.go @@ -28,6 +28,78 @@ const ( ClusterFinalizer = "vspherecluster.infrastructure.cluster.x-k8s.io" ) +// VSphereCluster's Ready condition and corresponding reasons that will be used in v1Beta2 API version. +const ( + // VSphereClusterReadyV1Beta2Condition is true if the VSphereCluster's deletionTimestamp is not set, VSphereCluster's + // FailureDomainsReady, VCenterAvailable and ClusterModulesReady conditions are true. + VSphereClusterReadyV1Beta2Condition = clusterv1.ReadyV1Beta2Condition + + // VSphereClusterReadyV1Beta2Reason surfaces when the VSphereCluster readiness criteria is met. + VSphereClusterReadyV1Beta2Reason = clusterv1.ReadyV1Beta2Reason + + // VSphereClusterNotReadyV1Beta2Reason surfaces when the VSphereCluster readiness criteria is not met. + VSphereClusterNotReadyV1Beta2Reason = clusterv1.NotReadyV1Beta2Reason + + // VSphereClusterReadyUnknownV1Beta2Reason surfaces when at least one VSphereCluster readiness criteria is unknown + // and no VSphereCluster readiness criteria is not met. + VSphereClusterReadyUnknownV1Beta2Reason = clusterv1.ReadyUnknownV1Beta2Reason +) + +// VSphereCluster's FailureDomainsReady condition and corresponding reasons that will be used in v1Beta2 API version. +const ( + // VSphereClusterFailureDomainsReadyV1Beta2Condition documents the status of failure domains for a VSphereCluster. + VSphereClusterFailureDomainsReadyV1Beta2Condition = "FailureDomainsReady" + + // VSphereClusterFailureDomainsReadyV1Beta2Reason surfaces when the failure domains for a VSphereCluster are ready. + VSphereClusterFailureDomainsReadyV1Beta2Reason = clusterv1.ReadyV1Beta2Reason + + // VSphereClusterFailureDomainsWaitingForFailureDomainStatusV1Beta2Reason surfaces when not all VSphereFailureDomains for a VSphereCluster are ready. + VSphereClusterFailureDomainsWaitingForFailureDomainStatusV1Beta2Reason = "WaitingForFailureDomainStatus" + + // VSphereClusterFailureDomainsNotReadyV1Beta2Reason surfaces when the failure domains for a VSphereCluster are not ready. + VSphereClusterFailureDomainsNotReadyV1Beta2Reason = clusterv1.NotReadyV1Beta2Reason + + // VSphereClusterFailureDomainsDeletingV1Beta2Reason surfaces when the failure domains for a VSphereCluster are being deleted. + VSphereClusterFailureDomainsDeletingV1Beta2Reason = clusterv1.DeletingV1Beta2Reason +) + +// VSphereCluster's VCenterAvailable condition and corresponding reasons that will be used in v1Beta2 API version. +const ( + // VSphereClusterVCenterAvailableV1Beta2Condition documents the status of vCenter for a VSphereCluster. + VSphereClusterVCenterAvailableV1Beta2Condition = "VCenterAvailable" + + // VSphereClusterVCenterAvailableV1Beta2Reason surfaces when the vCenter for a VSphereCluster is available. + VSphereClusterVCenterAvailableV1Beta2Reason = clusterv1.AvailableV1Beta2Reason + + // VSphereClusterVCenterUnreachableV1Beta2Reason surfaces when the vCenter for a VSphereCluster is unreachable. + VSphereClusterVCenterUnreachableV1Beta2Reason = "VCenterUnreachable" + + // VSphereClusterVCenterNotAvailableV1Beta2Reason surfaces when the vCenter for a VSphereCluster is not available. + VSphereClusterVCenterNotAvailableV1Beta2Reason = clusterv1.NotAvailableV1Beta2Reason + + // VSphereClusterVCenterAvailableDeletingV1Beta2Reason surfaces when the VSphereCluster is being deleted. + VSphereClusterVCenterAvailableDeletingV1Beta2Reason = clusterv1.DeletingV1Beta2Reason +) + +// VSphereCluster's ClusterModulesReady condition and corresponding reasons that will be used in v1Beta2 API version. +const ( + // VSphereClusterClusterModulesReadyV1Beta2Condition documents the status of vCenter for a VSphereCluster. + VSphereClusterClusterModulesReadyV1Beta2Condition = "ClusterModulesReady" + + // VSphereClusterClusterModulesReadyV1Beta2Reason surfaces when the cluster modules for a VSphereCluster are ready. + VSphereClusterClusterModulesReadyV1Beta2Reason = clusterv1.ReadyV1Beta2Reason + + // VSphereClusterModulesInvalidVCenterVersionV1Beta2Reason surfaces when the cluster modules for a VSphereCluster can't be reconciled + // due to an invalid vCenter version. + VSphereClusterModulesInvalidVCenterVersionV1Beta2Reason = "InvalidVCenterVersion" + + // VSphereClusterClusterModulesNotReadyV1Beta2Reason surfaces when the cluster modules for a VSphereCluster are not ready. + VSphereClusterClusterModulesNotReadyV1Beta2Reason = clusterv1.NotReadyV1Beta2Reason + + // VSphereClusterClusterModulesDeletingV1Beta2Reason surfaces when the cluster modules for a VSphereCluster are being deleted. + VSphereClusterClusterModulesDeletingV1Beta2Reason = clusterv1.DeletingV1Beta2Reason +) + // VCenterVersion conveys the API version of the vCenter instance. type VCenterVersion string @@ -114,7 +186,7 @@ type VSphereClusterStatus struct { // See https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20240916-improve-status-in-CAPI-resources.md for more context. type VSphereClusterV1Beta2Status struct { // conditions represents the observations of a VSphereCluster's current state. - // Known condition types are Paused. + // Known condition types are Ready, FailureDomainsReady, VCenterAvailable, ClusterModulesReady and Paused. // +optional // +listType=map // +listMapKey=type diff --git a/config/default/crd/bases/infrastructure.cluster.x-k8s.io_vsphereclusters.yaml b/config/default/crd/bases/infrastructure.cluster.x-k8s.io_vsphereclusters.yaml index ae003f165c..97df12fdec 100644 --- a/config/default/crd/bases/infrastructure.cluster.x-k8s.io_vsphereclusters.yaml +++ b/config/default/crd/bases/infrastructure.cluster.x-k8s.io_vsphereclusters.yaml @@ -863,7 +863,7 @@ spec: conditions: description: |- conditions represents the observations of a VSphereCluster's current state. - Known condition types are Paused. + Known condition types are Ready, FailureDomainsReady, VCenterAvailable, ClusterModulesReady and Paused. items: description: Condition contains details for one aspect of the current state of this API Resource. diff --git a/controllers/clustermodule_reconciler.go b/controllers/clustermodule_reconciler.go index fa1edcd16e..b3ed6b28e1 100644 --- a/controllers/clustermodule_reconciler.go +++ b/controllers/clustermodule_reconciler.go @@ -28,6 +28,7 @@ import ( controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1" "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/conditions" + v1beta2conditions "sigs.k8s.io/cluster-api/util/conditions/v1beta2" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" @@ -69,6 +70,12 @@ func (r Reconciler) Reconcile(ctx context.Context, clusterCtx *capvcontext.Clust if !clustermodule.IsClusterCompatible(clusterCtx) { conditions.MarkFalse(clusterCtx.VSphereCluster, infrav1.ClusterModulesAvailableCondition, infrav1.VCenterVersionIncompatibleReason, clusterv1.ConditionSeverityInfo, "vCenter version %s does not support cluster modules", clusterCtx.VSphereCluster.Status.VCenterVersion) + v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ + Type: infrav1.VSphereClusterClusterModulesReadyV1Beta2Condition, + Status: metav1.ConditionFalse, + Reason: infrav1.VSphereClusterModulesInvalidVCenterVersionV1Beta2Reason, + Message: fmt.Sprintf("vCenter version %s does not support cluster modules", clusterCtx.VSphereCluster.Status.VCenterVersion), + }) log.V(5).Info(fmt.Sprintf("vCenter version %s does not support cluster modules to implement anti affinity (vCenter >= 7 required)", clusterCtx.VSphereCluster.Status.VCenterVersion)) return reconcile.Result{}, nil } @@ -170,11 +177,22 @@ func (r Reconciler) Reconcile(ctx context.Context, clusterCtx *capvcontext.Clust } conditions.MarkFalse(clusterCtx.VSphereCluster, infrav1.ClusterModulesAvailableCondition, infrav1.ClusterModuleSetupFailedReason, clusterv1.ConditionSeverityWarning, generateClusterModuleErrorMessage(modErrs)) + v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ + Type: infrav1.VSphereClusterClusterModulesReadyV1Beta2Condition, + Status: metav1.ConditionFalse, + Reason: infrav1.VSphereClusterClusterModulesNotReadyV1Beta2Reason, + Message: generateClusterModuleErrorMessage(modErrs), + }) case len(modErrs) == 0 && len(clusterModuleSpecs) > 0: conditions.MarkTrue(clusterCtx.VSphereCluster, infrav1.ClusterModulesAvailableCondition) default: conditions.Delete(clusterCtx.VSphereCluster, infrav1.ClusterModulesAvailableCondition) } + v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ + Type: infrav1.VSphereClusterClusterModulesReadyV1Beta2Condition, + Status: metav1.ConditionTrue, + Reason: infrav1.VSphereClusterClusterModulesReadyV1Beta2Reason, + }) return reconcile.Result{}, err } diff --git a/controllers/vspherecluster_reconciler.go b/controllers/vspherecluster_reconciler.go index 07c48f4258..f8e98e774b 100644 --- a/controllers/vspherecluster_reconciler.go +++ b/controllers/vspherecluster_reconciler.go @@ -34,6 +34,7 @@ import ( clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" clusterutilv1 "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/conditions" + v1beta2conditions "sigs.k8s.io/cluster-api/util/conditions/v1beta2" "sigs.k8s.io/cluster-api/util/finalizers" "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/cluster-api/util/paused" @@ -107,7 +108,7 @@ func (r *clusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ // Always issue a patch when exiting this function so changes to the // resource are patched back to the API server. defer func() { - if err := clusterContext.Patch(ctx); err != nil { + if err := r.patch(ctx, clusterContext); err != nil { reterr = kerrors.NewAggregate([]error{reterr, err}) } }() @@ -117,18 +118,91 @@ func (r *clusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ return r.reconcileDelete(ctx, clusterContext) } - if cluster == nil { - log.Info("Waiting for Cluster controller to set OwnerRef on VSphereCluster") - return reconcile.Result{}, nil - } - // Handle non-deleted clusters return r.reconcileNormal(ctx, clusterContext) } +// patch updates the VSphereCluster and its status on the API server. +func (r *clusterReconciler) patch(ctx context.Context, clusterCtx *capvcontext.ClusterContext) error { + // always update the readyCondition. + conditions.SetSummary(clusterCtx.VSphereCluster, + conditions.WithConditions( + infrav1.VCenterAvailableCondition, + ), + ) + + if !v1beta2conditions.Has(clusterCtx.VSphereCluster, infrav1.VSphereClusterVCenterAvailableV1Beta2Condition) { + v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ + Type: infrav1.VSphereClusterVCenterAvailableV1Beta2Condition, + Status: metav1.ConditionFalse, + Reason: infrav1.VSphereClusterVCenterNotAvailableV1Beta2Reason, + Message: "did not observe VCenterAvailable status", + }) + } + + if !v1beta2conditions.Has(clusterCtx.VSphereCluster, infrav1.VSphereClusterClusterModulesReadyV1Beta2Condition) { + v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ + Type: infrav1.VSphereClusterClusterModulesReadyV1Beta2Condition, + Status: metav1.ConditionFalse, + Reason: infrav1.VSphereClusterClusterModulesNotReadyV1Beta2Reason, + Message: "did not observe cluster module status", + }) + } + + if err := v1beta2conditions.SetSummaryCondition(clusterCtx.VSphereCluster, clusterCtx.VSphereCluster, infrav1.VSphereClusterReadyV1Beta2Condition, + v1beta2conditions.ForConditionTypes{ + infrav1.VSphereClusterFailureDomainsReadyV1Beta2Condition, + infrav1.VSphereClusterVCenterAvailableV1Beta2Condition, + infrav1.VSphereClusterClusterModulesReadyV1Beta2Condition, + }, + // Using a custom merge strategy to override reasons applied during merge. + v1beta2conditions.CustomMergeStrategy{ + MergeStrategy: v1beta2conditions.DefaultMergeStrategy( + // Use custom reasons. + v1beta2conditions.ComputeReasonFunc(v1beta2conditions.GetDefaultComputeMergeReasonFunc( + infrav1.VSphereClusterNotReadyV1Beta2Reason, + infrav1.VSphereClusterReadyUnknownV1Beta2Reason, + infrav1.VSphereClusterReadyV1Beta2Reason, + )), + ), + }, + ); err != nil { + return pkgerrors.Wrapf(err, "failed to set %s condition", infrav1.VSphereClusterReadyV1Beta2Condition) + } + + return clusterCtx.PatchHelper.Patch(ctx, clusterCtx.VSphereCluster, + patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{}}, + patch.WithOwnedV1Beta2Conditions{Conditions: []string{ + clusterv1.PausedV1Beta2Condition, + infrav1.VSphereClusterReadyV1Beta2Condition, + infrav1.VSphereClusterFailureDomainsReadyV1Beta2Condition, + infrav1.VSphereClusterVCenterAvailableV1Beta2Condition, + infrav1.VSphereClusterClusterModulesReadyV1Beta2Condition, + }}, + ) +} + func (r *clusterReconciler) reconcileDelete(ctx context.Context, clusterCtx *capvcontext.ClusterContext) (reconcile.Result, error) { log := ctrl.LoggerFrom(ctx) + v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ + Type: infrav1.VSphereClusterVCenterAvailableV1Beta2Condition, + Status: metav1.ConditionFalse, + Reason: infrav1.VSphereClusterVCenterAvailableDeletingV1Beta2Reason, + }) + + v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ + Type: infrav1.VSphereClusterClusterModulesReadyV1Beta2Condition, + Status: metav1.ConditionFalse, + Reason: infrav1.VSphereClusterClusterModulesDeletingV1Beta2Reason, + }) + + v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ + Type: infrav1.VSphereClusterFailureDomainsReadyV1Beta2Condition, + Status: metav1.ConditionFalse, + Reason: infrav1.VSphereClusterFailureDomainsDeletingV1Beta2Reason, + }) + var vsphereMachines []client.Object var err error if clusterCtx.Cluster != nil { @@ -193,35 +267,82 @@ func (r *clusterReconciler) reconcileNormal(ctx context.Context, clusterCtx *cap ok, err := r.reconcileDeploymentZones(ctx, clusterCtx) if err != nil { + v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ + Type: infrav1.VSphereClusterFailureDomainsReadyV1Beta2Condition, + Status: metav1.ConditionFalse, + Reason: infrav1.VSphereClusterFailureDomainsNotReadyV1Beta2Reason, + Message: err.Error(), + }) return reconcile.Result{}, err } if !ok { - log.Info("Waiting for failure domains to be reconciled") + v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ + Type: infrav1.VSphereClusterFailureDomainsReadyV1Beta2Condition, + Status: metav1.ConditionFalse, + Reason: infrav1.VSphereClusterFailureDomainsWaitingForFailureDomainStatusV1Beta2Reason, + Message: "waiting for failure domains to report ready status", + }) return reconcile.Result{RequeueAfter: 10 * time.Second}, nil } + v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ + Type: infrav1.VSphereClusterFailureDomainsReadyV1Beta2Condition, + Status: metav1.ConditionTrue, + Reason: infrav1.VSphereClusterFailureDomainsReadyV1Beta2Reason, + }) + + // TODO check all occurencies of infrav1.VCenterAvailableCondition and infrav1.ClusterModulesAvailable/ready condition if err := r.reconcileIdentitySecret(ctx, clusterCtx); err != nil { conditions.MarkFalse(clusterCtx.VSphereCluster, infrav1.VCenterAvailableCondition, infrav1.VCenterUnreachableReason, clusterv1.ConditionSeverityError, err.Error()) + v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ + Type: infrav1.VSphereClusterVCenterAvailableV1Beta2Condition, + Status: metav1.ConditionFalse, + Reason: infrav1.VSphereClusterVCenterUnreachableV1Beta2Reason, + Message: err.Error(), + }) return reconcile.Result{}, err } vcenterSession, err := r.reconcileVCenterConnectivity(ctx, clusterCtx) if err != nil { conditions.MarkFalse(clusterCtx.VSphereCluster, infrav1.VCenterAvailableCondition, infrav1.VCenterUnreachableReason, clusterv1.ConditionSeverityError, err.Error()) + v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ + Type: infrav1.VSphereClusterVCenterAvailableV1Beta2Condition, + Status: metav1.ConditionFalse, + Reason: infrav1.VSphereClusterVCenterUnreachableV1Beta2Reason, + Message: err.Error(), + }) return reconcile.Result{}, pkgerrors.Wrapf(err, "unexpected error while probing vcenter for %s", clusterCtx) } conditions.MarkTrue(clusterCtx.VSphereCluster, infrav1.VCenterAvailableCondition) + v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ + Type: infrav1.VSphereClusterVCenterAvailableV1Beta2Condition, + Status: metav1.ConditionTrue, + Reason: infrav1.VSphereClusterVCenterAvailableV1Beta2Reason, + }) err = r.reconcileVCenterVersion(clusterCtx, vcenterSession) if err != nil || clusterCtx.VSphereCluster.Status.VCenterVersion == "" { - conditions.MarkFalse(clusterCtx.VSphereCluster, infrav1.ClusterModulesAvailableCondition, infrav1.MissingVCenterVersionReason, clusterv1.ConditionSeverityWarning, "vCenter version not set") + conditions.MarkFalse(clusterCtx.VSphereCluster, infrav1.ClusterModulesAvailableCondition, infrav1.MissingVCenterVersionReason, clusterv1.ConditionSeverityWarning, err.Error()) + v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ + Type: infrav1.VSphereClusterClusterModulesReadyV1Beta2Condition, + Status: metav1.ConditionFalse, + Reason: infrav1.VSphereClusterModulesInvalidVCenterVersionV1Beta2Reason, + Message: err.Error(), + }) log.Error(err, "could not reconcile vCenter version") } affinityReconcileResult, err := r.reconcileClusterModules(ctx, clusterCtx) if err != nil { conditions.MarkFalse(clusterCtx.VSphereCluster, infrav1.ClusterModulesAvailableCondition, infrav1.ClusterModuleSetupFailedReason, clusterv1.ConditionSeverityWarning, err.Error()) + v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ + Type: infrav1.VSphereClusterClusterModulesReadyV1Beta2Condition, + Status: metav1.ConditionFalse, + Reason: infrav1.VSphereClusterClusterModulesNotReadyV1Beta2Reason, + Message: err.Error(), + }) return affinityReconcileResult, err } @@ -295,13 +416,14 @@ func (r *clusterReconciler) reconcileVCenterConnectivity(ctx context.Context, cl func (r *clusterReconciler) reconcileVCenterVersion(clusterCtx *capvcontext.ClusterContext, s *session.Session) error { version, err := s.GetVersion() if err != nil { - return err + return pkgerrors.Wrapf(err, "invalid vCenter version") } clusterCtx.VSphereCluster.Status.VCenterVersion = version return nil } func (r *clusterReconciler) reconcileDeploymentZones(ctx context.Context, clusterCtx *capvcontext.ClusterContext) (bool, error) { + log := ctrl.LoggerFrom(ctx) // If there is no failure domain selector, skip reconciliation if clusterCtx.VSphereCluster.Spec.FailureDomainSelector == nil { return true, nil @@ -346,6 +468,7 @@ func (r *clusterReconciler) reconcileDeploymentZones(ctx context.Context, cluste clusterCtx.VSphereCluster.Status.FailureDomains = failureDomains if readyNotReported > 0 { + log.Info("Waiting for failure domains to be reconciled") conditions.MarkFalse(clusterCtx.VSphereCluster, infrav1.FailureDomainsAvailableCondition, infrav1.WaitingForFailureDomainStatusReason, clusterv1.ConditionSeverityInfo, "waiting for failure domains to report ready status") return false, nil } diff --git a/pkg/context/cluster_context.go b/pkg/context/cluster_context.go index 1166cc9566..79d24cb78d 100644 --- a/pkg/context/cluster_context.go +++ b/pkg/context/cluster_context.go @@ -17,11 +17,9 @@ limitations under the License. package context import ( - "context" "fmt" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - "sigs.k8s.io/cluster-api/util/conditions" "sigs.k8s.io/cluster-api/util/patch" infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" @@ -38,17 +36,3 @@ type ClusterContext struct { func (c *ClusterContext) String() string { return fmt.Sprintf("%s %s/%s", c.VSphereCluster.GroupVersionKind(), c.VSphereCluster.Namespace, c.VSphereCluster.Name) } - -// Patch updates the object and its status on the API server. -func (c *ClusterContext) Patch(ctx context.Context) error { - // always update the readyCondition. - conditions.SetSummary(c.VSphereCluster, - conditions.WithConditions( - infrav1.VCenterAvailableCondition, - ), - ) - - return c.PatchHelper.Patch(ctx, c.VSphereCluster, patch.WithOwnedV1Beta2Conditions{Conditions: []string{ - clusterv1.PausedV1Beta2Condition, - }}) -} diff --git a/pkg/session/error.go b/pkg/session/error.go deleted file mode 100644 index f53fb5b49e..0000000000 --- a/pkg/session/error.go +++ /dev/null @@ -1,40 +0,0 @@ -/* -Copyright 2022 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package session - -import ( - "fmt" - "strings" -) - -const ( - errString = "vCenter version cannot be identified" -) - -// TODO (srm09): figure out a common place for custom errors. -type unidentifiedVCenterVersion struct { - version string -} - -func (e unidentifiedVCenterVersion) Error() string { - return fmt.Sprintf("%s: %s", errString, e.version) -} - -// IsUnidentifiedVCenterVersion identies an error as an unidentifiedVCenterVersion error. -func IsUnidentifiedVCenterVersion(err error) bool { - return strings.HasPrefix(err.Error(), errString) -} diff --git a/pkg/session/session.go b/pkg/session/session.go index efc8885d06..1b667dbf4f 100644 --- a/pkg/session/session.go +++ b/pkg/session/session.go @@ -268,13 +268,13 @@ func (s *Session) GetVersion() (infrav1.VCenterVersion, error) { svcVersion := s.ServiceContent.About.Version version, err := semver.New(svcVersion) if err != nil { - return "", err + return "", errors.Wrapf(err, "failed to parse version %q", svcVersion) } if version.Major >= 6 { return infrav1.NewVCenterVersion(svcVersion), nil } - return "", unidentifiedVCenterVersion{version: svcVersion} + return "", errors.Errorf("version %q lower than 7", svcVersion) } // Clear is meant to destroy all the cached sessions. From a88b5133ecb36cc202544d487522b4ea170c5736 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Mon, 17 Mar 2025 18:16:58 +0100 Subject: [PATCH 2/4] fixes --- controllers/clustermodule_reconciler.go | 3 +- controllers/vspherecluster_reconciler.go | 66 +++++++++++------------- pkg/session/session.go | 2 +- 3 files changed, 34 insertions(+), 37 deletions(-) diff --git a/controllers/clustermodule_reconciler.go b/controllers/clustermodule_reconciler.go index b3ed6b28e1..a2bed77da1 100644 --- a/controllers/clustermodule_reconciler.go +++ b/controllers/clustermodule_reconciler.go @@ -183,6 +183,7 @@ func (r Reconciler) Reconcile(ctx context.Context, clusterCtx *capvcontext.Clust Reason: infrav1.VSphereClusterClusterModulesNotReadyV1Beta2Reason, Message: generateClusterModuleErrorMessage(modErrs), }) + return reconcile.Result{}, err case len(modErrs) == 0 && len(clusterModuleSpecs) > 0: conditions.MarkTrue(clusterCtx.VSphereCluster, infrav1.ClusterModulesAvailableCondition) default: @@ -337,7 +338,7 @@ type clusterModError struct { func generateClusterModuleErrorMessage(errList []clusterModError) string { sb := strings.Builder{} - sb.WriteString("failed to create cluster modules for: ") + sb.WriteString("Failed to create cluster modules for: ") for _, e := range errList { sb.WriteString(fmt.Sprintf("%s %s, ", e.name, e.err.Error())) diff --git a/controllers/vspherecluster_reconciler.go b/controllers/vspherecluster_reconciler.go index f8e98e774b..b9bd948e20 100644 --- a/controllers/vspherecluster_reconciler.go +++ b/controllers/vspherecluster_reconciler.go @@ -118,6 +118,11 @@ func (r *clusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ return r.reconcileDelete(ctx, clusterContext) } + if cluster == nil { + log.Info("Waiting for Cluster controller to set OwnerRef on VSphereCluster") + return reconcile.Result{}, nil + } + // Handle non-deleted clusters return r.reconcileNormal(ctx, clusterContext) } @@ -131,28 +136,15 @@ func (r *clusterReconciler) patch(ctx context.Context, clusterCtx *capvcontext.C ), ) - if !v1beta2conditions.Has(clusterCtx.VSphereCluster, infrav1.VSphereClusterVCenterAvailableV1Beta2Condition) { - v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ - Type: infrav1.VSphereClusterVCenterAvailableV1Beta2Condition, - Status: metav1.ConditionFalse, - Reason: infrav1.VSphereClusterVCenterNotAvailableV1Beta2Reason, - Message: "did not observe VCenterAvailable status", - }) - } - - if !v1beta2conditions.Has(clusterCtx.VSphereCluster, infrav1.VSphereClusterClusterModulesReadyV1Beta2Condition) { - v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ - Type: infrav1.VSphereClusterClusterModulesReadyV1Beta2Condition, - Status: metav1.ConditionFalse, - Reason: infrav1.VSphereClusterClusterModulesNotReadyV1Beta2Reason, - Message: "did not observe cluster module status", - }) - } - if err := v1beta2conditions.SetSummaryCondition(clusterCtx.VSphereCluster, clusterCtx.VSphereCluster, infrav1.VSphereClusterReadyV1Beta2Condition, v1beta2conditions.ForConditionTypes{ - infrav1.VSphereClusterFailureDomainsReadyV1Beta2Condition, infrav1.VSphereClusterVCenterAvailableV1Beta2Condition, + // FailureDomainsReady and ClusterModuelsReady may not be always set. + infrav1.VSphereClusterFailureDomainsReadyV1Beta2Condition, + infrav1.VSphereClusterClusterModulesReadyV1Beta2Condition, + }, + v1beta2conditions.IgnoreTypesIfMissing{ + infrav1.VSphereClusterFailureDomainsReadyV1Beta2Condition, infrav1.VSphereClusterClusterModulesReadyV1Beta2Condition, }, // Using a custom merge strategy to override reasons applied during merge. @@ -171,7 +163,6 @@ func (r *clusterReconciler) patch(ctx context.Context, clusterCtx *capvcontext.C } return clusterCtx.PatchHelper.Patch(ctx, clusterCtx.VSphereCluster, - patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{}}, patch.WithOwnedV1Beta2Conditions{Conditions: []string{ clusterv1.PausedV1Beta2Condition, infrav1.VSphereClusterReadyV1Beta2Condition, @@ -190,13 +181,11 @@ func (r *clusterReconciler) reconcileDelete(ctx context.Context, clusterCtx *cap Status: metav1.ConditionFalse, Reason: infrav1.VSphereClusterVCenterAvailableDeletingV1Beta2Reason, }) - v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ Type: infrav1.VSphereClusterClusterModulesReadyV1Beta2Condition, Status: metav1.ConditionFalse, Reason: infrav1.VSphereClusterClusterModulesDeletingV1Beta2Reason, }) - v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ Type: infrav1.VSphereClusterFailureDomainsReadyV1Beta2Condition, Status: metav1.ConditionFalse, @@ -265,6 +254,7 @@ func (r *clusterReconciler) reconcileDelete(ctx context.Context, clusterCtx *cap func (r *clusterReconciler) reconcileNormal(ctx context.Context, clusterCtx *capvcontext.ClusterContext) (reconcile.Result, error) { log := ctrl.LoggerFrom(ctx) + // Reconcile failure domains. ok, err := r.reconcileDeploymentZones(ctx, clusterCtx) if err != nil { v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ @@ -276,22 +266,10 @@ func (r *clusterReconciler) reconcileNormal(ctx context.Context, clusterCtx *cap return reconcile.Result{}, err } if !ok { - v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ - Type: infrav1.VSphereClusterFailureDomainsReadyV1Beta2Condition, - Status: metav1.ConditionFalse, - Reason: infrav1.VSphereClusterFailureDomainsWaitingForFailureDomainStatusV1Beta2Reason, - Message: "waiting for failure domains to report ready status", - }) return reconcile.Result{RequeueAfter: 10 * time.Second}, nil } - v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ - Type: infrav1.VSphereClusterFailureDomainsReadyV1Beta2Condition, - Status: metav1.ConditionTrue, - Reason: infrav1.VSphereClusterFailureDomainsReadyV1Beta2Reason, - }) - - // TODO check all occurencies of infrav1.VCenterAvailableCondition and infrav1.ClusterModulesAvailable/ready condition + // Reconcile vCenter availability. if err := r.reconcileIdentitySecret(ctx, clusterCtx); err != nil { conditions.MarkFalse(clusterCtx.VSphereCluster, infrav1.VCenterAvailableCondition, infrav1.VCenterUnreachableReason, clusterv1.ConditionSeverityError, err.Error()) v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ @@ -322,6 +300,7 @@ func (r *clusterReconciler) reconcileNormal(ctx context.Context, clusterCtx *cap Reason: infrav1.VSphereClusterVCenterAvailableV1Beta2Reason, }) + // Reconcile cluster modules. err = r.reconcileVCenterVersion(clusterCtx, vcenterSession) if err != nil || clusterCtx.VSphereCluster.Status.VCenterVersion == "" { conditions.MarkFalse(clusterCtx.VSphereCluster, infrav1.ClusterModulesAvailableCondition, infrav1.MissingVCenterVersionReason, clusterv1.ConditionSeverityWarning, err.Error()) @@ -470,14 +449,31 @@ func (r *clusterReconciler) reconcileDeploymentZones(ctx context.Context, cluste if readyNotReported > 0 { log.Info("Waiting for failure domains to be reconciled") conditions.MarkFalse(clusterCtx.VSphereCluster, infrav1.FailureDomainsAvailableCondition, infrav1.WaitingForFailureDomainStatusReason, clusterv1.ConditionSeverityInfo, "waiting for failure domains to report ready status") + v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ + Type: infrav1.VSphereClusterFailureDomainsReadyV1Beta2Condition, + Status: metav1.ConditionTrue, + Reason: infrav1.VSphereClusterFailureDomainsNotReadyV1Beta2Reason, + Message: "Waiting for failure domains to report ready status", + }) return false, nil } if len(failureDomains) > 0 { if notReady > 0 { conditions.MarkFalse(clusterCtx.VSphereCluster, infrav1.FailureDomainsAvailableCondition, infrav1.FailureDomainsSkippedReason, clusterv1.ConditionSeverityInfo, "one or more failure domains are not ready") + v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ + Type: infrav1.VSphereClusterFailureDomainsReadyV1Beta2Condition, + Status: metav1.ConditionTrue, + Reason: infrav1.VSphereClusterFailureDomainsNotReadyV1Beta2Reason, + Message: "One or more failure domains are not ready", + }) } else { conditions.MarkTrue(clusterCtx.VSphereCluster, infrav1.FailureDomainsAvailableCondition) + v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ + Type: infrav1.VSphereClusterFailureDomainsReadyV1Beta2Condition, + Status: metav1.ConditionTrue, + Reason: infrav1.VSphereClusterFailureDomainsReadyV1Beta2Reason, + }) } } else { // Remove the condition if failure domains do not exist diff --git a/pkg/session/session.go b/pkg/session/session.go index 1b667dbf4f..0f12d6964c 100644 --- a/pkg/session/session.go +++ b/pkg/session/session.go @@ -274,7 +274,7 @@ func (s *Session) GetVersion() (infrav1.VCenterVersion, error) { if version.Major >= 6 { return infrav1.NewVCenterVersion(svcVersion), nil } - return "", errors.Errorf("version %q lower than 7", svcVersion) + return "", errors.Errorf("version %q lower than 6", svcVersion) } // Clear is meant to destroy all the cached sessions. From c9298c0620944ef4a9359dd143f8b758cbef8cf2 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Tue, 18 Mar 2025 18:00:18 +0100 Subject: [PATCH 3/4] review fixes --- controllers/vspherecluster_reconciler.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/vspherecluster_reconciler.go b/controllers/vspherecluster_reconciler.go index b9bd948e20..bf7418de00 100644 --- a/controllers/vspherecluster_reconciler.go +++ b/controllers/vspherecluster_reconciler.go @@ -451,7 +451,7 @@ func (r *clusterReconciler) reconcileDeploymentZones(ctx context.Context, cluste conditions.MarkFalse(clusterCtx.VSphereCluster, infrav1.FailureDomainsAvailableCondition, infrav1.WaitingForFailureDomainStatusReason, clusterv1.ConditionSeverityInfo, "waiting for failure domains to report ready status") v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ Type: infrav1.VSphereClusterFailureDomainsReadyV1Beta2Condition, - Status: metav1.ConditionTrue, + Status: metav1.ConditionFalse, Reason: infrav1.VSphereClusterFailureDomainsNotReadyV1Beta2Reason, Message: "Waiting for failure domains to report ready status", }) @@ -463,7 +463,7 @@ func (r *clusterReconciler) reconcileDeploymentZones(ctx context.Context, cluste conditions.MarkFalse(clusterCtx.VSphereCluster, infrav1.FailureDomainsAvailableCondition, infrav1.FailureDomainsSkippedReason, clusterv1.ConditionSeverityInfo, "one or more failure domains are not ready") v1beta2conditions.Set(clusterCtx.VSphereCluster, metav1.Condition{ Type: infrav1.VSphereClusterFailureDomainsReadyV1Beta2Condition, - Status: metav1.ConditionTrue, + Status: metav1.ConditionFalse, Reason: infrav1.VSphereClusterFailureDomainsNotReadyV1Beta2Reason, Message: "One or more failure domains are not ready", }) From 121671d78ffadbfa30a045b7d1c45ed96c225288 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Wed, 19 Mar 2025 16:11:25 +0100 Subject: [PATCH 4/4] fix --- apis/v1beta1/vspherecluster_types.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/apis/v1beta1/vspherecluster_types.go b/apis/v1beta1/vspherecluster_types.go index 2a363f62e5..4dccb51952 100644 --- a/apis/v1beta1/vspherecluster_types.go +++ b/apis/v1beta1/vspherecluster_types.go @@ -74,9 +74,6 @@ const ( // VSphereClusterVCenterUnreachableV1Beta2Reason surfaces when the vCenter for a VSphereCluster is unreachable. VSphereClusterVCenterUnreachableV1Beta2Reason = "VCenterUnreachable" - // VSphereClusterVCenterNotAvailableV1Beta2Reason surfaces when the vCenter for a VSphereCluster is not available. - VSphereClusterVCenterNotAvailableV1Beta2Reason = clusterv1.NotAvailableV1Beta2Reason - // VSphereClusterVCenterAvailableDeletingV1Beta2Reason surfaces when the VSphereCluster is being deleted. VSphereClusterVCenterAvailableDeletingV1Beta2Reason = clusterv1.DeletingV1Beta2Reason )