From 85b6a78ac02b07531f317543b5ccd9a78b5f17e6 Mon Sep 17 00:00:00 2001 From: Deepak Muley Date: Wed, 1 May 2024 15:04:18 -0700 Subject: [PATCH] removed optional annotation from non-optional fields same code as https://github.com/nutanix-cloud-native/cluster-api-provider-nutanix/pull/400/files which was reverted in https://github.com/nutanix-cloud-native/cluster-api-provider-nutanix/pull/414 --- Makefile | 8 +- api/v1beta1/nutanixcluster_types.go | 10 +-- api/v1beta1/nutanixcluster_types_test.go | 22 ++++- api/v1beta1/nutanixmachine_types.go | 2 + ...ture.cluster.x-k8s.io_nutanixclusters.yaml | 3 + ...ster.x-k8s.io_nutanixclustertemplates.yaml | 3 + controllers/helpers_test.go | 24 ++--- controllers/nutanixcluster_controller.go | 7 +- controllers/nutanixcluster_controller_test.go | 87 +++++++++---------- pkg/client/client.go | 2 +- pkg/client/client_test.go | 5 +- templates/cluster-template-clusterclass.yaml | 10 +++ templates/clusterclass/nct.yaml | 10 +++ test/e2e/common.go | 14 +-- test/e2e/nutanix_client_test.go | 55 ++++-------- test/e2e/test_helpers.go | 7 +- 16 files changed, 150 insertions(+), 119 deletions(-) diff --git a/Makefile b/Makefile index 6d35913141..7f39312f4a 100644 --- a/Makefile +++ b/Makefile @@ -335,7 +335,7 @@ test-e2e: docker-build-e2e cluster-e2e-templates cluster-templates ## Run the en mkdir -p $(ARTIFACTS) NUTANIX_LOG_LEVEL=debug ginkgo -v \ --trace \ - --progress \ + --show-node-events \ --tags=e2e \ --label-filter=$(LABEL_FILTER_ARGS) \ $(_SKIP_ARGS) \ @@ -345,7 +345,7 @@ test-e2e: docker-build-e2e cluster-e2e-templates cluster-templates ## Run the en --output-dir="$(ARTIFACTS)" \ --junit-report=${JUNIT_REPORT_FILE} \ --timeout="24h" \ - --always-emit-ginkgo-writer \ + -v \ $(GINKGO_ARGS) ./test/e2e -- \ -e2e.artifacts-folder="$(ARTIFACTS)" \ -e2e.config="$(E2E_CONF_FILE)" \ @@ -357,7 +357,7 @@ test-e2e-no-kubeproxy: docker-build-e2e cluster-e2e-templates-no-kubeproxy clust mkdir -p $(ARTIFACTS) NUTANIX_LOG_LEVEL=debug ginkgo -v \ --trace \ - --progress \ + --show-node-events \ --tags=e2e \ --label-filter=$(LABEL_FILTER_ARGS) \ $(_SKIP_ARGS) \ @@ -366,7 +366,7 @@ test-e2e-no-kubeproxy: docker-build-e2e cluster-e2e-templates-no-kubeproxy clust --output-dir="$(ARTIFACTS)" \ --junit-report=${JUNIT_REPORT_FILE} \ --timeout="24h" \ - --always-emit-ginkgo-writer \ + -v \ $(GINKGO_ARGS) ./test/e2e -- \ -e2e.artifacts-folder="$(ARTIFACTS)" \ -e2e.config="$(E2E_CONF_FILE)" \ diff --git a/api/v1beta1/nutanixcluster_types.go b/api/v1beta1/nutanixcluster_types.go index d3646998b2..c7f6f6ee73 100644 --- a/api/v1beta1/nutanixcluster_types.go +++ b/api/v1beta1/nutanixcluster_types.go @@ -48,14 +48,12 @@ type NutanixClusterSpec struct { // ControlPlaneEndpoint represents the endpoint used to communicate with the control plane. // host can be either DNS name or ip address - // +optional ControlPlaneEndpoint capiv1.APIEndpoint `json:"controlPlaneEndpoint"` // prismCentral holds the endpoint address and port to access the Nutanix Prism Central. // When a cluster-wide proxy is installed, by default, this endpoint will be accessed via the proxy. // Should you wish for communication with this endpoint not to be proxied, please add the endpoint to the // proxy spec.noProxy list. - // +optional PrismCentral *credentialTypes.NutanixPrismEndpoint `json:"prismCentral"` // failureDomains configures failure domains information for the Nutanix platform. @@ -64,7 +62,7 @@ type NutanixClusterSpec struct { // +listType=map // +listMapKey=name // +optional - FailureDomains []NutanixFailureDomain `json:"failureDomains"` + FailureDomains []NutanixFailureDomain `json:"failureDomains,omitempty"` } // NutanixClusterStatus defines the observed state of NutanixCluster @@ -152,13 +150,13 @@ func (ncl *NutanixCluster) SetConditions(conditions capiv1.Conditions) { func (ncl *NutanixCluster) GetPrismCentralCredentialRef() (*credentialTypes.NutanixCredentialReference, error) { prismCentralInfo := ncl.Spec.PrismCentral if prismCentralInfo == nil { - return nil, nil + return nil, fmt.Errorf("prismCentral info is not provided.") } 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) + return nil, nil // returning nil so that we can use default controller's secret } if prismCentralInfo.CredentialRef.Kind != credentialTypes.SecretKind { - return nil, nil + return nil, fmt.Errorf("credentialRef should be of kind Secret") } return prismCentralInfo.CredentialRef, nil diff --git a/api/v1beta1/nutanixcluster_types_test.go b/api/v1beta1/nutanixcluster_types_test.go index 880f0be35b..0fd1958636 100644 --- a/api/v1beta1/nutanixcluster_types_test.go +++ b/api/v1beta1/nutanixcluster_types_test.go @@ -23,6 +23,7 @@ import ( "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + capiv1 "sigs.k8s.io/cluster-api/api/v1beta1" "github.com/nutanix-cloud-native/prism-go-client/environment/credentials" ) @@ -43,6 +44,10 @@ func TestGetCredentialRefForCluster(t *testing.T) { Namespace: corev1.NamespaceDefault, }, Spec: NutanixClusterSpec{ + ControlPlaneEndpoint: capiv1.APIEndpoint{ + Host: "host", + Port: 6443, + }, PrismCentral: &credentials.NutanixPrismEndpoint{ Address: "address", Port: 9440, @@ -61,7 +66,7 @@ func TestGetCredentialRefForCluster(t *testing.T) { }, }, { - name: "prismCentralInfo is nil, should not fail", + name: "prismCentralInfo is nil, should fail", nutanixCluster: &NutanixCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test", @@ -69,15 +74,20 @@ func TestGetCredentialRefForCluster(t *testing.T) { }, Spec: NutanixClusterSpec{}, }, + expectedErr: fmt.Errorf("prismCentral info is not provided."), }, { - name: "CredentialRef kind is not kind Secret, should not fail", + name: "CredentialRef kind is not kind Secret, should fail", nutanixCluster: &NutanixCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test", Namespace: corev1.NamespaceDefault, }, Spec: NutanixClusterSpec{ + ControlPlaneEndpoint: capiv1.APIEndpoint{ + Host: "host", + Port: 6443, + }, PrismCentral: &credentials.NutanixPrismEndpoint{ CredentialRef: &credentials.NutanixCredentialReference{ Kind: "unknown", @@ -85,21 +95,25 @@ func TestGetCredentialRefForCluster(t *testing.T) { }, }, }, + expectedErr: fmt.Errorf("credentialRef should be of kind Secret"), }, { - name: "prismCentralInfo is not nil but CredentialRef is nil, should fail", + name: "prismCentralInfo is not nil but CredentialRef is nil, should not fail", nutanixCluster: &NutanixCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test", Namespace: corev1.NamespaceDefault, }, Spec: NutanixClusterSpec{ + ControlPlaneEndpoint: capiv1.APIEndpoint{ + Host: "host", + Port: 6443, + }, PrismCentral: &credentials.NutanixPrismEndpoint{ Address: "address", }, }, }, - expectedErr: fmt.Errorf("credentialRef must be set on prismCentral attribute for cluster test in namespace default"), }, } for _, tt := range tests { diff --git a/api/v1beta1/nutanixmachine_types.go b/api/v1beta1/nutanixmachine_types.go index 97a5c7cd5a..2b8675132d 100644 --- a/api/v1beta1/nutanixmachine_types.go +++ b/api/v1beta1/nutanixmachine_types.go @@ -83,6 +83,7 @@ type NutanixMachineSpec struct { Subnets []NutanixResourceIdentifier `json:"subnet"` // List of categories that need to be added to the machines. Categories must already exist in Prism Central // +kubebuilder:validation:Optional + // +optional AdditionalCategories []NutanixCategoryIdentifier `json:"additionalCategories,omitempty"` // Add the machine resources to a Prism Central project // +optional @@ -104,6 +105,7 @@ type NutanixMachineSpec struct { // List of GPU devices that need to be added to the machines. // +kubebuilder:validation:Optional + // +optional GPUs []NutanixGPU `json:"gpus,omitempty"` } diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_nutanixclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_nutanixclusters.yaml index 67f0bb6b0f..c161dbd26c 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_nutanixclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_nutanixclusters.yaml @@ -512,6 +512,9 @@ spec: - address - port type: object + required: + - controlPlaneEndpoint + - prismCentral type: object status: description: NutanixClusterStatus defines the observed state of NutanixCluster diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_nutanixclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_nutanixclustertemplates.yaml index 8a55c5a1d0..dc3937706e 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_nutanixclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_nutanixclustertemplates.yaml @@ -228,6 +228,9 @@ spec: - address - port type: object + required: + - controlPlaneEndpoint + - prismCentral type: object required: - spec diff --git a/controllers/helpers_test.go b/controllers/helpers_test.go index 7b5f0027c5..2ab74c9391 100644 --- a/controllers/helpers_test.go +++ b/controllers/helpers_test.go @@ -23,7 +23,7 @@ import ( "testing" "github.com/golang/mock/gomock" - credentialtypes "github.com/nutanix-cloud-native/prism-go-client/environment/credentials" + credentialTypes "github.com/nutanix-cloud-native/prism-go-client/environment/credentials" prismclientv3 "github.com/nutanix-cloud-native/prism-go-client/v3" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -31,6 +31,7 @@ import ( "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + capiv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/util" infrav1 "github.com/nutanix-cloud-native/cluster-api-provider-nutanix/api/v1beta1" @@ -60,7 +61,8 @@ func TestControllerHelpers(t *testing.T) { Namespace: "default", }, Spec: infrav1.NutanixClusterSpec{ - PrismCentral: &credentialtypes.NutanixPrismEndpoint{ + ControlPlaneEndpoint: capiv1.APIEndpoint{}, + PrismCentral: &credentialTypes.NutanixPrismEndpoint{ // Adding port info to override default value (0) Port: 9440, }, @@ -134,11 +136,11 @@ func TestGetPrismCentralClientForCluster(t *testing.T) { ctx := context.Background() cluster := &infrav1.NutanixCluster{ Spec: infrav1.NutanixClusterSpec{ - PrismCentral: &credentialtypes.NutanixPrismEndpoint{ + PrismCentral: &credentialTypes.NutanixPrismEndpoint{ Address: "prismcentral.nutanix.com", Port: 9440, - CredentialRef: &credentialtypes.NutanixCredentialReference{ - Kind: credentialtypes.SecretKind, + CredentialRef: &credentialTypes.NutanixCredentialReference{ + Kind: credentialTypes.SecretKind, Name: "test-credential", Namespace: "test-ns", }, @@ -164,9 +166,9 @@ func TestGetPrismCentralClientForCluster(t *testing.T) { t.Run("GetOrCreate Fails", func(t *testing.T) { ctrl := gomock.NewController(t) - creds := []credentialtypes.Credential{ + creds := []credentialTypes.Credential{ { - Type: credentialtypes.BasicAuthCredentialType, + Type: credentialTypes.BasicAuthCredentialType, Data: []byte(`{"prismCentral":{"username":"user","password":"password"}}`), }, } @@ -175,7 +177,7 @@ func TestGetPrismCentralClientForCluster(t *testing.T) { secret := &corev1.Secret{ Data: map[string][]byte{ - credentialtypes.KeyName: credsMarshal, + credentialTypes.KeyName: credsMarshal, }, } @@ -202,9 +204,9 @@ func TestGetPrismCentralClientForCluster(t *testing.T) { // Create a new client cache with session auth disabled to avoid network calls in tests nutanixclient.NutanixClientCache = prismclientv3.NewClientCache() - creds := []credentialtypes.Credential{ + creds := []credentialTypes.Credential{ { - Type: credentialtypes.BasicAuthCredentialType, + Type: credentialTypes.BasicAuthCredentialType, Data: []byte(`{"prismCentral":{"username":"user","password":"password"}}`), }, } @@ -213,7 +215,7 @@ func TestGetPrismCentralClientForCluster(t *testing.T) { require.NoError(t, err) secret := &corev1.Secret{ Data: map[string][]byte{ - credentialtypes.KeyName: credsMarshal, + credentialTypes.KeyName: credsMarshal, }, } diff --git a/controllers/nutanixcluster_controller.go b/controllers/nutanixcluster_controller.go index d435801223..faabb8f3e6 100644 --- a/controllers/nutanixcluster_controller.go +++ b/controllers/nutanixcluster_controller.go @@ -336,11 +336,12 @@ func (r *NutanixClusterReconciler) reconcileCredentialRefDelete(ctx context.Cont log := ctrl.LoggerFrom(ctx) credentialRef, err := getPrismCentralCredentialRefForCluster(nutanixCluster) if err != nil { - log.Error(err, fmt.Sprintf("error occurred while getting credential ref for cluster %s", nutanixCluster.Name)) + // do not use nutanixCluster in error case as it might be nil + log.Error(err, "Failed to get PC Creds") return err } if credentialRef == nil { - log.V(1).Info(fmt.Sprintf("Credential ref is nil for cluster %s. Ignoring since object must be deleted", nutanixCluster.Name)) + log.V(1).Info(fmt.Sprintf("Credential ref is nil for cluster %s. Ignoring since object must be deleted or was not overriden", nutanixCluster.Name)) return nil } log.V(1).Info(fmt.Sprintf("Credential ref is kind Secret for cluster %s. Continue with deletion of secret", nutanixCluster.Name)) @@ -377,6 +378,8 @@ func (r *NutanixClusterReconciler) reconcileCredentialRef(ctx context.Context, n log := ctrl.LoggerFrom(ctx) credentialRef, err := getPrismCentralCredentialRefForCluster(nutanixCluster) if err != nil { + // do not use nutanixCluster in error case as it might be nil + log.Error(err, "Failed to get PC Creds") return err } diff --git a/controllers/nutanixcluster_controller_test.go b/controllers/nutanixcluster_controller_test.go index 9df61fa28f..5bbd11b12e 100644 --- a/controllers/nutanixcluster_controller_test.go +++ b/controllers/nutanixcluster_controller_test.go @@ -22,7 +22,7 @@ import ( "testing" "github.com/golang/mock/gomock" - credentialtypes "github.com/nutanix-cloud-native/prism-go-client/environment/credentials" + credentialTypes "github.com/nutanix-cloud-native/prism-go-client/environment/credentials" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/onsi/gomega/gstruct" @@ -86,7 +86,8 @@ func TestNutanixClusterReconciler(t *testing.T) { UID: utilruntime.NewUUID(), }, Spec: infrav1.NutanixClusterSpec{ - PrismCentral: &credentialtypes.NutanixPrismEndpoint{ + ControlPlaneEndpoint: capiv1.APIEndpoint{}, + PrismCentral: &credentialTypes.NutanixPrismEndpoint{ // Adding port info to override default value (0) Port: 9440, }, @@ -248,7 +249,8 @@ func TestNutanixClusterReconciler(t *testing.T) { Namespace: corev1.NamespaceDefault, }, Spec: infrav1.NutanixClusterSpec{ - PrismCentral: &credentialtypes.NutanixPrismEndpoint{ + ControlPlaneEndpoint: capiv1.APIEndpoint{}, + PrismCentral: &credentialTypes.NutanixPrismEndpoint{ // Adding port info to override default value (0) Port: 9440, }, @@ -257,8 +259,8 @@ func TestNutanixClusterReconciler(t *testing.T) { g.Expect(k8sClient.Create(ctx, additionalNtnxCluster)).To(Succeed()) // Add credential ref to the ntnxCluster resource - ntnxCluster.Spec.PrismCentral.CredentialRef = &credentialtypes.NutanixCredentialReference{ - Kind: credentialtypes.SecretKind, + ntnxCluster.Spec.PrismCentral.CredentialRef = &credentialTypes.NutanixCredentialReference{ + Kind: credentialTypes.SecretKind, Name: ntnxSecret.Name, Namespace: ntnxSecret.Namespace, } @@ -280,8 +282,8 @@ func TestNutanixClusterReconciler(t *testing.T) { }) It("should add credentialRef and finalizer if not owned by other cluster", func() { // Add credential ref to the ntnxCluster resource - ntnxCluster.Spec.PrismCentral.CredentialRef = &credentialtypes.NutanixCredentialReference{ - Kind: credentialtypes.SecretKind, + ntnxCluster.Spec.PrismCentral.CredentialRef = &credentialTypes.NutanixCredentialReference{ + Kind: credentialTypes.SecretKind, Name: ntnxSecret.Name, Namespace: ntnxSecret.Namespace, } @@ -307,8 +309,8 @@ func TestNutanixClusterReconciler(t *testing.T) { }) It("does not add another credentialRef if it is already set", func() { // Add credential ref to the ntnxCluster resource - ntnxCluster.Spec.PrismCentral.CredentialRef = &credentialtypes.NutanixCredentialReference{ - Kind: credentialtypes.SecretKind, + ntnxCluster.Spec.PrismCentral.CredentialRef = &credentialTypes.NutanixCredentialReference{ + Kind: credentialTypes.SecretKind, Name: ntnxSecret.Name, Namespace: ntnxSecret.Namespace, } @@ -344,8 +346,8 @@ func TestNutanixClusterReconciler(t *testing.T) { It("allows multiple ownerReferences with different kinds", func() { // Add credential ref to the ntnxCluster resource - ntnxCluster.Spec.PrismCentral.CredentialRef = &credentialtypes.NutanixCredentialReference{ - Kind: credentialtypes.SecretKind, + ntnxCluster.Spec.PrismCentral.CredentialRef = &credentialTypes.NutanixCredentialReference{ + Kind: credentialTypes.SecretKind, Name: ntnxSecret.Name, Namespace: ntnxSecret.Namespace, } @@ -379,8 +381,8 @@ func TestNutanixClusterReconciler(t *testing.T) { }) It("should error if secret does not exist", func() { // Add credential ref to the ntnxCluster resource - ntnxCluster.Spec.PrismCentral.CredentialRef = &credentialtypes.NutanixCredentialReference{ - Kind: credentialtypes.SecretKind, + ntnxCluster.Spec.PrismCentral.CredentialRef = &credentialTypes.NutanixCredentialReference{ + Kind: credentialTypes.SecretKind, Name: ntnxSecret.Name, Namespace: ntnxSecret.Namespace, } @@ -412,10 +414,11 @@ func TestNutanixClusterReconciler(t *testing.T) { Namespace: "default", }, Spec: infrav1.NutanixClusterSpec{ - PrismCentral: &credentialtypes.NutanixPrismEndpoint{ + ControlPlaneEndpoint: capiv1.APIEndpoint{}, + PrismCentral: &credentialTypes.NutanixPrismEndpoint{ // Adding port info to override default value (0) Port: 9440, - CredentialRef: &credentialtypes.NutanixCredentialReference{ + CredentialRef: &credentialTypes.NutanixCredentialReference{ Name: "test", Namespace: "default", Kind: "Secret", @@ -460,8 +463,8 @@ func TestNutanixClusterReconciler(t *testing.T) { }) }) - Context("Delete credentials ref reconcile failed: no credential ref", func() { - It("Should return error", func() { + Context("Delete credentials ref reconcile did not fail: no credential ref overriden", func() { + It("Should not return error", func() { ctx := context.Background() reconciler := &NutanixClusterReconciler{ Client: k8sClient, @@ -474,7 +477,8 @@ func TestNutanixClusterReconciler(t *testing.T) { Namespace: "default", }, Spec: infrav1.NutanixClusterSpec{ - PrismCentral: &credentialtypes.NutanixPrismEndpoint{ + ControlPlaneEndpoint: capiv1.APIEndpoint{}, + PrismCentral: &credentialTypes.NutanixPrismEndpoint{ // Adding port info to override default value (0) Port: 9440, }, @@ -490,7 +494,7 @@ func TestNutanixClusterReconciler(t *testing.T) { // Reconile Delete credential ref err := reconciler.reconcileCredentialRefDelete(ctx, ntnxCluster) - g.Expect(err).To(HaveOccurred()) + g.Expect(err).NotTo(HaveOccurred()) }) }) @@ -508,10 +512,11 @@ func TestNutanixClusterReconciler(t *testing.T) { Namespace: "default", }, Spec: infrav1.NutanixClusterSpec{ - PrismCentral: &credentialtypes.NutanixPrismEndpoint{ + ControlPlaneEndpoint: capiv1.APIEndpoint{}, + PrismCentral: &credentialTypes.NutanixPrismEndpoint{ // Adding port info to override default value (0) Port: 9440, - CredentialRef: &credentialtypes.NutanixCredentialReference{ + CredentialRef: &credentialTypes.NutanixCredentialReference{ Name: "test", Namespace: "default", Kind: "Secret", @@ -533,13 +538,9 @@ func TestNutanixClusterReconciler(t *testing.T) { }) }) - Context("Delete credentials ref reconcile failed: PrismCentral Info is null", func() { + Context("NutanixCluster creation failed: PrismCentral Info is null", func() { It("Should not return error", func() { ctx := context.Background() - reconciler := &NutanixClusterReconciler{ - Client: k8sClient, - Scheme: runtime.NewScheme(), - } ntnxCluster := &infrav1.NutanixCluster{ ObjectMeta: metav1.ObjectMeta{ @@ -547,20 +548,13 @@ func TestNutanixClusterReconciler(t *testing.T) { Namespace: "default", }, Spec: infrav1.NutanixClusterSpec{ - PrismCentral: nil, + ControlPlaneEndpoint: capiv1.APIEndpoint{}, + PrismCentral: nil, }, } // Create the NutanixCluster object - g.Expect(k8sClient.Create(ctx, ntnxCluster)).To(Succeed()) - defer func() { - err := k8sClient.Delete(ctx, ntnxCluster) - Expect(err).NotTo(HaveOccurred()) - }() - - // Reconile Delete credential ref - err := reconciler.reconcileCredentialRefDelete(ctx, ntnxCluster) - g.Expect(err).NotTo(HaveOccurred()) + g.Expect(k8sClient.Create(ctx, ntnxCluster)).NotTo(Succeed()) }) }) }) @@ -571,7 +565,10 @@ func TestReconcileCredentialRefWithPrismCentralNotSetOnCluster(t *testing.T) { ctx := context.Background() fakeClient := mockctlclient.NewMockClient(mockCtrl) nutanixCluster := &infrav1.NutanixCluster{ - Spec: infrav1.NutanixClusterSpec{}, + Spec: infrav1.NutanixClusterSpec{ + ControlPlaneEndpoint: capiv1.APIEndpoint{}, + PrismCentral: nil, + }, } reconciler := &NutanixClusterReconciler{ @@ -579,7 +576,7 @@ func TestReconcileCredentialRefWithPrismCentralNotSetOnCluster(t *testing.T) { } err := reconciler.reconcileCredentialRef(ctx, nutanixCluster) - assert.NoError(t, err) + assert.Error(t, err) } func TestReconcileCredentialRefWithValidCredentialRef(t *testing.T) { @@ -587,9 +584,10 @@ func TestReconcileCredentialRefWithValidCredentialRef(t *testing.T) { nutanixCluster := &infrav1.NutanixCluster{ Spec: infrav1.NutanixClusterSpec{ - PrismCentral: &credentialtypes.NutanixPrismEndpoint{ - CredentialRef: &credentialtypes.NutanixCredentialReference{ - Kind: credentialtypes.SecretKind, + ControlPlaneEndpoint: capiv1.APIEndpoint{}, + PrismCentral: &credentialTypes.NutanixPrismEndpoint{ + CredentialRef: &credentialTypes.NutanixCredentialReference{ + Kind: credentialTypes.SecretKind, Name: "test-credential", Namespace: "test-ns", }, @@ -625,9 +623,10 @@ func TestReconcileCredentialRefWithValidCredentialRefFailedUpdate(t *testing.T) fakeClient := mockctlclient.NewMockClient(mockCtrl) nutanixCluster := &infrav1.NutanixCluster{ Spec: infrav1.NutanixClusterSpec{ - PrismCentral: &credentialtypes.NutanixPrismEndpoint{ - CredentialRef: &credentialtypes.NutanixCredentialReference{ - Kind: credentialtypes.SecretKind, + ControlPlaneEndpoint: capiv1.APIEndpoint{}, + PrismCentral: &credentialTypes.NutanixPrismEndpoint{ + CredentialRef: &credentialTypes.NutanixCredentialReference{ + Kind: credentialTypes.SecretKind, Name: "test-credential", Namespace: "test-ns", }, diff --git a/pkg/client/client.go b/pkg/client/client.go index 12ff59f7ed..7fe2ce22d7 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -124,7 +124,7 @@ func (n *NutanixClientHelper) buildProviderFromNutanixCluster(nutanixCluster *in return nil, err } // If namespace is empty, use the cluster namespace - if credentialRef.Namespace == "" { + if credentialRef != nil && credentialRef.Namespace == "" { credentialRef.Namespace = nutanixCluster.Namespace } additionalTrustBundleRef := prismCentralInfo.AdditionalTrustBundle diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index f4f1669ee7..5cfd154ce0 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -375,7 +375,7 @@ func Test_buildProviderFromNutanixCluster(t *testing.T) { expectedErr: ErrPrismPortNotSet, }, { - name: "CredentialRef is not set, should fail", + name: "CredentialRef is not set, should not fail assuming not overriden and user wanted to use default from controller", helper: testHelper(), nutanixCluster: &infrav1.NutanixCluster{ ObjectMeta: metav1.ObjectMeta{ @@ -394,8 +394,7 @@ func Test_buildProviderFromNutanixCluster(t *testing.T) { }, }, }, - expectProviderToBeNil: true, - expectedErr: fmt.Errorf("credentialRef must be set on prismCentral attribute for cluster %s in namespace %s", "test", corev1.NamespaceDefault), + expectProviderToBeNil: false, }, } for _, tt := range tests { diff --git a/templates/cluster-template-clusterclass.yaml b/templates/cluster-template-clusterclass.yaml index d5d4909022..1fd99d1ea6 100644 --- a/templates/cluster-template-clusterclass.yaml +++ b/templates/cluster-template-clusterclass.yaml @@ -699,7 +699,17 @@ metadata: spec: template: spec: + controlPlaneEndpoint: + host: PLACEHOLDER + port: 6443 failureDomains: [] + prismCentral: + address: PLACEHOLDER + credentialRef: + kind: Secret + name: PLACEHOLDER + namespace: default + port: 9440 --- apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 kind: NutanixMachineTemplate diff --git a/templates/clusterclass/nct.yaml b/templates/clusterclass/nct.yaml index 350355cde9..794183bf8f 100644 --- a/templates/clusterclass/nct.yaml +++ b/templates/clusterclass/nct.yaml @@ -5,4 +5,14 @@ metadata: spec: template: spec: + controlPlaneEndpoint: + host: PLACEHOLDER + port: 6443 + prismCentral: + address: PLACEHOLDER + port: 9440 + credentialRef: + name: PLACEHOLDER + kind: Secret + namespace: default failureDomains: [] diff --git a/test/e2e/common.go b/test/e2e/common.go index 7aaefd828b..15ff29e3ac 100644 --- a/test/e2e/common.go +++ b/test/e2e/common.go @@ -65,10 +65,12 @@ func setupSpecNamespace(ctx context.Context, specName string, clusterProxy frame } func dumpSpecResourcesAndCleanup(ctx context.Context, specName string, clusterProxy framework.ClusterProxy, artifactFolder string, namespace *corev1.Namespace, cancelWatches context.CancelFunc, cluster *clusterv1.Cluster, intervalsGetter func(spec, key string) []interface{}, skipCleanup bool) { - Byf("Dumping logs from the %q workload cluster", cluster.Name) - - // Dump all the logs from the workload cluster before deleting them. - clusterProxy.CollectWorkloadClusterLogs(ctx, cluster.Namespace, cluster.Name, filepath.Join(artifactFolder, "clusters", cluster.Name)) + // Some tests may not create workload cluster hence the check + if cluster != nil { + Byf("Dumping logs from the %q workload cluster", cluster.Name) + // Dump all the logs from the workload cluster before deleting them. + clusterProxy.CollectWorkloadClusterLogs(ctx, cluster.Namespace, cluster.Name, filepath.Join(artifactFolder, "clusters", cluster.Name)) + } Byf("Dumping all the Cluster API resources in the %q namespace", namespace.Name) @@ -80,7 +82,9 @@ func dumpSpecResourcesAndCleanup(ctx context.Context, specName string, clusterPr }) if !skipCleanup { - Byf("Deleting cluster %s/%s", cluster.Namespace, cluster.Name) + if cluster != nil { + Byf("Deleting cluster %s/%s", cluster.Namespace, cluster.Name) + } // While https://github.com/kubernetes-sigs/cluster-api/issues/2955 is addressed in future iterations, there is a chance // that cluster variable is not set even if the cluster exists, so we are calling DeleteAllClustersAndWait // instead of DeleteClusterAndWait diff --git a/test/e2e/nutanix_client_test.go b/test/e2e/nutanix_client_test.go index 4779c8fee1..98a4038185 100644 --- a/test/e2e/nutanix_client_test.go +++ b/test/e2e/nutanix_client_test.go @@ -72,8 +72,8 @@ var _ = Describe("Nutanix client", Label("capx-feature-test", "nutanix-client"), dumpSpecResourcesAndCleanup(ctx, specName, bootstrapClusterProxy, artifactFolder, namespace, cancelWatches, clusterResources.Cluster, e2eConfig.GetIntervals, skipCleanup) }) - // credentialRef is a mandatory parameters for the prismCentral attribute - It("Create a cluster without credentialRef (should fail)", func() { + // credentialRef is a optaional parameters for the prismCentral attribute + It("Create a cluster without credentialRef (should not fail as it will use default creds of controller)", Label("cluster-without-credref"), func() { flavor = "no-nutanix-cluster" Expect(namespace).NotTo(BeNil()) @@ -114,16 +114,14 @@ var _ = Describe("Nutanix client", Label("capx-feature-test", "nutanix-client"), }, clusterResources) }) - By("Checking CredentialRefSecretOwnerSet condition is false", func() { + By("Checking CredentialRefSecretOwnerSet condition is true", func() { testHelper.verifyConditionOnNutanixCluster(verifyConditionParams{ clusterName: clusterName, namespace: namespace, bootstrapClusterProxy: bootstrapClusterProxy, expectedCondition: clusterv1.Condition{ - Type: infrav1.CredentialRefSecretOwnerSetCondition, - Status: corev1.ConditionFalse, - Reason: infrav1.CredentialRefSecretOwnerSetFailed, - Severity: clusterv1.ConditionSeverityError, + Type: infrav1.CredentialRefSecretOwnerSetCondition, + Status: corev1.ConditionTrue, }, }) }) @@ -131,11 +129,11 @@ var _ = Describe("Nutanix client", Label("capx-feature-test", "nutanix-client"), By("PASSED!") }) - It("Create a cluster without prismCentral attribute (use default credentials)", func() { + It("Create a cluster without prismCentral attribute", Label("cluster-without-prism-central"), func() { flavor = "no-nutanix-cluster" Expect(namespace).NotTo(BeNil()) - By("Creating NutanixCluster resource without credentialRef", func() { + By("Creating NutanixCluster resource without prismCentral", func() { ntnxCluster := testHelper.createDefaultNutanixCluster( clusterName, namespace.Name, @@ -143,39 +141,20 @@ var _ = Describe("Nutanix client", Label("capx-feature-test", "nutanix-client"), controlplaneEndpointPort, ) - testHelper.createCapiObject(ctx, createCapiObjectParams{ - creator: bootstrapClusterProxy.GetClient(), - capiObject: ntnxCluster, - }) - }) - - By("Creating a workload cluster", func() { - testHelper.deployCluster( - deployClusterParams{ - clusterName: clusterName, - namespace: namespace, - flavor: flavor, - clusterctlConfigPath: clusterctlConfigPath, - artifactFolder: artifactFolder, - bootstrapClusterProxy: bootstrapClusterProxy, - }, clusterResources) - }) - By("Checking cluster prism client init condition is true", func() { - testHelper.verifyConditionOnNutanixCluster(verifyConditionParams{ - clusterName: clusterName, - namespace: namespace, - bootstrapClusterProxy: bootstrapClusterProxy, - expectedCondition: clusterv1.Condition{ - Type: infrav1.PrismCentralClientCondition, - Status: corev1.ConditionTrue, - }, - }) + // Creating NutanixCluster without PrismCentral set should not succeed + // as its a required parameter + Eventually(func() error { + return testHelper.createObject(ctx, createCapiObjectParams{ + creator: bootstrapClusterProxy.GetClient(), + capiObject: ntnxCluster, + }) + }, defaultTimeout, defaultInterval).ShouldNot(Succeed()) }) By("PASSED!") }) - It("Create a cluster without secret and add it later", func() { + It("Create a cluster without secret and add it later", Label("cluster-without-secret-add-later"), func() { flavor = "no-secret" Expect(namespace).NotTo(BeNil()) @@ -242,7 +221,7 @@ var _ = Describe("Nutanix client", Label("capx-feature-test", "nutanix-client"), By("PASSED!") }) - It("Create a cluster with invalid credentials (should fail)", func() { + It("Create a cluster with invalid credentials (should fail)", Label("cluster-wit-invalid-creds"), func() { const ( flavor = "no-secret" ) diff --git a/test/e2e/test_helpers.go b/test/e2e/test_helpers.go index 74e76e411e..4a025cbbf6 100644 --- a/test/e2e/test_helpers.go +++ b/test/e2e/test_helpers.go @@ -122,6 +122,7 @@ type testHelperInterface interface { createDefaultNMT(clusterName, namespace string) *infrav1.NutanixMachineTemplate createDefaultNutanixCluster(clusterName, namespace, controlPlaneEndpointIP string, controlPlanePort int32) *infrav1.NutanixCluster createNameGPUNMT(ctx context.Context, clusterName, namespace string, params createGPUNMTParams) *infrav1.NutanixMachineTemplate + createObject(ctx context.Context, params createCapiObjectParams) error createCapiObject(ctx context.Context, params createCapiObjectParams) createDeviceIDGPUNMT(ctx context.Context, clusterName, namespace string, params createGPUNMTParams) *infrav1.NutanixMachineTemplate createSecret(params createSecretParams) @@ -362,9 +363,13 @@ type createCapiObjectParams struct { capiObject client.Object } +func (t testHelper) createObject(ctx context.Context, params createCapiObjectParams) error { + return params.creator.Create(ctx, params.capiObject) +} + func (t testHelper) createCapiObject(ctx context.Context, params createCapiObjectParams) { Eventually(func() error { - return params.creator.Create(ctx, params.capiObject) + return t.createObject(ctx, params) }, defaultTimeout, defaultInterval).Should(Succeed()) }