Skip to content

Commit

Permalink
refactor: client.go file helper methods
Browse files Browse the repository at this point in the history
Refactored the existing methods and functions to be unit testable.
Also made some methods that do not use the struct as generic functions.
Added unit test coverage.
  • Loading branch information
dkoshkin committed Jan 10, 2024
1 parent 629d802 commit 12a8c67
Show file tree
Hide file tree
Showing 6 changed files with 900 additions and 144 deletions.
31 changes: 24 additions & 7 deletions api/v1beta1/nutanixcluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package v1beta1

import (
"fmt"

credentialTypes "github.com/nutanix-cloud-native/prism-go-client/environment/credentials"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
capiv1 "sigs.k8s.io/cluster-api/api/v1beta1"
Expand Down Expand Up @@ -86,12 +88,12 @@ type NutanixClusterStatus struct {
FailureMessage *string `json:"failureMessage,omitempty"`
}

//+kubebuilder:object:root=true
//+kubebuilder:resource:path=nutanixclusters,shortName=ncl,scope=Namespaced,categories=cluster-api
//+kubebuilder:subresource:status
//+kubebuilder:storageversion
//+kubebuilder:printcolumn:name="ControlplaneEndpoint",type="string",JSONPath=".spec.controlPlaneEndpoint.host",description="ControlplaneEndpoint"
//+kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.ready",description="in ready status"
// +kubebuilder:object:root=true
// +kubebuilder:resource:path=nutanixclusters,shortName=ncl,scope=Namespaced,categories=cluster-api
// +kubebuilder:subresource:status
// +kubebuilder:storageversion
// +kubebuilder:printcolumn:name="ControlplaneEndpoint",type="string",JSONPath=".spec.controlPlaneEndpoint.host",description="ControlplaneEndpoint"
// +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.ready",description="in ready status"

// NutanixCluster is the Schema for the nutanixclusters API
type NutanixCluster struct {
Expand Down Expand Up @@ -145,7 +147,22 @@ func (ncl *NutanixCluster) SetConditions(conditions capiv1.Conditions) {
ncl.Status.Conditions = conditions
}

//+kubebuilder:object:root=true
func (ncl *NutanixCluster) GetCredentialRefForCluster() (*credentialTypes.NutanixCredentialReference, error) {
prismCentralInfo := ncl.Spec.PrismCentral
if prismCentralInfo == nil {
return nil, nil
}
if prismCentralInfo.CredentialRef == nil {
return nil, fmt.Errorf("credentialRef must be set on prismCentral attribute for cluster %s in namespace %s", ncl.Name, ncl.Namespace)
}
if prismCentralInfo.CredentialRef.Kind != credentialTypes.SecretKind {
return nil, nil

Check warning on line 159 in api/v1beta1/nutanixcluster_types.go

View check run for this annotation

Codecov / codecov/patch

api/v1beta1/nutanixcluster_types.go#L159

Added line #L159 was not covered by tests
}

return prismCentralInfo.CredentialRef, nil
}

// +kubebuilder:object:root=true

// NutanixClusterList contains a list of NutanixCluster
type NutanixClusterList struct {
Expand Down
15 changes: 6 additions & 9 deletions controllers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,14 @@ import (
"strings"

"github.com/google/uuid"
infrav1 "github.com/nutanix-cloud-native/cluster-api-provider-nutanix/api/v1beta1"
nutanixClientHelper "github.com/nutanix-cloud-native/cluster-api-provider-nutanix/pkg/client"
"github.com/nutanix-cloud-native/prism-go-client/utils"
nutanixClientV3 "github.com/nutanix-cloud-native/prism-go-client/v3"
"k8s.io/apimachinery/pkg/api/resource"
coreinformers "k8s.io/client-go/informers/core/v1"
ctrl "sigs.k8s.io/controller-runtime"

infrav1 "github.com/nutanix-cloud-native/cluster-api-provider-nutanix/api/v1beta1"
nutanixClient "github.com/nutanix-cloud-native/cluster-api-provider-nutanix/pkg/client"
)

const (
Expand All @@ -47,12 +48,8 @@ const (
func CreateNutanixClient(ctx context.Context, secretInformer coreinformers.SecretInformer, cmInformer coreinformers.ConfigMapInformer, nutanixCluster *infrav1.NutanixCluster) (*nutanixClientV3.Client, error) {
log := ctrl.LoggerFrom(ctx)
log.V(1).Info("creating nutanix client")
helper, err := nutanixClientHelper.NewNutanixClientHelper(secretInformer, cmInformer)
if err != nil {
log.Error(err, "error creating nutanix client helper")
return nil, err
}
return helper.GetClientFromEnvironment(ctx, nutanixCluster)
helper := nutanixClient.NewHelper(secretInformer, cmInformer)
return helper.BuildClientForNutanixClusterWithFallback(ctx, nutanixCluster)

Check warning on line 52 in controllers/helpers.go

View check run for this annotation

Codecov / codecov/patch

controllers/helpers.go#L51-L52

Added lines #L51 - L52 were not covered by tests
}

// DeleteVM deletes a VM and is invoked by the NutanixMachineReconciler
Expand Down Expand Up @@ -346,7 +343,7 @@ func GetImageUUID(ctx context.Context, client *nutanixClientV3.Client, imageName
// HasTaskInProgress returns true if the given task is in progress
func HasTaskInProgress(ctx context.Context, client *nutanixClientV3.Client, taskUUID string) (bool, error) {
log := ctrl.LoggerFrom(ctx)
taskStatus, err := nutanixClientHelper.GetTaskState(ctx, client, taskUUID)
taskStatus, err := nutanixClient.GetTaskState(ctx, client, taskUUID)

Check warning on line 346 in controllers/helpers.go

View check run for this annotation

Codecov / codecov/patch

controllers/helpers.go#L346

Added line #L346 was not covered by tests
if err != nil {
return false, err
}
Expand Down
23 changes: 15 additions & 8 deletions controllers/nutanixcluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"time"

credentialTypes "github.com/nutanix-cloud-native/prism-go-client/environment/credentials"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -43,7 +44,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/source"

infrav1 "github.com/nutanix-cloud-native/cluster-api-provider-nutanix/api/v1beta1"
nutanixClient "github.com/nutanix-cloud-native/cluster-api-provider-nutanix/pkg/client"
nctx "github.com/nutanix-cloud-native/cluster-api-provider-nutanix/pkg/context"
)

Expand Down Expand Up @@ -101,11 +101,11 @@ func (r *NutanixClusterReconciler) SetupWithManager(ctx context.Context, mgr ctr
return nil
}

//+kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;watch;update;delete
//+kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters;clusters/status,verbs=get;list;watch
//+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=nutanixclusters,verbs=get;list;watch;create;update;patch;delete
//+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=nutanixclusters/status,verbs=get;update;patch
//+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=nutanixclusters/finalizers,verbs=update
// +kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;watch;update;delete
// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters;clusters/status,verbs=get;list;watch
// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=nutanixclusters,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=nutanixclusters/status,verbs=get;update;patch
// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=nutanixclusters/finalizers,verbs=update

// Reconcile is part of the main kubernetes reconciliation loop which aims to
// move the current state of the cluster closer to the desired state.
Expand Down Expand Up @@ -331,7 +331,7 @@ func (r *NutanixClusterReconciler) reconcileCategoriesDelete(rctx *nctx.ClusterC

func (r *NutanixClusterReconciler) reconcileCredentialRefDelete(ctx context.Context, nutanixCluster *infrav1.NutanixCluster) error {
log := ctrl.LoggerFrom(ctx)
credentialRef, err := nutanixClient.GetCredentialRefForCluster(nutanixCluster)
credentialRef, err := mustGetCredentialRefForCluster(nutanixCluster)
if err != nil {
log.Error(err, fmt.Sprintf("error occurred while getting credential ref for cluster %s", nutanixCluster.Name))
return err
Expand Down Expand Up @@ -372,7 +372,7 @@ func (r *NutanixClusterReconciler) reconcileCredentialRefDelete(ctx context.Cont

func (r *NutanixClusterReconciler) reconcileCredentialRef(ctx context.Context, nutanixCluster *infrav1.NutanixCluster) error {
log := ctrl.LoggerFrom(ctx)
credentialRef, err := nutanixClient.GetCredentialRefForCluster(nutanixCluster)
credentialRef, err := mustGetCredentialRefForCluster(nutanixCluster)
if err != nil {
return err
}
Expand Down Expand Up @@ -418,3 +418,10 @@ func (r *NutanixClusterReconciler) reconcileCredentialRef(ctx context.Context, n
}
return nil
}

func mustGetCredentialRefForCluster(nutanixCluster *infrav1.NutanixCluster) (*credentialTypes.NutanixCredentialReference, error) {
if nutanixCluster == nil {
return nil, fmt.Errorf("cannot get credential reference if nutanix cluster object is nil")
}
return nutanixCluster.GetCredentialRefForCluster()
}
Loading

0 comments on commit 12a8c67

Please sign in to comment.