From c6730433d1f649dd55ce89e92245283d888cdf33 Mon Sep 17 00:00:00 2001 From: Nahshon Unna Tsameret Date: Wed, 7 Jan 2026 16:23:58 +0200 Subject: [PATCH 1/5] Revert moving to claster-api v1beta2 Going back to v1beta1 Signed-off-by: Nahshon Unna Tsameret --- api/v1alpha1/condition_consts.go | 10 +- api/v1alpha1/kubevirtcluster_types.go | 10 +- api/v1alpha1/kubevirtclustertemplate_types.go | 2 +- api/v1alpha1/kubevirtmachine_types.go | 11 +- api/v1alpha1/zz_generated.deepcopy.go | 21 ++- ...ure.cluster.x-k8s.io_kubevirtclusters.yaml | 72 ++++---- ...ter.x-k8s.io_kubevirtclustertemplates.yaml | 1 - ...ure.cluster.x-k8s.io_kubevirtmachines.yaml | 53 +++--- config/rbac/role.yaml | 7 - controllers/kubevirtcluster_controller.go | 89 ++-------- .../kubevirtcluster_controller_test.go | 12 +- controllers/kubevirtmachine_controller.go | 168 ++++-------------- .../kubevirtmachine_controller_test.go | 60 +++---- e2e/create-cluster_test.go | 150 +++++++++------- e2e/v1beta1_helpers_test.go | 98 ---------- e2e/v1beta2_helpers_test.go | 98 ---------- pkg/capiv1beta1/resources.go | 31 ++-- pkg/context/cluster_context.go | 57 ++---- pkg/context/machine_context.go | 21 +-- pkg/crds/crds.go | 32 ---- pkg/crds/crds_test.go | 84 --------- pkg/kubevirt/utils.go | 4 +- pkg/testing/common.go | 20 ++- 23 files changed, 298 insertions(+), 813 deletions(-) delete mode 100644 e2e/v1beta1_helpers_test.go delete mode 100644 e2e/v1beta2_helpers_test.go delete mode 100644 pkg/crds/crds.go delete mode 100644 pkg/crds/crds_test.go diff --git a/api/v1alpha1/condition_consts.go b/api/v1alpha1/condition_consts.go index c736d1c0f..7848d6328 100644 --- a/api/v1alpha1/condition_consts.go +++ b/api/v1alpha1/condition_consts.go @@ -16,12 +16,14 @@ limitations under the License. package v1alpha1 +import clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta1" //nolint SA1019 + // Conditions and condition Reasons for the KubevirtMachine object const ( // VMProvisionedCondition documents the status of the provisioning of the VM // generated by a KubevirtMachine. - VMProvisionedCondition = "VMProvisioned" + VMProvisionedCondition clusterv1.ConditionType = "VMProvisioned" // WaitingForClusterInfrastructureReason (Severity=Info) documents a KubevirtMachine waiting for the cluster // infrastructure to be ready before starting to create the container that provides the KubevirtMachine @@ -37,7 +39,7 @@ const ( VMCreateFailedReason = "VMCreateFailed" // VMLiveMigratableCondition documents whether the VM is live-migratable or not - VMLiveMigratableCondition = "VMLiveMigratable" + VMLiveMigratableCondition clusterv1.ConditionType = "VMLiveMigratable" ) const ( @@ -45,7 +47,7 @@ const ( // It is set based on successful execution of bootstrap commands and on the existence of // the /run/cluster-api/bootstrap-success.complete file. // The condition gets generated after VMProvisionedCondition is True. - BootstrapExecSucceededCondition = "BootstrapExecSucceeded" + BootstrapExecSucceededCondition clusterv1.ConditionType = "BootstrapExecSucceeded" // BootstrappingReason documents (Severity=Info) a KubevirtMachine currently executing the bootstrap // script that creates the Kubernetes node on the newly provisioned machine infrastructure. @@ -61,7 +63,7 @@ const ( const ( // LoadBalancerAvailableCondition documents the availability of the service that implements the cluster load balancer. - LoadBalancerAvailableCondition = "LoadBalancerAvailable" + LoadBalancerAvailableCondition clusterv1.ConditionType = "LoadBalancerAvailable" // LoadBalancerProvisioningFailedReason (Severity=Warning) documents a KubevirtCluster controller detecting // an error while provisioning the service that provides the cluster load balancer; those kind of diff --git a/api/v1alpha1/kubevirtcluster_types.go b/api/v1alpha1/kubevirtcluster_types.go index 73368e0a2..618b7b103 100644 --- a/api/v1alpha1/kubevirtcluster_types.go +++ b/api/v1alpha1/kubevirtcluster_types.go @@ -19,7 +19,7 @@ package v1alpha1 import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta1" //nolint SA1019 ) const ( @@ -74,11 +74,11 @@ type KubevirtClusterStatus struct { // FailureDomains don't mean much in CAPD since it's all local, but we can see how the rest of cluster API // will use this if we populate it. - FailureDomains []clusterv1.FailureDomain `json:"failureDomains,omitempty"` + FailureDomains clusterv1.FailureDomains `json:"failureDomains,omitempty"` // Conditions defines current service state of the KubevirtCluster. // +optional - Conditions []metav1.Condition `json:"conditions,omitempty"` + Conditions []clusterv1.Condition `json:"conditions,omitempty"` } // APIEndpoint represents a reachable Kubernetes API endpoint. @@ -140,11 +140,11 @@ type KubevirtCluster struct { Status KubevirtClusterStatus `json:"status,omitempty"` } -func (c *KubevirtCluster) GetConditions() []metav1.Condition { +func (c *KubevirtCluster) GetConditions() clusterv1.Conditions { return c.Status.Conditions } -func (c *KubevirtCluster) SetConditions(conditions []metav1.Condition) { +func (c *KubevirtCluster) SetConditions(conditions clusterv1.Conditions) { c.Status.Conditions = conditions } diff --git a/api/v1alpha1/kubevirtclustertemplate_types.go b/api/v1alpha1/kubevirtclustertemplate_types.go index 0d784b8fd..d66883d42 100644 --- a/api/v1alpha1/kubevirtclustertemplate_types.go +++ b/api/v1alpha1/kubevirtclustertemplate_types.go @@ -18,7 +18,7 @@ package v1alpha1 import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta1" //nolint SA1019 ) // KubevirtClusterTemplateResource describes the data needed to create a KubevirtCluster from a template. diff --git a/api/v1alpha1/kubevirtmachine_types.go b/api/v1alpha1/kubevirtmachine_types.go index 64e0228d2..e419b429a 100644 --- a/api/v1alpha1/kubevirtmachine_types.go +++ b/api/v1alpha1/kubevirtmachine_types.go @@ -20,7 +20,8 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kubevirtv1 "kubevirt.io/api/core/v1" - clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta1" //nolint SA1019 + "sigs.k8s.io/cluster-api/errors" //nolint SA1019 ) const ( @@ -84,7 +85,7 @@ type KubevirtMachineStatus struct { // Conditions defines current service state of the KubevirtMachine. // +optional - Conditions []metav1.Condition `json:"conditions,omitempty"` + Conditions clusterv1.Conditions `json:"conditions,omitempty"` // NodeUpdated denotes that the ProviderID is updated on Node of this KubevirtMachine // +optional @@ -107,7 +108,7 @@ type KubevirtMachineStatus struct { // can be added as events to the Machine object and/or logged in the // controller's output. // +optional - FailureReason string `json:"failureReason,omitempty"` + FailureReason *errors.MachineStatusError `json:"failureReason,omitempty"` // FailureMessage will be set in the event that there is a terminal problem // reconciling the Machine and will contain a more verbose string suitable @@ -145,11 +146,11 @@ type KubevirtMachine struct { Status KubevirtMachineStatus `json:"status,omitempty"` } -func (c *KubevirtMachine) GetConditions() []metav1.Condition { +func (c *KubevirtMachine) GetConditions() clusterv1.Conditions { return c.Status.Conditions } -func (c *KubevirtMachine) SetConditions(conditions []metav1.Condition) { +func (c *KubevirtMachine) SetConditions(conditions clusterv1.Conditions) { c.Status.Conditions = conditions } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 26bbf7d41..8b8008ec2 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -22,9 +22,9 @@ package v1alpha1 import ( "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/cluster-api/api/core/v1beta2" + "sigs.k8s.io/cluster-api/api/core/v1beta1" + "sigs.k8s.io/cluster-api/errors" ) // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. @@ -146,14 +146,14 @@ func (in *KubevirtClusterStatus) DeepCopyInto(out *KubevirtClusterStatus) { *out = *in if in.FailureDomains != nil { in, out := &in.FailureDomains, &out.FailureDomains - *out = make([]v1beta2.FailureDomain, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) + *out = make(v1beta1.FailureDomains, len(*in)) + for key, val := range *in { + (*out)[key] = *val.DeepCopy() } } if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions - *out = make([]metav1.Condition, len(*in)) + *out = make([]v1beta1.Condition, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -352,16 +352,21 @@ func (in *KubevirtMachineStatus) DeepCopyInto(out *KubevirtMachineStatus) { *out = *in if in.Addresses != nil { in, out := &in.Addresses, &out.Addresses - *out = make([]v1beta2.MachineAddress, len(*in)) + *out = make([]v1beta1.MachineAddress, len(*in)) copy(*out, *in) } if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions - *out = make([]metav1.Condition, len(*in)) + *out = make(v1beta1.Conditions, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.FailureReason != nil { + in, out := &in.FailureReason, &out.FailureReason + *out = new(errors.MachineStatusError) + **out = **in + } if in.FailureMessage != nil { in, out := &in.FailureMessage, &out.FailureMessage *out = new(string) diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_kubevirtclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_kubevirtclusters.yaml index bf5cc3e58..56d9c90f1 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_kubevirtclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_kubevirtclusters.yaml @@ -190,67 +190,59 @@ spec: conditions: description: Conditions defines current service state of the KubevirtCluster. items: - description: Condition contains details for one aspect of the current - state of this API Resource. + description: Condition defines an observation of a Cluster API resource + operational state. properties: lastTransitionTime: description: |- lastTransitionTime is the last time the condition transitioned from one status to another. - This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + This should be when the underlying condition changed. If that is not known, then using the time when + the API field changed is acceptable. format: date-time type: string message: description: |- message is a human readable message indicating details about the transition. - This may be an empty string. - maxLength: 32768 + This field may be empty. + maxLength: 10240 + minLength: 1 type: string - observedGeneration: - description: |- - observedGeneration represents the .metadata.generation that the condition was set based upon. - For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date - with respect to the current state of the instance. - format: int64 - minimum: 0 - type: integer reason: description: |- - reason contains a programmatic identifier indicating the reason for the condition's last transition. - Producers of specific condition types may define expected values and meanings for this field, - and whether the values are considered a guaranteed API. - The value should be a CamelCase string. - This field may not be empty. - maxLength: 1024 + reason is the reason for the condition's last transition in CamelCase. + The specific API may choose whether or not this field is considered a guaranteed API. + This field may be empty. + maxLength: 256 minLength: 1 - pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + severity: + description: |- + severity provides an explicit classification of Reason code, so the users or machines can immediately + understand the current situation and act accordingly. + The Severity field MUST be set only when Status=False. + maxLength: 32 type: string status: description: status of the condition, one of True, False, Unknown. - enum: - - "True" - - "False" - - Unknown type: string type: - description: type of condition in CamelCase or in foo.example.com/CamelCase. - maxLength: 316 - pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + description: |- + type of condition in CamelCase or in foo.example.com/CamelCase. + Many .condition.type values are consistent across resources like Available, but because arbitrary conditions + can be useful (see .node.status.conditions), the ability to deconflict is important. + maxLength: 256 + minLength: 1 type: string required: - lastTransitionTime - - message - - reason - status - type type: object type: array failureDomains: - description: |- - FailureDomains don't mean much in CAPD since it's all local, but we can see how the rest of cluster API - will use this if we populate it. - items: + additionalProperties: description: |- - FailureDomain is the Schema for Cluster API failure domains. + FailureDomainSpec is the Schema for Cluster API failure domains. It allows controllers to understand how many failure domains a cluster can optionally span across. properties: attributes: @@ -263,15 +255,11 @@ spec: description: controlPlane determines if this failure domain is suitable for use by control plane machines. type: boolean - name: - description: name is the name of the failure domain. - maxLength: 256 - minLength: 1 - type: string - required: - - name type: object - type: array + description: |- + FailureDomains don't mean much in CAPD since it's all local, but we can see how the rest of cluster API + will use this if we populate it. + type: object ready: default: false description: Ready denotes that the infrastructure is ready. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_kubevirtclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_kubevirtclustertemplates.yaml index 64be5e8c5..be90dc88d 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_kubevirtclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_kubevirtclustertemplates.yaml @@ -72,7 +72,6 @@ spec: In future versions, controller-tools@v2 might allow overriding the type and validation for embedded types. When that happens, this hack should be revisited. - minProperties: 1 properties: annotations: additionalProperties: diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_kubevirtmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_kubevirtmachines.yaml index 6783add81..b291ffd17 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_kubevirtmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_kubevirtmachines.yaml @@ -4513,56 +4513,51 @@ spec: conditions: description: Conditions defines current service state of the KubevirtMachine. items: - description: Condition contains details for one aspect of the current - state of this API Resource. + description: Condition defines an observation of a Cluster API resource + operational state. properties: lastTransitionTime: description: |- lastTransitionTime is the last time the condition transitioned from one status to another. - This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + This should be when the underlying condition changed. If that is not known, then using the time when + the API field changed is acceptable. format: date-time type: string message: description: |- message is a human readable message indicating details about the transition. - This may be an empty string. - maxLength: 32768 + This field may be empty. + maxLength: 10240 + minLength: 1 type: string - observedGeneration: - description: |- - observedGeneration represents the .metadata.generation that the condition was set based upon. - For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date - with respect to the current state of the instance. - format: int64 - minimum: 0 - type: integer reason: description: |- - reason contains a programmatic identifier indicating the reason for the condition's last transition. - Producers of specific condition types may define expected values and meanings for this field, - and whether the values are considered a guaranteed API. - The value should be a CamelCase string. - This field may not be empty. - maxLength: 1024 + reason is the reason for the condition's last transition in CamelCase. + The specific API may choose whether or not this field is considered a guaranteed API. + This field may be empty. + maxLength: 256 minLength: 1 - pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + severity: + description: |- + severity provides an explicit classification of Reason code, so the users or machines can immediately + understand the current situation and act accordingly. + The Severity field MUST be set only when Status=False. + maxLength: 32 type: string status: description: status of the condition, one of True, False, Unknown. - enum: - - "True" - - "False" - - Unknown type: string type: - description: type of condition in CamelCase or in foo.example.com/CamelCase. - maxLength: 316 - pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + description: |- + type of condition in CamelCase or in foo.example.com/CamelCase. + Many .condition.type values are consistent across resources like Available, but because arbitrary conditions + can be useful (see .node.status.conditions), the ability to deconflict is important. + maxLength: 256 + minLength: 1 type: string required: - lastTransitionTime - - message - - reason - status - type type: object diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index b8b1be295..19f9dc6de 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -25,13 +25,6 @@ rules: - patch - update - watch -- apiGroups: - - apiextensions.k8s.io - resources: - - customresourcedefinitions - verbs: - - get - - list - apiGroups: - apps resources: diff --git a/controllers/kubevirtcluster_controller.go b/controllers/kubevirtcluster_controller.go index 9323bd58b..4869f43ae 100644 --- a/controllers/kubevirtcluster_controller.go +++ b/controllers/kubevirtcluster_controller.go @@ -19,10 +19,8 @@ package controllers import ( gocontext "context" "fmt" - "slices" "time" - clusterv1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1" //nolint SA1019 "sigs.k8s.io/controller-runtime/pkg/builder" "github.com/go-logr/logr" @@ -32,11 +30,11 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" utilerrors "k8s.io/apimachinery/pkg/util/errors" - clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta1" //nolint SA1019 "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/annotations" - "sigs.k8s.io/cluster-api/util/conditions" - "sigs.k8s.io/cluster-api/util/patch" + "sigs.k8s.io/cluster-api/util/deprecated/v1beta1/conditions" //nolint SA1019 + "sigs.k8s.io/cluster-api/util/deprecated/v1beta1/patch" //nolint SA1019 "sigs.k8s.io/cluster-api/util/predicates" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -46,26 +44,20 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-kubevirt/api/v1alpha1" "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/capiv1beta1" "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/context" - "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/crds" "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/infracluster" "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/loadbalancer" "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/ssh" ) -const ( - clusterCRDName = "clusters.cluster.x-k8s.io" -) - // KubevirtClusterReconciler reconciles a KubevirtCluster object. type KubevirtClusterReconciler struct { client.Client // APIReader is used to prune the Cloud Controller resources for the given cluster: // this client doesn't locally cache the resources upon a GET/LIST request, // decreasing memory consumption and avoiding granting further RBAC verbs. - APIReader client.Reader - InfraCluster infracluster.InfraCluster - Log logr.Logger - GetOwnerCluster func(ctx gocontext.Context, c client.Client, obj metav1.ObjectMeta) (*clusterv1.Cluster, error) + APIReader client.Reader + InfraCluster infracluster.InfraCluster + Log logr.Logger } func GetLoadBalancerNamespace(kc *infrav1.KubevirtCluster, infraClusterNamespace string) string { @@ -82,7 +74,6 @@ func GetLoadBalancerNamespace(kc *infrav1.KubevirtCluster, infraClusterNamespace // +kubebuilder:rbac:groups="",resources=serviceaccounts;configmaps,verbs=delete;list // +kubebuilder:rbac:groups=apps,resources=deployments,verbs=delete;list // +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,verbs=delete;list -// +kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get;list // Reconcile reads that state of the cluster for a KubevirtCluster object and makes changes based on the state read // and what is in the KubevirtCluster.Spec. @@ -105,7 +96,7 @@ func (r *KubevirtClusterReconciler) Reconcile(goctx gocontext.Context, req ctrl. } // Fetch the Cluster. - cluster, err := r.GetOwnerCluster(goctx, r.Client, kubevirtCluster.ObjectMeta) + cluster, err := capiv1beta1.GetOwnerCluster(goctx, r.Client, kubevirtCluster.ObjectMeta) if err != nil { return ctrl.Result{}, err } @@ -176,12 +167,7 @@ func (r *KubevirtClusterReconciler) reconcileNormal(ctx *context.ClusterContext, // Create the service serving as load balancer, if not existing if !externalLoadBalancer.IsFound() { if err := externalLoadBalancer.Create(ctx); err != nil { - conditions.Set(ctx.KubevirtCluster, metav1.Condition{ - Type: infrav1.LoadBalancerAvailableCondition, - Status: metav1.ConditionFalse, - Reason: infrav1.LoadBalancerProvisioningFailedReason, - Message: fmt.Sprintf("%v", err.Error()), - }) + conditions.MarkFalse(ctx.KubevirtCluster, infrav1.LoadBalancerAvailableCondition, infrav1.LoadBalancerProvisioningFailedReason, clusterv1.ConditionSeverityWarning, "%v", err.Error()) return ctrl.Result{}, errors.Wrap(err, "failed to create load balancer") } } @@ -196,12 +182,7 @@ func (r *KubevirtClusterReconciler) reconcileNormal(ctx *context.ClusterContext, } else if ctx.KubevirtCluster.Spec.ControlPlaneServiceTemplate.Spec.Type == "LoadBalancer" { lbip4, err := externalLoadBalancer.ExternalIP(ctx) if err != nil { - conditions.Set(ctx.KubevirtCluster, metav1.Condition{ - Type: infrav1.LoadBalancerAvailableCondition, - Status: metav1.ConditionFalse, - Reason: infrav1.LoadBalancerProvisioningFailedReason, - Message: fmt.Sprintf("%v", err.Error()), - }) + conditions.MarkFalse(ctx.KubevirtCluster, infrav1.LoadBalancerAvailableCondition, infrav1.LoadBalancerProvisioningFailedReason, clusterv1.ConditionSeverityWarning, "%v", err.Error()) return ctrl.Result{}, errors.Wrap(err, "failed to get ExternalIP for the load balancer") } ctx.KubevirtCluster.Spec.ControlPlaneEndpoint = infrav1.APIEndpoint{ @@ -213,12 +194,7 @@ func (r *KubevirtClusterReconciler) reconcileNormal(ctx *context.ClusterContext, } else { lbip4, err := externalLoadBalancer.IP(ctx) if err != nil { - conditions.Set(ctx.KubevirtCluster, metav1.Condition{ - Type: infrav1.LoadBalancerAvailableCondition, - Status: metav1.ConditionFalse, - Reason: infrav1.LoadBalancerProvisioningFailedReason, - Message: fmt.Sprintf("%v", err.Error()), - }) + conditions.MarkFalse(ctx.KubevirtCluster, infrav1.LoadBalancerAvailableCondition, infrav1.LoadBalancerProvisioningFailedReason, clusterv1.ConditionSeverityWarning, "%v", err.Error()) return ctrl.Result{}, errors.Wrap(err, "failed to get ClusterIP for the load balancer") } ctx.KubevirtCluster.Spec.ControlPlaneEndpoint = infrav1.APIEndpoint{ @@ -227,12 +203,7 @@ func (r *KubevirtClusterReconciler) reconcileNormal(ctx *context.ClusterContext, } } - conditions.Set(ctx.KubevirtCluster, metav1.Condition{ - Type: infrav1.LoadBalancerAvailableCondition, - Status: metav1.ConditionTrue, - Reason: clusterv1.UpToDateReason, - Message: "", - }) + conditions.MarkTrue(ctx.KubevirtCluster, infrav1.LoadBalancerAvailableCondition) // Generate ssh keys for cluster nodes, and persist them to a secret clusterNodeSSHKeys := ssh.NewClusterNodeSshKeys(ctx, r.Client) @@ -274,12 +245,7 @@ func (r *KubevirtClusterReconciler) reconcileDelete(ctx *context.ClusterContext, if err != nil { return ctrl.Result{}, err } - conditions.Set(ctx.KubevirtCluster, metav1.Condition{ - Type: infrav1.LoadBalancerAvailableCondition, - Status: metav1.ConditionFalse, - Reason: clusterv1.DeletingReason, - Message: "", - }) + conditions.MarkFalse(ctx.KubevirtCluster, infrav1.LoadBalancerAvailableCondition, clusterv1.DeletingReason, clusterv1.ConditionSeverityInfo, "") if err := ctx.PatchKubevirtCluster(patchHelper); err != nil { return ctrl.Result{}, errors.Wrap(err, "failed to patch KubevirtCluster") } @@ -303,20 +269,11 @@ func (r *KubevirtClusterReconciler) reconcileDelete(ctx *context.ClusterContext, // SetupWithManager will add watches for this controller. func (r *KubevirtClusterReconciler) SetupWithManager(ctx gocontext.Context, mgr ctrl.Manager) error { - clusterAPIVersions, err := crds.GetSupportedVersions(ctx, mgr.GetAPIReader(), clusterCRDName) - if err != nil { - return fmt.Errorf("unable to get CRD versions of %s: %w", clusterCRDName, err) - } - - typedBuilder := ctrl.NewControllerManagedBy(mgr). + return ctrl.NewControllerManagedBy(mgr). For(&infrav1.KubevirtCluster{}). WithEventFilter(predicates.ResourceNotPaused(r.Scheme(), r.Log)). - WithEventFilter(predicates.ResourceIsNotExternallyManaged(r.Scheme(), r.Log)) - - if slices.Contains(clusterAPIVersions, "v1beta2") { - r.Log.Info("reconciling cluster-api Cluster v1beta2") - r.GetOwnerCluster = util.GetOwnerCluster - typedBuilder.Watches( + WithEventFilter(predicates.ResourceIsNotExternallyManaged(r.Scheme(), r.Log)). + Watches( &clusterv1.Cluster{}, handler.EnqueueRequestsFromMapFunc(util.ClusterToInfrastructureMapFunc( ctx, @@ -325,20 +282,8 @@ func (r *KubevirtClusterReconciler) SetupWithManager(ctx gocontext.Context, mgr &infrav1.KubevirtCluster{}, )), builder.WithPredicates(predicates.ClusterUnpaused(r.Scheme(), r.Log)), - ) - } else if slices.Contains(clusterAPIVersions, "v1beta1") { - r.Log.Info("reconciling cluster-api Cluster v1beta1") - r.GetOwnerCluster = capiv1beta1.GetOwnerCluster - typedBuilder.Watches( - &clusterv1beta1.Cluster{}, - handler.EnqueueRequestsFromMapFunc(capiv1beta1.MapV1beta1ClusterToKVKind(mgr.GetClient(), "KubevirtCluster")), - builder.WithPredicates(predicates.ClusterUnpaused(r.Scheme(), r.Log)), - ) - } else { - return fmt.Errorf("unsupported cluster-api versions: %v", clusterAPIVersions) - } - - return typedBuilder.Complete(r) + ). + Complete(r) } func (r *KubevirtClusterReconciler) deleteExtraGVK(ctx *context.ClusterContext, extraGVK schema.GroupVersionKind) error { diff --git a/controllers/kubevirtcluster_controller_test.go b/controllers/kubevirtcluster_controller_test.go index d647a7b77..e98021a6b 100644 --- a/controllers/kubevirtcluster_controller_test.go +++ b/controllers/kubevirtcluster_controller_test.go @@ -8,8 +8,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" - "sigs.k8s.io/cluster-api/util" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta1" //nolint SA1019 ctrl "sigs.k8s.io/controller-runtime" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" ctrlfake "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -48,11 +47,10 @@ var _ = Describe("Reconcile", func() { WithStatusSubresource(objects...). Build() kubevirtClusterReconciler = controllers.KubevirtClusterReconciler{ - Client: fakeClient, - InfraCluster: infraClusterMock, - APIReader: fakeClient, - Log: testLogger, - GetOwnerCluster: util.GetOwnerCluster, + Client: fakeClient, + InfraCluster: infraClusterMock, + APIReader: fakeClient, + Log: testLogger, } } diff --git a/controllers/kubevirtmachine_controller.go b/controllers/kubevirtmachine_controller.go index 9a3352eca..bb16a5bf9 100644 --- a/controllers/kubevirtmachine_controller.go +++ b/controllers/kubevirtmachine_controller.go @@ -20,7 +20,6 @@ import ( gocontext "context" "fmt" "regexp" - "slices" "time" "github.com/go-logr/logr" @@ -31,13 +30,12 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" utilerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/utils/ptr" - clusterv1v1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1" //nolint SA1019 - clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta1" //nolint SA1019 + capierrors "sigs.k8s.io/cluster-api/errors" //nolint SA1019 "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/annotations" - "sigs.k8s.io/cluster-api/util/conditions" - "sigs.k8s.io/cluster-api/util/patch" + "sigs.k8s.io/cluster-api/util/deprecated/v1beta1/conditions" //nolint SA1019 + "sigs.k8s.io/cluster-api/util/deprecated/v1beta1/patch" //nolint SA1019 "sigs.k8s.io/cluster-api/util/predicates" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" @@ -49,25 +47,18 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-kubevirt/api/v1alpha1" "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/capiv1beta1" "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/context" - "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/crds" "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/infracluster" "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/kubevirt" "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/ssh" "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/workloadcluster" ) -const ( - machineCRDName = "machines.cluster.x-k8s.io" -) - // KubevirtMachineReconciler reconciles a KubevirtMachine object. type KubevirtMachineReconciler struct { client.Client - InfraCluster infracluster.InfraCluster - WorkloadCluster workloadcluster.WorkloadCluster - MachineFactory kubevirt.MachineFactory - getOwnerMachine func(ctx gocontext.Context, c client.Client, obj metav1.ObjectMeta) (*clusterv1.Machine, error) - getClusterFromMetadata func(ctx gocontext.Context, c client.Client, obj metav1.ObjectMeta) (*clusterv1.Cluster, error) + InfraCluster infracluster.InfraCluster + WorkloadCluster workloadcluster.WorkloadCluster + MachineFactory kubevirt.MachineFactory } // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=kubevirtmachines,verbs=get;list;watch;create;update;patch;delete @@ -93,7 +84,7 @@ func (r *KubevirtMachineReconciler) Reconcile(goctx gocontext.Context, req ctrl. } // Fetch the Machine. - machine, err := r.getOwnerMachine(goctx, r.Client, kubevirtMachine.ObjectMeta) + machine, err := capiv1beta1.GetOwnerMachine(goctx, r.Client, kubevirtMachine.ObjectMeta) if err != nil { return ctrl.Result{}, err } @@ -120,7 +111,7 @@ func (r *KubevirtMachineReconciler) Reconcile(goctx gocontext.Context, req ctrl. } // Fetch the Cluster. - cluster, err := r.getClusterFromMetadata(goctx, r.Client, machine.ObjectMeta) + cluster, err := capiv1beta1.GetClusterFromMetadata(goctx, r.Client, machine.ObjectMeta) if err != nil { log.Info("KubevirtMachine owner Machine is missing cluster label or cluster does not exist") return ctrl.Result{}, err @@ -178,14 +169,9 @@ func (r *KubevirtMachineReconciler) Reconcile(goctx gocontext.Context, req ctrl. } // Check if the infrastructure is ready, otherwise return and wait for the cluster object to be updated - if !conditions.IsTrue(cluster, clusterv1.InfrastructureReadyCondition) { + if !cluster.Status.InfrastructureReady { log.Info("Waiting for KubevirtCluster Controller to create cluster infrastructure") - conditions.Set(kubevirtMachine, metav1.Condition{ - Type: infrav1.VMProvisionedCondition, - Status: metav1.ConditionFalse, - Reason: infrav1.WaitingForClusterInfrastructureReason, - Message: "", - }) + conditions.MarkFalse(kubevirtMachine, infrav1.VMProvisionedCondition, infrav1.WaitingForClusterInfrastructureReason, clusterv1.ConditionSeverityInfo, "") return ctrl.Result{}, nil } @@ -205,24 +191,14 @@ func (r *KubevirtMachineReconciler) reconcileNormal(ctx *context.MachineContext) // Make sure bootstrap data is available and populated. if ctx.Machine.Spec.Bootstrap.DataSecretName == nil { - if !util.IsControlPlaneMachine(ctx.Machine) && ptr.Equal(ctx.Cluster.Status.Initialization.ControlPlaneInitialized, ptr.To(false)) { + if !capiv1beta1.IsControlPlaneMachine(ctx.Machine) && !conditions.IsTrue(ctx.Cluster, clusterv1.ControlPlaneInitializedCondition) { ctx.Logger.Info("Waiting for the control plane to be initialized...") - conditions.Set(ctx.KubevirtMachine, metav1.Condition{ - Type: infrav1.VMProvisionedCondition, - Status: metav1.ConditionFalse, - Reason: clusterv1.WaitingForControlPlaneInitializedReason, - Message: "", - }) + conditions.MarkFalse(ctx.KubevirtMachine, infrav1.VMProvisionedCondition, clusterv1.WaitingForControlPlaneAvailableReason, clusterv1.ConditionSeverityInfo, "") return ctrl.Result{}, nil } ctx.Logger.Info("Waiting for Machine.Spec.Bootstrap.DataSecretName...") - conditions.Set(ctx.KubevirtMachine, metav1.Condition{ - Type: infrav1.VMProvisionedCondition, - Status: metav1.ConditionFalse, - Reason: infrav1.WaitingForBootstrapDataReason, - Message: "", - }) + conditions.MarkFalse(ctx.KubevirtMachine, infrav1.VMProvisionedCondition, infrav1.WaitingForBootstrapDataReason, clusterv1.ConditionSeverityInfo, "") return ctrl.Result{}, nil } @@ -267,12 +243,7 @@ func (r *KubevirtMachineReconciler) reconcileNormal(ctx *context.MachineContext) } if err := r.reconcileKubevirtBootstrapSecret(ctx, infraClusterClient, vmNamespace, clusterNodeSshKeys); err != nil { - conditions.Set(ctx.KubevirtMachine, metav1.Condition{ - Type: infrav1.VMProvisionedCondition, - Status: metav1.ConditionFalse, - Reason: infrav1.WaitingForBootstrapDataReason, - Message: "", - }) + conditions.MarkFalse(ctx.KubevirtMachine, infrav1.VMProvisionedCondition, infrav1.WaitingForBootstrapDataReason, clusterv1.ConditionSeverityInfo, "") return ctrl.Result{RequeueAfter: 10 * time.Second}, errors.Wrap(err, "failed to fetch kubevirt bootstrap secret") } @@ -287,8 +258,8 @@ func (r *KubevirtMachineReconciler) reconcileNormal(ctx *context.MachineContext) return ctrl.Result{}, errors.Wrapf(err, "failed checking VM for terminal state") } if isTerminal { - failureErr := "UpdateError" - ctx.KubevirtMachine.Status.FailureReason = failureErr + failureErr := capierrors.UpdateMachineError + ctx.KubevirtMachine.Status.FailureReason = &failureErr ctx.KubevirtMachine.Status.FailureMessage = &terminalReason } @@ -296,12 +267,7 @@ func (r *KubevirtMachineReconciler) reconcileNormal(ctx *context.MachineContext) if !isTerminal && !externalMachine.Exists() { ctx.KubevirtMachine.Status.Ready = false if err := externalMachine.Create(ctx.Context); err != nil { - conditions.Set(ctx.KubevirtMachine, metav1.Condition{ - Type: infrav1.VMProvisionedCondition, - Status: metav1.ConditionFalse, - Reason: infrav1.VMCreateFailedReason, - Message: fmt.Sprintf("Failed vm creation: %v", err), - }) + conditions.MarkFalse(ctx.KubevirtMachine, infrav1.VMProvisionedCondition, infrav1.VMCreateFailedReason, clusterv1.ConditionSeverityError, "Failed vm creation: %v", err) return ctrl.Result{}, errors.Wrap(err, "failed to create VM instance") } ctx.Logger.Info("VM Created, waiting on vm to be provisioned.") @@ -311,20 +277,10 @@ func (r *KubevirtMachineReconciler) reconcileNormal(ctx *context.MachineContext) // Checks to see if a VM's active VMI is ready or not if externalMachine.IsReady() { // Mark VMProvisionedCondition to indicate that the VM has successfully started - conditions.Set(ctx.KubevirtMachine, metav1.Condition{ - Type: infrav1.VMProvisionedCondition, - Status: metav1.ConditionTrue, - Reason: clusterv1.UpToDateReason, - Message: "", - }) + conditions.MarkTrue(ctx.KubevirtMachine, infrav1.VMProvisionedCondition) } else { reason, message := externalMachine.GetVMNotReadyReason() - conditions.Set(ctx.KubevirtMachine, metav1.Condition{ - Type: infrav1.VMProvisionedCondition, - Status: metav1.ConditionFalse, - Reason: reason, - Message: message, - }) + conditions.MarkFalse(ctx.KubevirtMachine, infrav1.VMProvisionedCondition, reason, clusterv1.ConditionSeverityInfo, "%s", message) // Waiting for VM to boot ctx.KubevirtMachine.Status.Ready = false @@ -360,22 +316,12 @@ func (r *KubevirtMachineReconciler) reconcileNormal(ctx *context.MachineContext) if externalMachine.SupportsCheckingIsBootstrapped() && !conditions.IsTrue(ctx.KubevirtMachine, infrav1.BootstrapExecSucceededCondition) { if !externalMachine.IsBootstrapped() { ctx.Logger.Info("Waiting for underlying VM to bootstrap...") - conditions.Set(ctx.KubevirtMachine, metav1.Condition{ - Type: infrav1.BootstrapExecSucceededCondition, - Status: metav1.ConditionFalse, - Reason: infrav1.BootstrapFailedReason, - Message: "VM not bootstrapped yet", - }) + conditions.MarkFalse(ctx.KubevirtMachine, infrav1.BootstrapExecSucceededCondition, infrav1.BootstrapFailedReason, clusterv1.ConditionSeverityWarning, "VM not bootstrapped yet") ctx.KubevirtMachine.Status.Ready = false return ctrl.Result{RequeueAfter: 10 * time.Second}, nil } // Update the condition BootstrapExecSucceededCondition - conditions.Set(ctx.KubevirtMachine, metav1.Condition{ - Type: infrav1.BootstrapExecSucceededCondition, - Status: metav1.ConditionTrue, - Reason: clusterv1.UpToDateReason, - Message: "", - }) + conditions.MarkTrue(ctx.KubevirtMachine, infrav1.BootstrapExecSucceededCondition) ctx.Logger.Info("Underlying VM has boostrapped.") } @@ -424,19 +370,10 @@ func (r *KubevirtMachineReconciler) reconcileNormal(ctx *context.MachineContext) } if liveMigratable { // Mark VMLiveMigratableCondition to indicate whether the VM can be live migrated or not - conditions.Set(ctx.KubevirtMachine, metav1.Condition{ - Type: infrav1.VMLiveMigratableCondition, - Status: metav1.ConditionTrue, - Reason: clusterv1.AvailableReason, - Message: "", - }) + conditions.MarkTrue(ctx.KubevirtMachine, infrav1.VMLiveMigratableCondition) } else { - conditions.Set(ctx.KubevirtMachine, metav1.Condition{ - Type: infrav1.VMLiveMigratableCondition, - Status: metav1.ConditionFalse, - Reason: reason, - Message: fmt.Sprintf("%s is not a live migratable machine: %s", ctx.KubevirtMachine.Name, message), - }) + conditions.MarkFalse(ctx.KubevirtMachine, infrav1.VMLiveMigratableCondition, reason, clusterv1.ConditionSeverityInfo, + "%s is not a live migratable machine: %s", ctx.KubevirtMachine.Name, message) } return ctrl.Result{}, nil @@ -546,11 +483,7 @@ func (r *KubevirtMachineReconciler) reconcileDelete(ctx *context.MachineContext) // Set the VMProvisionedCondition reporting delete is started, and attempt to issue a patch in // order to make this visible to the users. - conditions.Set(ctx.KubevirtMachine, metav1.Condition{ - Type: infrav1.VMProvisionedCondition, - Status: metav1.ConditionFalse, - Reason: clusterv1.DeletingReason, - }) + conditions.MarkFalse(ctx.KubevirtMachine, infrav1.VMProvisionedCondition, clusterv1.DeletingReason, clusterv1.ConditionSeverityInfo, "") if err := ctx.PatchKubevirtMachine(patchHelper); err != nil { if err = utilerrors.FilterOut(err, apierrors.IsNotFound); err != nil { return ctrl.Result{}, errors.Wrap(err, "failed to patch KubevirtMachine") @@ -567,53 +500,24 @@ func (r *KubevirtMachineReconciler) SetupWithManager(goctx gocontext.Context, mg return err } - clusterAPIVersions, err := crds.GetSupportedVersions(goctx, mgr.GetAPIReader(), machineCRDName) - if err != nil { - return fmt.Errorf("unable to get CRD versions of %s: %w", machineCRDName, err) - } - - controllerBuilder := ctrl.NewControllerManagedBy(mgr). + return ctrl.NewControllerManagedBy(mgr). For(&infrav1.KubevirtMachine{}). WithOptions(options). WithEventFilter(predicates.ResourceNotPaused(r.Scheme(), ctrl.LoggerFrom(goctx))). Watches( - &infrav1.KubevirtCluster{}, - handler.EnqueueRequestsFromMapFunc(r.KubevirtClusterToKubevirtMachines), - ) - - if slices.Contains(clusterAPIVersions, "v1beta2") { - logger.Info("reconciling cluster-api Machine v1beta2") - - r.getOwnerMachine = util.GetOwnerMachine - r.getClusterFromMetadata = util.GetClusterFromMetadata - - controllerBuilder.Watches( &clusterv1.Machine{}, handler.EnqueueRequestsFromMapFunc(util.MachineToInfrastructureMapFunc(infrav1.GroupVersion.WithKind("KubevirtMachine"))), - ).Watches( + ). + Watches( + &infrav1.KubevirtCluster{}, + handler.EnqueueRequestsFromMapFunc(r.KubevirtClusterToKubevirtMachines), + ). + Watches( &clusterv1.Cluster{}, handler.EnqueueRequestsFromMapFunc(clusterToKubevirtMachines), builder.WithPredicates(predicates.ClusterPausedTransitionsOrInfrastructureProvisioned(r.Scheme(), ctrl.LoggerFrom(goctx))), - ) - } else if slices.Contains(clusterAPIVersions, "v1beta1") { - logger.Info("reconciling cluster-api Machine v1beta1") - - r.getOwnerMachine = capiv1beta1.GetOwnerMachine - r.getClusterFromMetadata = capiv1beta1.GetClusterFromMetadata - - controllerBuilder.Watches( - &clusterv1v1beta1.Machine{}, - handler.EnqueueRequestsFromMapFunc(capiv1beta1.MapV1beta1MachineToKVMachine), - ).Watches( - &clusterv1v1beta1.Cluster{}, - handler.EnqueueRequestsFromMapFunc(capiv1beta1.MapV1beta1ClusterToKVKind(mgr.GetClient(), "KubevirtMachine")), - builder.WithPredicates(predicates.ClusterPausedTransitionsOrInfrastructureProvisioned(r.Scheme(), ctrl.LoggerFrom(goctx))), - ) - } else { - return fmt.Errorf("unsupported controller types: %v", controllerBuilder) - } - - return controllerBuilder.Complete(r) + ). + Complete(r) } // KubevirtClusterToKubevirtMachines is a handler.ToRequestsFunc to be used to enqueue @@ -625,7 +529,7 @@ func (r *KubevirtMachineReconciler) KubevirtClusterToKubevirtMachines(ctx gocont panic(fmt.Sprintf("Expected a KubevirtCluster but got a %T", o)) } - cluster, err := util.GetOwnerCluster(ctx, r.Client, c.ObjectMeta) + cluster, err := capiv1beta1.GetOwnerCluster(ctx, r.Client, c.ObjectMeta) switch { case apierrors.IsNotFound(err) || cluster == nil: return result diff --git a/controllers/kubevirtmachine_controller_test.go b/controllers/kubevirtmachine_controller_test.go index 7bfad47d1..0a592678c 100644 --- a/controllers/kubevirtmachine_controller_test.go +++ b/controllers/kubevirtmachine_controller_test.go @@ -21,13 +21,11 @@ import ( "fmt" "time" + "github.com/go-logr/logr" "github.com/golang/mock/gomock" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/pkg/errors" - "k8s.io/utils/ptr" - "sigs.k8s.io/cluster-api/util" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -36,8 +34,8 @@ import ( "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/kubevirt" - clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" - "sigs.k8s.io/cluster-api/util/conditions" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta1" //nolint SA1019 + "sigs.k8s.io/cluster-api/util/deprecated/v1beta1/conditions" //nolint SA1019 ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -117,10 +115,8 @@ var _ = Describe("KubevirtClusterToKubevirtMachines", func() { } fakeClient = fake.NewClientBuilder().WithScheme(testing.SetupScheme()).WithObjects(objects...).Build() kubevirtMachineReconciler = KubevirtMachineReconciler{ - Client: fakeClient, - MachineFactory: kubevirt.DefaultMachineFactory{}, - getOwnerMachine: util.GetOwnerMachine, - getClusterFromMetadata: util.GetClusterFromMetadata, + Client: fakeClient, + MachineFactory: kubevirt.DefaultMachineFactory{}, } ctx = gocontext.Background() @@ -326,7 +322,7 @@ var _ = Describe("reconcile a kubevirt machine", func() { setupClientWithInterceptors := func(machineFactory kubevirt.MachineFactory, objects []client.Object, interceptorFuncs interceptor.Funcs) { machineContext = &context.MachineContext{ - Context: gocontext.Background(), + Context: logr.NewContext(gocontext.Background(), GinkgoLogr), Cluster: cluster, KubevirtCluster: kubevirtCluster, Machine: machine, @@ -336,12 +332,10 @@ var _ = Describe("reconcile a kubevirt machine", func() { fakeClient = fake.NewClientBuilder().WithScheme(testing.SetupScheme()).WithObjects(objects...).WithStatusSubresource(objects...).WithInterceptorFuncs(interceptorFuncs).Build() kubevirtMachineReconciler = KubevirtMachineReconciler{ - Client: fakeClient, - WorkloadCluster: workloadClusterMock, - InfraCluster: infraClusterMock, - MachineFactory: machineFactory, - getOwnerMachine: util.GetOwnerMachine, - getClusterFromMetadata: util.GetClusterFromMetadata, + Client: fakeClient, + WorkloadCluster: workloadClusterMock, + InfraCluster: infraClusterMock, + MachineFactory: machineFactory, } } @@ -767,10 +761,7 @@ var _ = Describe("reconcile a kubevirt machine", func() { Context("update kubevirt machine conditions correctly", func() { It("adds a failed VMProvisionedCondition with reason WaitingForClusterInfrastructureReason when the infrastructure is not ready", func() { - conditions.Set(cluster, metav1.Condition{ - Type: clusterv1.InfrastructureReadyCondition, - Status: metav1.ConditionFalse, - }) + cluster.Status.InfrastructureReady = false objects := []client.Object{ cluster, @@ -833,7 +824,7 @@ var _ = Describe("reconcile a kubevirt machine", func() { It("adds a failed VMProvisionedCondition with reason WaitingForControlPlaneAvailableReason when the control plane is not yet available", func() { machine.Spec.Bootstrap.DataSecretName = nil delete(machine.Labels, clusterv1.MachineControlPlaneNameLabel) - cluster.Status.Initialization.ControlPlaneInitialized = ptr.To(false) + conditions.MarkFalse(cluster, clusterv1.ControlPlaneInitializedCondition, "nonce", clusterv1.ConditionSeverityInfo, "") objects := []client.Object{ cluster, @@ -853,15 +844,12 @@ var _ = Describe("reconcile a kubevirt machine", func() { conditions := machineContext.KubevirtMachine.GetConditions() Expect(conditions[0].Type).To(Equal(infrav1.VMProvisionedCondition)) - Expect(conditions[0].Reason).To(Equal(clusterv1.WaitingForControlPlaneInitializedReason)) + Expect(conditions[0].Reason).To(Equal(clusterv1.WaitingForControlPlaneAvailableReason)) }) It("adds a failed VMProvisionedCondition with reason WaitingForBootstrapDataReason when bootstrap data is not yet available", func() { machine.Spec.Bootstrap.DataSecretName = nil delete(machine.Labels, clusterv1.MachineControlPlaneNameLabel) - conditions.Set(cluster, metav1.Condition{ - Type: clusterv1.AvailableCondition, - Status: metav1.ConditionTrue, - }) + conditions.MarkTrue(cluster, clusterv1.ControlPlaneInitializedCondition) objects := []client.Object{ cluster, @@ -936,7 +924,7 @@ var _ = Describe("reconcile a kubevirt machine", func() { // should expect condition conditions := machineContext.KubevirtMachine.GetConditions() Expect(conditions[0].Type).To(Equal(infrav1.VMProvisionedCondition)) - Expect(conditions[0].Status).To(Equal(metav1.ConditionFalse)) + Expect(conditions[0].Status).To(Equal(corev1.ConditionFalse)) Expect(conditions[0].Reason).To(Equal(infrav1.VMCreateFailedReason)) }) @@ -979,9 +967,9 @@ var _ = Describe("reconcile a kubevirt machine", func() { conditions := machineContext.KubevirtMachine.GetConditions() Expect(conditions[0].Type).To(Equal(infrav1.VMLiveMigratableCondition)) - Expect(conditions[0].Status).To(Equal(metav1.ConditionFalse)) + Expect(conditions[0].Status).To(Equal(corev1.ConditionFalse)) Expect(conditions[1].Type).To(Equal(infrav1.VMProvisionedCondition)) - Expect(conditions[1].Status).To(Equal(metav1.ConditionTrue)) + Expect(conditions[1].Status).To(Equal(corev1.ConditionTrue)) }) It("adds a failed BootstrapExecSucceededCondition with reason BootstrapFailedReason when bootstraping is possible and failed", func() { vmiReadyCondition := kubevirtv1.VirtualMachineInstanceCondition{ @@ -1082,7 +1070,7 @@ var _ = Describe("reconcile a kubevirt machine", func() { conditions := machineContext.KubevirtMachine.GetConditions() Expect(conditions[0].Type).To(Equal(infrav1.BootstrapExecSucceededCondition)) - Expect(conditions[0].Status).To(Equal(metav1.ConditionTrue)) + Expect(conditions[0].Status).To(Equal(corev1.ConditionTrue)) }) It("adds a succeeded VMLiveMigratableCondition", func() { @@ -1138,9 +1126,9 @@ var _ = Describe("reconcile a kubevirt machine", func() { conditions := machineContext.KubevirtMachine.GetConditions() Expect(conditions[0].Type).To(Equal(infrav1.BootstrapExecSucceededCondition)) - Expect(conditions[0].Status).To(Equal(metav1.ConditionTrue)) + Expect(conditions[0].Status).To(Equal(corev1.ConditionTrue)) Expect(conditions[1].Type).To(Equal(infrav1.VMLiveMigratableCondition)) - Expect(conditions[1].Status).To(Equal(metav1.ConditionTrue)) + Expect(conditions[1].Status).To(Equal(corev1.ConditionTrue)) }) It("should requeue on node draining", func() { @@ -1344,11 +1332,9 @@ var _ = Describe("updateNodeProviderID", func() { } fakeClient = fake.NewClientBuilder().WithScheme(testing.SetupScheme()).WithObjects(objects...).Build() kubevirtMachineReconciler = KubevirtMachineReconciler{ - Client: fakeClient, - WorkloadCluster: workloadClusterMock, - InfraCluster: infraClusterMock, - getOwnerMachine: util.GetOwnerMachine, - getClusterFromMetadata: util.GetClusterFromMetadata, + Client: fakeClient, + WorkloadCluster: workloadClusterMock, + InfraCluster: infraClusterMock, } workloadClusterObjects := []client.Object{ diff --git a/e2e/create-cluster_test.go b/e2e/create-cluster_test.go index 673719425..ca88a04cd 100644 --- a/e2e/create-cluster_test.go +++ b/e2e/create-cluster_test.go @@ -3,13 +3,11 @@ package e2e_test import ( "context" "encoding/json" - "errors" "fmt" "os" "os/exec" "path/filepath" "regexp" - "slices" "strings" "time" @@ -25,9 +23,8 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/utils/ptr" kubevirtv1 "kubevirt.io/api/core/v1" - "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/crds" - clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" - "sigs.k8s.io/cluster-api/util/conditions" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta1" //nolint SA1019 + "sigs.k8s.io/cluster-api/util/deprecated/v1beta1/conditions" //nolint SA1019 "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/kind/pkg/cluster/constants" @@ -50,12 +47,6 @@ var _ = Describe("CreateCluster", func() { var namespace string var tenantAccessor tenantClusterAccess - var ( - waitForControlPlane func(ctx context.Context, k8sclient client.Client, namespace string) - postDefaultMHC func(ctx context.Context, k8sclient client.Client, namespace string, clusterName string) - deleteCluster func(ctx context.Context, k8sclient client.Client, namespace string, clusterName string) - ) - BeforeEach(func(ctx context.Context) { var err error @@ -75,19 +66,6 @@ var _ = Describe("CreateCluster", func() { tenantAccessor = newTenantClusterAccess(namespace, tenantKubeconfigFile) Expect(k8sclient.Create(ctx, ns)).To(Succeed()) - - apiVersion, err := getClusterAPIVersion(ctx) - Expect(err).NotTo(HaveOccurred()) - - waitForControlPlane = waitForV1beta2ControlPlane - postDefaultMHC = postDefaultMHCV1Beta2 - deleteCluster = deleteV1Beta2Cluster - - if apiVersion == "v1beta1" { - waitForControlPlane = waitForV1beta1ControlPlane - postDefaultMHC = postDefaultMHCV1Beta1 - deleteCluster = deleteV1Beta1Cluster - } }) AfterEach(func(ctx context.Context) { @@ -109,7 +87,13 @@ var _ = Describe("CreateCluster", func() { Expect(tenantAccessor.stopForwardingTenantAPI()).To(Succeed()) By("removing cluster") - deleteCluster(ctx, k8sclient, namespace, "kvcluster") + cluster := &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: "kvcluster", + }, + } + DeleteAndWait(ctx, k8sclient, cluster, 120) externalInfraSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -203,12 +187,7 @@ var _ = Describe("CreateCluster", func() { } Expect(k8sclient.Update(ctx, kvCluster)).To(Succeed()) - conditions.Set(kvCluster, metav1.Condition{ - Type: infrav1.LoadBalancerAvailableCondition, - Status: metav1.ConditionTrue, - Reason: clusterv1.UpToDateReason, - Message: "", - }) + conditions.MarkTrue(kvCluster, infrav1.LoadBalancerAvailableCondition) kvCluster.Status.Ready = true Expect(k8sclient.Status().Update(ctx, kvCluster)).To(Succeed()) @@ -364,6 +343,36 @@ var _ = Describe("CreateCluster", func() { }, 5*time.Minute, 5*time.Second).Should(Succeed(), "waiting for expected readiness.") } + waitForControlPlane := func(ctx context.Context) { + By("Waiting on cluster's control plane to initialize") + Eventually(func(g Gomega) { + cluster := &clusterv1.Cluster{} + key := client.ObjectKey{Namespace: namespace, Name: "kvcluster"} + g.Expect(k8sclient.Get(ctx, key, cluster)).To(Succeed()) + g.Expect(conditions.IsTrue(cluster, clusterv1.ControlPlaneInitializedCondition)).To( + BeTrue(), + "still waiting on controlPlaneReady condition to be true", + ) + }).WithOffset(1). + WithTimeout(20*time.Minute). + WithPolling(5*time.Second). + Should(Succeed(), "cluster should have control plane initialized") + + By("Waiting on cluster's control plane to be ready") + Eventually(func(g Gomega) { + cluster := &clusterv1.Cluster{} + key := client.ObjectKey{Namespace: namespace, Name: "kvcluster"} + g.Expect(k8sclient.Get(ctx, key, cluster)).To(Succeed()) + g.Expect(conditions.IsTrue(cluster, clusterv1.ControlPlaneReadyCondition)).To( + BeTrue(), + "still waiting on controlPlaneInitialized condition to be true", + ) + }).WithOffset(1). + WithTimeout(15*time.Minute). + WithPolling(5*time.Second). + Should(Succeed(), "cluster should have control plane initialized") + } + injectKubevirtClusterExternallyManagedAnnotation := func(yamlStr string) string { strs := strings.Split(yamlStr, "\n") @@ -437,6 +446,47 @@ var _ = Describe("CreateCluster", func() { Should(Succeed(), printObjFunc(obj)) } + postDefaultMHC := func(ctx context.Context, clusterName string) { + maxUnhealthy := intstr.FromString("100%") + mhc := &clusterv1.MachineHealthCheck{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testmhc", + Namespace: namespace, + }, + Spec: clusterv1.MachineHealthCheckSpec{ + ClusterName: clusterName, + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "cluster.x-k8s.io/cluster-name": clusterName, + }, + }, + MaxUnhealthy: &maxUnhealthy, + + UnhealthyConditions: []clusterv1.UnhealthyCondition{ + { + Type: corev1.NodeReady, + Status: corev1.ConditionFalse, + Timeout: metav1.Duration{ + Duration: 5 * time.Minute, + }, + }, + { + Type: corev1.NodeReady, + Status: corev1.ConditionUnknown, + Timeout: metav1.Duration{ + Duration: 5 * time.Minute, + }, + }, + }, + NodeStartupTimeout: &metav1.Duration{ + Duration: 10 * time.Minute, + }, + }, + } + + Expect(k8sclient.Create(ctx, mhc)).To(Succeed()) + } + It("creates a simple cluster with ephemeral VMs", Label("ephemeralVMs"), func(ctx context.Context) { By("generating cluster manifests from example template") cmd := exec.Command(ClusterctlPath, "generate", "cluster", "kvcluster", @@ -457,7 +507,7 @@ var _ = Describe("CreateCluster", func() { RunCmd(cmd) By("Waiting for control plane") - waitForControlPlane(ctx, k8sclient, namespace) + waitForControlPlane(ctx) By("Waiting on kubevirt machines to bootstrap") waitForBootstrappedMachines(ctx) @@ -494,7 +544,7 @@ var _ = Describe("CreateCluster", func() { RunCmd(cmd) By("Waiting for control plane") - waitForControlPlane(ctx, k8sclient, namespace) + waitForControlPlane(ctx) By("Waiting on kubevirt machines to bootstrap") waitForBootstrappedMachines(ctx) @@ -506,7 +556,7 @@ var _ = Describe("CreateCluster", func() { waitForTenantAccess(ctx, 2) By("creating machine health check") - postDefaultMHC(ctx, k8sclient, namespace, "kvcluster") + postDefaultMHC(ctx, "kvcluster") // trigger remediation by marking a running VMI as being in a failed state By("Selecting a worker node to remediate") @@ -565,7 +615,7 @@ var _ = Describe("CreateCluster", func() { RunCmd(cmd) By("Waiting for control plane") - waitForControlPlane(ctx, k8sclient, namespace) + waitForControlPlane(ctx) By("Waiting on kubevirt machines to bootstrap") waitForBootstrappedMachines(ctx) @@ -577,7 +627,7 @@ var _ = Describe("CreateCluster", func() { waitForTenantAccess(ctx, 2) By("creating machine health check") - postDefaultMHC(ctx, k8sclient, namespace, "kvcluster") + postDefaultMHC(ctx, "kvcluster") // trigger remediation by putting the VMI in a permanent stopped state By("Selecting new worker node to remediate") @@ -639,7 +689,7 @@ var _ = Describe("CreateCluster", func() { markExternalKubeVirtClusterReady(ctx, "kvcluster", namespace) By("Waiting for control plane") - waitForControlPlane(ctx, k8sclient, namespace) + waitForControlPlane(ctx) By("Waiting on kubevirt machines to be ready") waitForMachineReadiness(ctx, 2, 0) @@ -675,7 +725,7 @@ var _ = Describe("CreateCluster", func() { RunCmd(cmd) By("Waiting for control plane") - waitForControlPlane(ctx, k8sclient, namespace) + waitForControlPlane(ctx) By("Waiting on kubevirt machines to bootstrap") waitForBootstrappedMachines(ctx) @@ -807,7 +857,7 @@ var _ = Describe("CreateCluster", func() { waitForBootstrappedMachines(ctx) By("waiting for control plane") - waitForControlPlane(ctx, k8sclient, namespace) + waitForControlPlane(ctx) }) }) @@ -966,25 +1016,3 @@ func printObjFunc(obj any) func() string { return sb.String() } } - -// sorted by priority -var supportedVersions = []string{"v1beta2", "v1beta1"} - -func getClusterAPIVersion(ctx context.Context) (string, error) { - versions, err := crds.GetSupportedVersions(ctx, k8sclient, "clusters.cluster.x-k8s.io") - if err != nil { - return "", err - } - - if len(versions) == 0 { - return "", errors.New("no cluster-api version found") - } - - for _, version := range supportedVersions { - if slices.Contains(versions, version) { - return version, nil - } - } - - return "", errors.New("no supported cluster-api version found") -} diff --git a/e2e/v1beta1_helpers_test.go b/e2e/v1beta1_helpers_test.go deleted file mode 100644 index bb60affa5..000000000 --- a/e2e/v1beta1_helpers_test.go +++ /dev/null @@ -1,98 +0,0 @@ -package e2e_test - -import ( - "context" - "time" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/intstr" - clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta1" //nolint SA1019 - "sigs.k8s.io/cluster-api/util/deprecated/v1beta1/conditions" //nolint SA1019 - "sigs.k8s.io/controller-runtime/pkg/client" -) - -func waitForV1beta1ControlPlane(ctx context.Context, k8sclient client.Client, namespace string) { - By("Waiting on cluster's control plane to initialize") - Eventually(func(g Gomega) { - cluster := &clusterv1.Cluster{} - key := client.ObjectKey{Namespace: namespace, Name: "kvcluster"} - g.Expect(k8sclient.Get(ctx, key, cluster)).To(Succeed()) - g.Expect(conditions.IsTrue(cluster, clusterv1.ControlPlaneInitializedCondition)).To( - BeTrue(), - "still waiting on controlPlaneReady condition to be true", - ) - }).WithOffset(1). - WithTimeout(20*time.Minute). - WithPolling(5*time.Second). - Should(Succeed(), "cluster should have control plane initialized") - - By("Waiting on cluster's control plane to be ready") - Eventually(func(g Gomega) { - cluster := &clusterv1.Cluster{} - key := client.ObjectKey{Namespace: namespace, Name: "kvcluster"} - g.Expect(k8sclient.Get(ctx, key, cluster)).To(Succeed()) - g.Expect(conditions.IsTrue(cluster, clusterv1.ControlPlaneReadyCondition)).To( - BeTrue(), - "still waiting on controlPlaneInitialized condition to be true", - ) - }).WithOffset(1). - WithTimeout(15*time.Minute). - WithPolling(5*time.Second). - Should(Succeed(), "cluster should have control plane initialized") -} - -func postDefaultMHCV1Beta1(ctx context.Context, k8sclient client.Client, namespace string, clusterName string) { - maxUnhealthy := intstr.FromString("100%") - mhc := &clusterv1.MachineHealthCheck{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testmhc", - Namespace: namespace, - }, - Spec: clusterv1.MachineHealthCheckSpec{ - ClusterName: clusterName, - Selector: metav1.LabelSelector{ - MatchLabels: map[string]string{ - "cluster.x-k8s.io/cluster-name": clusterName, - }, - }, - MaxUnhealthy: &maxUnhealthy, - - UnhealthyConditions: []clusterv1.UnhealthyCondition{ - { - Type: corev1.NodeReady, - Status: corev1.ConditionFalse, - Timeout: metav1.Duration{ - Duration: 5 * time.Minute, - }, - }, - { - Type: corev1.NodeReady, - Status: corev1.ConditionUnknown, - Timeout: metav1.Duration{ - Duration: 5 * time.Minute, - }, - }, - }, - NodeStartupTimeout: &metav1.Duration{ - Duration: 10 * time.Minute, - }, - }, - } - - Expect(k8sclient.Create(ctx, mhc)).To(Succeed()) -} - -func deleteV1Beta1Cluster(ctx context.Context, k8sclient client.Client, namespace string, clusterName string) { - GinkgoHelper() - - cluster := &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: namespace, - Name: clusterName, - }, - } - DeleteAndWait(ctx, k8sclient, cluster, 120) -} diff --git a/e2e/v1beta2_helpers_test.go b/e2e/v1beta2_helpers_test.go deleted file mode 100644 index fa8f89eac..000000000 --- a/e2e/v1beta2_helpers_test.go +++ /dev/null @@ -1,98 +0,0 @@ -package e2e_test - -import ( - "context" - "time" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/utils/ptr" - clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" - "sigs.k8s.io/cluster-api/util/conditions" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -func waitForV1beta2ControlPlane(ctx context.Context, k8sclient client.Client, namespace string) { - By("Waiting on cluster's control plane to initialize") - Eventually(func(g Gomega) { - cluster := &clusterv1.Cluster{} - key := client.ObjectKey{Namespace: namespace, Name: "kvcluster"} - g.Expect(k8sclient.Get(ctx, key, cluster)).To(Succeed()) - g.Expect(ptr.Equal(cluster.Status.Initialization.ControlPlaneInitialized, ptr.To(true))).To( - BeTrue(), - "still waiting on controlPlaneInitialized condition to be true", - ) - }).WithOffset(1). - WithTimeout(20*time.Minute). - WithPolling(5*time.Second). - Should(Succeed(), "cluster should have control plane initialized") - - By("Waiting on cluster's control plane to be ready") - Eventually(func(g Gomega) { - cluster := &clusterv1.Cluster{} - key := client.ObjectKey{Namespace: namespace, Name: "kvcluster"} - g.Expect(k8sclient.Get(ctx, key, cluster)).To(Succeed()) - g.Expect(conditions.IsTrue(cluster, clusterv1.ClusterControlPlaneAvailableCondition)).To( - BeTrue(), - "still waiting on controlplaneAvailable condition to be true", - ) - }).WithOffset(1). - WithTimeout(15*time.Minute). - WithPolling(5*time.Second). - Should(Succeed(), "cluster should have control plane available") -} - -func postDefaultMHCV1Beta2(ctx context.Context, k8sclient client.Client, namespace string, clusterName string) { - GinkgoHelper() - maxUnhealthy := intstr.FromString("100%") - mhc := &clusterv1.MachineHealthCheck{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testmhc", - Namespace: namespace, - }, - Spec: clusterv1.MachineHealthCheckSpec{ - ClusterName: clusterName, - Selector: metav1.LabelSelector{ - MatchLabels: map[string]string{ - "cluster.x-k8s.io/cluster-name": clusterName, - }, - }, - Checks: clusterv1.MachineHealthCheckChecks{ - UnhealthyNodeConditions: []clusterv1.UnhealthyNodeCondition{ - { - Type: corev1.NodeReady, - Status: corev1.ConditionFalse, - TimeoutSeconds: ptr.To(int32(300)), - }, - { - Type: corev1.NodeReady, - Status: corev1.ConditionUnknown, - TimeoutSeconds: ptr.To(int32(300)), - }, - }, - }, - Remediation: clusterv1.MachineHealthCheckRemediation{ - TriggerIf: clusterv1.MachineHealthCheckRemediationTriggerIf{ - UnhealthyLessThanOrEqualTo: &maxUnhealthy, - }, - }, - }, - } - - Expect(k8sclient.Create(ctx, mhc)).To(Succeed()) -} - -func deleteV1Beta2Cluster(ctx context.Context, k8sclient client.Client, namespace string, clusterName string) { - GinkgoHelper() - - cluster := &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: namespace, - Name: clusterName, - }, - } - DeleteAndWait(ctx, k8sclient, cluster, 120) -} diff --git a/pkg/capiv1beta1/resources.go b/pkg/capiv1beta1/resources.go index 05b9230ac..f18c5187a 100644 --- a/pkg/capiv1beta1/resources.go +++ b/pkg/capiv1beta1/resources.go @@ -5,9 +5,9 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" - clusterv1v1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1" //nolint SA1019 - clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta1" //nolint SA1019 "sigs.k8s.io/cluster-api/util" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/kind/pkg/errors" ) @@ -19,7 +19,7 @@ func GetOwnerMachine(ctx context.Context, c client.Client, obj metav1.ObjectMeta if err != nil { return nil, err } - if ref.Kind == "Machine" && gv.Group == clusterv1v1beta1.GroupVersion.Group { + if ref.Kind == "Machine" && gv.Group == clusterv1.GroupVersion.Group { return GetMachineByName(ctx, c, obj.Namespace, ref.Name) } } @@ -28,18 +28,12 @@ func GetOwnerMachine(ctx context.Context, c client.Client, obj metav1.ObjectMeta // GetMachineByName finds and return a Machine object using the specified params. func GetMachineByName(ctx context.Context, c client.Client, namespace, name string) (*clusterv1.Machine, error) { - m := &clusterv1v1beta1.Machine{} + m := &clusterv1.Machine{} key := client.ObjectKey{Name: name, Namespace: namespace} if err := c.Get(ctx, key, m); err != nil { return nil, err } - - o := &clusterv1.Machine{} - if err := m.ConvertTo(o); err != nil { - return nil, err - } - - return o, nil + return m, nil } // GetOwnerCluster returns the Cluster object owning the current resource. @@ -52,7 +46,7 @@ func GetOwnerCluster(ctx context.Context, c client.Client, obj metav1.ObjectMeta if err != nil { return nil, err } - if gv.Group == clusterv1v1beta1.GroupVersion.Group { + if gv.Group == clusterv1.GroupVersion.Group { return GetClusterByName(ctx, c, obj.Namespace, ref.Name) } } @@ -69,7 +63,7 @@ func GetClusterFromMetadata(ctx context.Context, c client.Client, obj metav1.Obj // GetClusterByName finds and return a Cluster object using the specified params. func GetClusterByName(ctx context.Context, c client.Client, namespace, name string) (*clusterv1.Cluster, error) { - cluster := &clusterv1v1beta1.Cluster{} + cluster := &clusterv1.Cluster{} key := client.ObjectKey{ Namespace: namespace, Name: name, @@ -79,11 +73,10 @@ func GetClusterByName(ctx context.Context, c client.Client, namespace, name stri return nil, errors.Wrapf(err, "failed to get Cluster/%s", name) } - v1Cluster := &clusterv1.Cluster{} - - if err := cluster.ConvertTo(v1Cluster); err != nil { - return nil, errors.Wrapf(err, "failed to convert Cluster/%s", name) - } + return cluster, nil +} - return v1Cluster, nil +func IsControlPlaneMachine(machine *clusterv1.Machine) bool { + _, ok := machine.Labels[clusterv1.MachineControlPlaneLabel] + return ok } diff --git a/pkg/context/cluster_context.go b/pkg/context/cluster_context.go index 9159dd15e..49d6d71ae 100644 --- a/pkg/context/cluster_context.go +++ b/pkg/context/cluster_context.go @@ -21,10 +21,9 @@ import ( "fmt" "github.com/go-logr/logr" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" - "sigs.k8s.io/cluster-api/util/conditions" - "sigs.k8s.io/cluster-api/util/patch" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta1" //nolint SA1019 + "sigs.k8s.io/cluster-api/util/deprecated/v1beta1/conditions" //nolint SA1019 + "sigs.k8s.io/cluster-api/util/deprecated/v1beta1/patch" //nolint SA1019 infrav1 "sigs.k8s.io/cluster-api-provider-kubevirt/api/v1alpha1" ) @@ -42,58 +41,22 @@ func (c *ClusterContext) String() string { return fmt.Sprintf("%s %s/%s", c.KubevirtCluster.GroupVersionKind(), c.KubevirtCluster.Namespace, c.KubevirtCluster.Name) } -// WithStepCounterIf is a custom merge strategy that adds a step counter to the message. -type WithStepCounterIf struct { - defaultStrategy conditions.MergeStrategy - addStepCounter bool -} - -// Merge merges the conditions and adds a step counter to the message if addStepCounter is true. -func (s *WithStepCounterIf) Merge(op conditions.MergeOperation, conditions []conditions.ConditionWithOwnerInfo, conditionTypes []string) (metav1.ConditionStatus, string, string, error) { - status, reason, message, err := s.defaultStrategy.Merge(op, conditions, conditionTypes) - if err != nil { - return status, reason, message, err - } - - if s.addStepCounter { - // 조건 중 True인 개수를 계산 - trueCount := 0 - for _, c := range conditions { - if c.Status == metav1.ConditionTrue { - trueCount++ - } - } - totalCount := len(conditionTypes) - message = fmt.Sprintf("%s (%d/%d conditions met)", message, trueCount, totalCount) - } - - return status, reason, message, nil -} - // PatchKubevirtCluster patches the KubevirtCluster object and status. func (c *ClusterContext) PatchKubevirtCluster(patchHelper *patch.Helper) error { // Always update the readyCondition by summarizing the state of other conditions. // A step counter is added to represent progress during the provisioning process (instead we are hiding it during the deletion process). - if err := conditions.SetSummaryCondition( - c.KubevirtCluster, - c.KubevirtCluster, - clusterv1.ReadyCondition, - conditions.ForConditionTypes{infrav1.LoadBalancerAvailableCondition}, - conditions.CustomMergeStrategy{ - MergeStrategy: &WithStepCounterIf{ - defaultStrategy: conditions.DefaultMergeStrategy(), - addStepCounter: c.KubevirtCluster.DeletionTimestamp.IsZero(), - }, - }, - ); err != nil { - return fmt.Errorf("failed to set summary condition: %w", err) - } + conditions.SetSummary(c.KubevirtCluster, + conditions.WithConditions( + infrav1.LoadBalancerAvailableCondition, + ), + conditions.WithStepCounterIf(c.KubevirtCluster.DeletionTimestamp.IsZero()), + ) // Patch the object, ignoring conflicts on the conditions owned by this controller. return patchHelper.Patch( c.Context, c.KubevirtCluster, - patch.WithOwnedConditions{Conditions: []string{ + patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{ clusterv1.ReadyCondition, infrav1.LoadBalancerAvailableCondition, }}, diff --git a/pkg/context/machine_context.go b/pkg/context/machine_context.go index 7d35ac685..064b81dd4 100644 --- a/pkg/context/machine_context.go +++ b/pkg/context/machine_context.go @@ -24,9 +24,9 @@ import ( "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" - clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" - "sigs.k8s.io/cluster-api/util/conditions" - "sigs.k8s.io/cluster-api/util/patch" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta1" //nolint SA1019 + "sigs.k8s.io/cluster-api/util/deprecated/v1beta1/conditions" //nolint SA1019 + "sigs.k8s.io/cluster-api/util/deprecated/v1beta1/patch" //nolint SA1019 infrav1 "sigs.k8s.io/cluster-api-provider-kubevirt/api/v1alpha1" ) @@ -61,23 +61,18 @@ func (c *MachineContext) String() string { func (c *MachineContext) PatchKubevirtMachine(patchHelper *patch.Helper) error { // Always update the readyCondition by summarizing the state of other conditions. // A step counter is added to represent progress during the provisioning process (instead we are hiding the step counter during the deletion process). - if err := conditions.SetSummaryCondition( - c.KubevirtMachine, - c.KubevirtMachine, - clusterv1.ReadyCondition, - conditions.ForConditionTypes{ + conditions.SetSummary(c.KubevirtMachine, + conditions.WithConditions( infrav1.VMProvisionedCondition, infrav1.BootstrapExecSucceededCondition, - }, - ); err != nil { - return fmt.Errorf("failed to set summary condition: %w", err) - } + ), + ) // Patch the object, ignoring conflicts on the conditions owned by this controller. return patchHelper.Patch( c.Context, c.KubevirtMachine, - patch.WithOwnedConditions{Conditions: []string{ + patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{ clusterv1.ReadyCondition, infrav1.VMProvisionedCondition, infrav1.BootstrapExecSucceededCondition, diff --git a/pkg/crds/crds.go b/pkg/crds/crds.go deleted file mode 100644 index f34c48ab7..000000000 --- a/pkg/crds/crds.go +++ /dev/null @@ -1,32 +0,0 @@ -package crds - -import ( - "context" - - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -func GetSupportedVersions(ctx context.Context, cli client.Reader, name string) ([]string, error) { - crd := &apiextensionsv1.CustomResourceDefinition{} - err := cli.Get(ctx, client.ObjectKey{Name: name}, crd) - if err != nil { - if apierrors.IsNotFound(err) { - return nil, nil - } - - return nil, err - } - - if crd == nil || len(crd.Spec.Versions) == 0 { - return nil, nil - } - - versions := make([]string, 0, len(crd.Spec.Versions)) - for _, version := range crd.Spec.Versions { - versions = append(versions, version.Name) - } - - return versions, nil -} diff --git a/pkg/crds/crds_test.go b/pkg/crds/crds_test.go deleted file mode 100644 index 4ad394a1c..000000000 --- a/pkg/crds/crds_test.go +++ /dev/null @@ -1,84 +0,0 @@ -package crds_test - -import ( - "context" - "testing" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/crds" - "sigs.k8s.io/controller-runtime/pkg/client/fake" -) - -func TestCRDs(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "CRDs Suite") -} - -const crdName = "crd-name" - -var _ = Describe("CRDs", func() { - DescribeTable("version list", func(ctx context.Context, crd *apiextensionsv1.CustomResourceDefinition, expected []string) { - s := runtime.NewScheme() - Expect(apiextensionsv1.AddToScheme(s)).To(Succeed()) - cl := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(crd).Build() - - versions, err := crds.GetSupportedVersions(ctx, cl, crdName) - Expect(err).NotTo(HaveOccurred()) - Expect(versions).To(Equal(expected)) - }, - Entry("should return empty list if not found", &apiextensionsv1.CustomResourceDefinition{ - TypeMeta: metav1.TypeMeta{ - Kind: "CustomResourceDefinition", - APIVersion: apiextensionsv1.SchemeGroupVersion.String(), - }, - }, nil), - Entry("should return empty list if no versions", &apiextensionsv1.CustomResourceDefinition{ - TypeMeta: metav1.TypeMeta{ - Kind: "CustomResourceDefinition", - APIVersion: apiextensionsv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: crdName, - }, - }, nil), - Entry("should return list of one version", &apiextensionsv1.CustomResourceDefinition{ - TypeMeta: metav1.TypeMeta{ - Kind: "CustomResourceDefinition", - APIVersion: apiextensionsv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: crdName, - }, - Spec: apiextensionsv1.CustomResourceDefinitionSpec{ - Versions: []apiextensionsv1.CustomResourceDefinitionVersion{ - { - Name: "v1beta1", - }, - }, - }, - }, []string{"v1beta1"}), - Entry("should return list of two version", &apiextensionsv1.CustomResourceDefinition{ - TypeMeta: metav1.TypeMeta{ - Kind: "CustomResourceDefinition", - APIVersion: apiextensionsv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: crdName, - }, - Spec: apiextensionsv1.CustomResourceDefinitionSpec{ - Versions: []apiextensionsv1.CustomResourceDefinitionVersion{ - { - Name: "v1beta1", - }, - { - Name: "v1beta2", - }, - }, - }, - }, []string{"v1beta1", "v1beta2"}), - ) -}) diff --git a/pkg/kubevirt/utils.go b/pkg/kubevirt/utils.go index 137a3345b..e5f579195 100644 --- a/pkg/kubevirt/utils.go +++ b/pkg/kubevirt/utils.go @@ -22,9 +22,9 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kubevirtv1 "kubevirt.io/api/core/v1" - "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/kind/pkg/cluster/constants" + "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/capiv1beta1" "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/context" ) @@ -166,7 +166,7 @@ func buildVirtualMachineInstanceTemplate(ctx *context.MachineContext) *kubevirtv // nodeRole returns the role of this node ("control-plane" or "worker"). func nodeRole(ctx *context.MachineContext) string { - if util.IsControlPlaneMachine(ctx.Machine) { + if capiv1beta1.IsControlPlaneMachine(ctx.Machine) { return constants.ControlPlaneNodeRoleValue } return constants.WorkerNodeRoleValue diff --git a/pkg/testing/common.go b/pkg/testing/common.go index ad1b3fcbe..16eef05dd 100644 --- a/pkg/testing/common.go +++ b/pkg/testing/common.go @@ -8,7 +8,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" kubevirtv1 "kubevirt.io/api/core/v1" cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1" - clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta1" //nolint SA1019 infrav1 "sigs.k8s.io/cluster-api-provider-kubevirt/api/v1alpha1" ) @@ -21,10 +21,11 @@ func NewCluster(clusterName string, kubevirtCluster *infrav1.KubevirtCluster) *c }, } if kubevirtCluster != nil { - cluster.Spec.InfrastructureRef = clusterv1.ContractVersionedObjectReference{ - Name: kubevirtCluster.Name, - Kind: kubevirtCluster.Kind, - APIGroup: kubevirtCluster.GroupVersionKind().Group, + cluster.Spec.InfrastructureRef = &corev1.ObjectReference{ + Name: kubevirtCluster.Name, + Namespace: kubevirtCluster.Namespace, + Kind: kubevirtCluster.Kind, + APIVersion: kubevirtCluster.GroupVersionKind().GroupVersion().String(), } } return cluster @@ -117,10 +118,11 @@ func NewMachine(clusterName, machineName string, kubevirtMachine *infrav1.Kubevi }, } if kubevirtMachine != nil { - machine.Spec.InfrastructureRef = clusterv1.ContractVersionedObjectReference{ - Name: kubevirtMachine.Name, - Kind: kubevirtMachine.Kind, - APIGroup: kubevirtMachine.GroupVersionKind().Group, + machine.Spec.InfrastructureRef = corev1.ObjectReference{ + Name: kubevirtMachine.Name, + Namespace: kubevirtMachine.Namespace, + Kind: kubevirtMachine.Kind, + APIVersion: kubevirtMachine.GroupVersionKind().GroupVersion().String(), } } return machine From 88ae09772cb89f1bd4a4de906f1efd2637647705 Mon Sep 17 00:00:00 2001 From: Enrique Llorente Date: Fri, 23 Jan 2026 14:04:28 +0100 Subject: [PATCH 2/5] Expose dual-stack addresses in KubevirtMachine status Add support for exposing all IP addresses (IPv4 and IPv6) in the KubevirtMachine status instead of just the primary IP. This fixes CSR auto-approval on dual-stack hosted control plane clusters where the machine-approver needs to see both IPv4 and IPv6 addresses. Changes: - Add Addresses() method to MachineInterface to retrieve all IPs from all VMI network interfaces - Update controller to populate multiple InternalIP and ExternalIP entries for each address - Add unit tests for dual-stack address handling Co-Authored-By: Claude Opus 4.5 Signed-off-by: Enrique Llorente --- controllers/kubevirtmachine_controller.go | 37 ++++++++++++------- .../kubevirtmachine_controller_test.go | 3 ++ pkg/kubevirt/machine.go | 14 +++++++ pkg/kubevirt/machine_factory.go | 2 + pkg/kubevirt/machine_test.go | 37 +++++++++++++++++++ .../mock/machine_factory_generated.go | 14 +++++++ 6 files changed, 94 insertions(+), 13 deletions(-) diff --git a/controllers/kubevirtmachine_controller.go b/controllers/kubevirtmachine_controller.go index bb16a5bf9..5e93f99df 100644 --- a/controllers/kubevirtmachine_controller.go +++ b/controllers/kubevirtmachine_controller.go @@ -325,25 +325,36 @@ func (r *KubevirtMachineReconciler) reconcileNormal(ctx *context.MachineContext) ctx.Logger.Info("Underlying VM has boostrapped.") } - ctx.KubevirtMachine.Status.Addresses = []clusterv1.MachineAddress{ + // Build the machine addresses list with all IPs for dual-stack support + machineAddresses := []clusterv1.MachineAddress{ { Type: clusterv1.MachineHostName, Address: ctx.KubevirtMachine.Name, }, - { - Type: clusterv1.MachineInternalIP, - Address: ipAddress, - }, - { - Type: clusterv1.MachineExternalIP, - Address: ipAddress, - }, - { - Type: clusterv1.MachineInternalDNS, - Address: ctx.KubevirtMachine.Name, - }, } + // Add all IP addresses from the VMI interfaces (supports dual-stack IPv4/IPv6) + allAddresses := externalMachine.Addresses() + for _, addr := range allAddresses { + machineAddresses = append(machineAddresses, + clusterv1.MachineAddress{ + Type: clusterv1.MachineInternalIP, + Address: addr, + }, + clusterv1.MachineAddress{ + Type: clusterv1.MachineExternalIP, + Address: addr, + }, + ) + } + + machineAddresses = append(machineAddresses, clusterv1.MachineAddress{ + Type: clusterv1.MachineInternalDNS, + Address: ctx.KubevirtMachine.Name, + }) + + ctx.KubevirtMachine.Status.Addresses = machineAddresses + if ctx.KubevirtMachine.Spec.ProviderID == nil || *ctx.KubevirtMachine.Spec.ProviderID == "" { providerID, err := externalMachine.GenerateProviderID() if err != nil { diff --git a/controllers/kubevirtmachine_controller_test.go b/controllers/kubevirtmachine_controller_test.go index 0a592678c..85c059ec6 100644 --- a/controllers/kubevirtmachine_controller_test.go +++ b/controllers/kubevirtmachine_controller_test.go @@ -956,6 +956,7 @@ var _ = Describe("reconcile a kubevirt machine", func() { machineMock.EXPECT().Address().Return("1.1.1.1").Times(1) machineMock.EXPECT().SupportsCheckingIsBootstrapped().Return(false).Times(1) machineMock.EXPECT().DrainNodeIfNeeded(gomock.Any()).Return(time.Duration(0), nil) + machineMock.EXPECT().Addresses().Return([]string{"1.1.1.1"}).Times(1) machineMock.EXPECT().IsLiveMigratable().Return(false, "", "", nil).Times(1) machineFactoryMock.EXPECT().NewMachine(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(machineMock, nil).Times(1) @@ -1056,6 +1057,7 @@ var _ = Describe("reconcile a kubevirt machine", func() { machineMock.EXPECT().SupportsCheckingIsBootstrapped().Return(true) machineMock.EXPECT().IsBootstrapped().Return(true) machineMock.EXPECT().DrainNodeIfNeeded(gomock.Any()).Return(time.Duration(0), nil) + machineMock.EXPECT().Addresses().Return([]string{"1.1.1.1"}).Times(1) machineMock.EXPECT().IsLiveMigratable().Return(false, "", "", nil).Times(1) machineFactoryMock.EXPECT().NewMachine(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(machineMock, nil).Times(1) @@ -1112,6 +1114,7 @@ var _ = Describe("reconcile a kubevirt machine", func() { machineMock.EXPECT().SupportsCheckingIsBootstrapped().Return(true) machineMock.EXPECT().IsBootstrapped().Return(true) machineMock.EXPECT().DrainNodeIfNeeded(gomock.Any()).Return(time.Duration(0), nil) + machineMock.EXPECT().Addresses().Return([]string{"1.1.1.1"}).Times(1) machineMock.EXPECT().IsLiveMigratable().Return(true, "", "", nil).Times(1) machineFactoryMock.EXPECT().NewMachine(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(machineMock, nil).Times(1) diff --git a/pkg/kubevirt/machine.go b/pkg/kubevirt/machine.go index 43401cca2..e42619263 100644 --- a/pkg/kubevirt/machine.go +++ b/pkg/kubevirt/machine.go @@ -232,6 +232,20 @@ func (m *Machine) Address() string { return "" } +// Addresses returns all IP addresses of the VM (for dual-stack support). +// It collects IPs from all network interfaces, supporting both IPv4 and IPv6. +func (m *Machine) Addresses() []string { + if m.vmiInstance == nil { + return nil + } + + var addresses []string + for _, iface := range m.vmiInstance.Status.Interfaces { + addresses = append(addresses, iface.IPs...) + } + return addresses +} + // IsReady checks if the VM is ready func (m *Machine) IsReady() bool { return m.hasReadyCondition() diff --git a/pkg/kubevirt/machine_factory.go b/pkg/kubevirt/machine_factory.go index d5724dfa0..580958883 100644 --- a/pkg/kubevirt/machine_factory.go +++ b/pkg/kubevirt/machine_factory.go @@ -29,6 +29,8 @@ type MachineInterface interface { IsLiveMigratable() (bool, string, string, error) // Address returns the IP address of the VM. Address() string + // Addresses returns all IP addresses of the VM (for dual-stack support). + Addresses() []string // SupportsCheckingIsBootstrapped checks if we have a method of checking // that this bootstrapper has completed. SupportsCheckingIsBootstrapped() bool diff --git a/pkg/kubevirt/machine_test.go b/pkg/kubevirt/machine_test.go index eb6bf4ef6..7b766a64d 100644 --- a/pkg/kubevirt/machine_test.go +++ b/pkg/kubevirt/machine_test.go @@ -118,6 +118,12 @@ var _ = Describe("Without KubeVirt VM running", func() { Expect(externalMachine.Address()).To(Equal("")) }) + It("Addresses should return empty slice when VMI is nil", func() { + externalMachine, err := defaultTestMachine(machineContext, namespace, fakeClient, fakeVMCommandExecutor, []byte{}) + Expect(err).NotTo(HaveOccurred()) + Expect(externalMachine.Addresses()).To(BeEmpty()) + }) + It("IsReady should return false", func() { externalMachine, err := defaultTestMachine(machineContext, namespace, fakeClient, fakeVMCommandExecutor, []byte{}) Expect(err).NotTo(HaveOccurred()) @@ -262,6 +268,37 @@ var _ = Describe("With KubeVirt VM running", func() { Expect(externalMachine.Address()).To(Equal(virtualMachineInstance.Status.Interfaces[0].IP)) }) + It("Addresses should return all IPs from interfaces", func() { + // Set up dual-stack IPs on the interface + virtualMachineInstance.Status.Interfaces[0].IPs = []string{"1.1.1.1", "2001:db8::1"} + fakeClient = fake.NewClientBuilder().WithScheme(testing.SetupScheme()).WithRuntimeObjects(cluster, kubevirtMachine, virtualMachine, virtualMachineInstance, bootstrapDataSecret).Build() + + externalMachine, err := defaultTestMachine(machineContext, namespace, fakeClient, fakeVMCommandExecutor, []byte(sshKey)) + Expect(err).NotTo(HaveOccurred()) + addresses := externalMachine.Addresses() + Expect(addresses).To(HaveLen(2)) + Expect(addresses).To(ContainElement("1.1.1.1")) + Expect(addresses).To(ContainElement("2001:db8::1")) + }) + + It("Addresses should return IPs from multiple interfaces", func() { + // Set up multiple interfaces with IPs + virtualMachineInstance.Status.Interfaces = []kubevirtv1.VirtualMachineInstanceNetworkInterface{ + {IP: "1.1.1.1", IPs: []string{"1.1.1.1", "2001:db8::1"}}, + {IP: "10.0.0.1", IPs: []string{"10.0.0.1", "2001:db8::2"}}, + } + fakeClient = fake.NewClientBuilder().WithScheme(testing.SetupScheme()).WithRuntimeObjects(cluster, kubevirtMachine, virtualMachine, virtualMachineInstance, bootstrapDataSecret).Build() + + externalMachine, err := defaultTestMachine(machineContext, namespace, fakeClient, fakeVMCommandExecutor, []byte(sshKey)) + Expect(err).NotTo(HaveOccurred()) + addresses := externalMachine.Addresses() + Expect(addresses).To(HaveLen(4)) + Expect(addresses).To(ContainElement("1.1.1.1")) + Expect(addresses).To(ContainElement("2001:db8::1")) + Expect(addresses).To(ContainElement("10.0.0.1")) + Expect(addresses).To(ContainElement("2001:db8::2")) + }) + It("IsReady should return true", func() { externalMachine, err := defaultTestMachine(machineContext, namespace, fakeClient, fakeVMCommandExecutor, []byte(sshKey)) Expect(err).NotTo(HaveOccurred()) diff --git a/pkg/kubevirt/mock/machine_factory_generated.go b/pkg/kubevirt/mock/machine_factory_generated.go index 2846f6bee..51335aeba 100644 --- a/pkg/kubevirt/mock/machine_factory_generated.go +++ b/pkg/kubevirt/mock/machine_factory_generated.go @@ -55,6 +55,20 @@ func (mr *MockMachineInterfaceMockRecorder) Address() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Address", reflect.TypeOf((*MockMachineInterface)(nil).Address)) } +// Addresses mocks base method. +func (m *MockMachineInterface) Addresses() []string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Addresses") + ret0, _ := ret[0].([]string) + return ret0 +} + +// Addresses indicates an expected call of Addresses. +func (mr *MockMachineInterfaceMockRecorder) Addresses() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Addresses", reflect.TypeOf((*MockMachineInterface)(nil).Addresses)) +} + // Create mocks base method. func (m *MockMachineInterface) Create(ctx context.Context) error { m.ctrl.T.Helper() From 65f182e3c9a056adeac94e68846c8d0fb7601526 Mon Sep 17 00:00:00 2001 From: Nahshon Unna Tsameret Date: Wed, 25 Feb 2026 10:03:18 +0200 Subject: [PATCH 3/5] fix: use guest node name when draining node on VM eviction drainNode() was using vmiInstance.Status.EvacuationNodeName to look up the node in the guest cluster, but that field contains the host/infra node name. This caused the drain to silently fail with a NotFound error, meaning workloads were abruptly killed instead of being gracefully evicted. Use KubevirtMachine.Name instead, which is the guest node name, consistent with the rest of the codebase (e.g. updateNodeProviderID, GenerateProviderID). Update unit tests to use distinct values for the host node name and guest node name, so the bug cannot be masked again. Co-Authored-By: Claude Opus 4.6 --- pkg/kubevirt/machine.go | 2 +- pkg/kubevirt/machine_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/kubevirt/machine.go b/pkg/kubevirt/machine.go index e42619263..7067a2e17 100644 --- a/pkg/kubevirt/machine.go +++ b/pkg/kubevirt/machine.go @@ -546,7 +546,7 @@ func (m *Machine) drainNode(wrkldClstr workloadcluster.WorkloadCluster) (time.Du return 0, fmt.Errorf("failed to get client to remote cluster; %w", err) } - nodeName := m.vmiInstance.Status.EvacuationNodeName + nodeName := m.machineContext.KubevirtMachine.Name node, err := kubeClient.CoreV1().Nodes().Get(m.machineContext, nodeName, metav1.GetOptions{}) if err != nil { if apierrors.IsNotFound(err) { diff --git a/pkg/kubevirt/machine_test.go b/pkg/kubevirt/machine_test.go index 7b766a64d..f3e1e3861 100644 --- a/pkg/kubevirt/machine_test.go +++ b/pkg/kubevirt/machine_test.go @@ -358,7 +358,7 @@ var _ = Describe("With KubeVirt VM running", func() { }) Context("test DrainNodeIfNeeded", func() { - const nodeName = "control-plane1" + const hostNodeName = "host-node-1" var ( wlCluster *mock.MockWorkloadCluster @@ -368,7 +368,7 @@ var _ = Describe("With KubeVirt VM running", func() { virtualMachineInstance = testing.NewVirtualMachineInstance(kubevirtMachine) strategy := kubevirtv1.EvictionStrategyExternal virtualMachineInstance.Spec.EvictionStrategy = &strategy - virtualMachineInstance.Status.EvacuationNodeName = nodeName + virtualMachineInstance.Status.EvacuationNodeName = hostNodeName if kubevirtMachine.Annotations == nil { kubevirtMachine.Annotations = make(map[string]string) @@ -497,7 +497,7 @@ var _ = Describe("With KubeVirt VM running", func() { It("Should drain the node", func() { node := &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ - Name: nodeName, + Name: kubevirtMachineName, }, } @@ -535,7 +535,7 @@ var _ = Describe("With KubeVirt VM running", func() { It("Should not drain the node", func() { node := &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ - Name: nodeName, + Name: kubevirtMachineName, }, } From 6162f6506e5e55d9de53e13748d2534b4732e3a0 Mon Sep 17 00:00:00 2001 From: Nahshon Unna Tsameret Date: Wed, 25 Feb 2026 10:03:45 +0200 Subject: [PATCH 4/5] e2e: verify guest node is cordoned during eviction test The eviction e2e test only validated that the VMI was deleted and recreated, but did not check that the guest node was actually drained. Add an assertion that the guest node is marked unschedulable (cordoned) before the VMI finalizer is removed. Co-Authored-By: Claude Opus 4.6 --- e2e/create-cluster_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/e2e/create-cluster_test.go b/e2e/create-cluster_test.go index ca88a04cd..f1d7522db 100644 --- a/e2e/create-cluster_test.go +++ b/e2e/create-cluster_test.go @@ -795,6 +795,9 @@ var _ = Describe("CreateCluster", func() { By("wait for a VMI to be marked for deletion") waitForVMIDraining(ctx, k8sclient, vmiName, namespace) + By("Verify the guest node was cordoned (drained) before VMI deletion") + waitForNodeCordoned(ctx, clientSet, vmiName) + By("remove the test finalizer") removeFinalizerFromVMI(ctx, k8sclient, recreatedVMI) @@ -877,6 +880,18 @@ func waitForVMIDraining(ctx context.Context, k8sclient client.Client, vmiName, n Should(Succeed()) } +func waitForNodeCordoned(ctx context.Context, cl *kubernetes.Clientset, nodeName string) { + By("wait for guest node to be cordoned") + Eventually(func(g Gomega) { + node, err := cl.CoreV1().Nodes().Get(ctx, nodeName, metav1.GetOptions{}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(node.Spec.Unschedulable).To(BeTrueBecause("expected guest node %q to be cordoned (unschedulable)", nodeName)) + }).WithOffset(1). + WithTimeout(time.Minute * 5). + WithPolling(time.Second * 5). + Should(Succeed()) +} + func evictNode(ctx context.Context, cli *kubernetes.Clientset, pod *corev1.Pod) { err := cli.CoreV1().Pods(pod.Namespace).EvictV1beta1(ctx, &policy.Eviction{ ObjectMeta: metav1.ObjectMeta{ From 9a5c58b94cf20eac71cc1cd11342c7d0d3a56e31 Mon Sep 17 00:00:00 2001 From: Nahshon Unna Tsameret Date: Wed, 25 Feb 2026 12:29:55 +0200 Subject: [PATCH 5/5] don't hold the PR if Coverall is down, ad fails the coverage github action Signed-off-by: Nahshon Unna Tsameret --- .github/workflows/test.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 9200f255c..44a98b8b8 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -36,6 +36,7 @@ jobs: mkdir -p coverprofiles make test - name: Push to coveralls.io + continue-on-error: true env: COVERALLS_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: |-