From 1779f49205fcdf631a16564b6dd6f925b13bce90 Mon Sep 17 00:00:00 2001 From: Clinton Knight Date: Fri, 27 Sep 2024 10:31:33 -0400 Subject: [PATCH] Disable ACP --- acp/client.go | 16 ---- acp/client_test.go | 34 +------- cli/cmd/version.go | 23 ++---- frontend/crd/snapshot_restore_test.go | 78 ------------------- .../crd/trident_action_mirror_update_test.go | 55 ------------- frontend/rest/controller_handlers.go | 3 +- .../controllers/orchestrator/controller.go | 2 +- storage/backend_test.go | 38 --------- storage_drivers/azure/azure_anf_test.go | 10 +-- storage_drivers/ontap/ontap_nas_qtree_test.go | 39 +--------- 10 files changed, 13 insertions(+), 285 deletions(-) diff --git a/acp/client.go b/acp/client.go index b3f69323d..504702794 100644 --- a/acp/client.go +++ b/acp/client.go @@ -124,21 +124,5 @@ func (c *client) GetVersionWithBackoff(ctx context.Context) (*version.Version, e } func (c *client) IsFeatureEnabled(ctx context.Context, feature string) error { - fields := LogFields{"feature": feature} - Logc(ctx).WithFields(fields).Debug("Checking if feature is enabled.") - - if !c.Enabled() { - Logc(ctx).WithFields(fields).Warning("Feature requires Trident-ACP to be enabled.") - return errors.UnsupportedConfigError("acp is not enabled") - } - - // Entitled will return different errors based on the response from the API call. Return the exact error - // so consumers of this client may act on certain error conditions. - if err := c.restClient.Entitled(ctx, feature); err != nil { - Logc(ctx).WithFields(fields).WithError(err).Error("Feature enablement failed.") - return err - } - - Logc(ctx).WithFields(fields).Debug("Feature is enabled.") return nil } diff --git a/acp/client_test.go b/acp/client_test.go index a3ce3df39..05ecaaae4 100644 --- a/acp/client_test.go +++ b/acp/client_test.go @@ -92,38 +92,6 @@ func TestTridentACP_IsFeatureEnabled(t *testing.T) { // Reset the package-level state after the test completes. client := newClient(nil, false) err := client.IsFeatureEnabled(ctx, FeatureSnapshotRestore) - assert.Error(t, err, "expected error") - assert.True(t, errors.IsUnsupportedConfigError(err), "should be unsupported config error") - assert.False(t, errors.IsUnlicensedError(err), "should not be unlicensed error") - }) - - t.Run("WithAPIErrorDuringFeatureEntitlementCheck", func(t *testing.T) { - // Reset the package-level state after the test completes. - testFeature := FeatureSnapshotRestore - - mockCtrl := gomock.NewController(t) - mockRest := mock_acp.NewMockREST(mockCtrl) - mockRest.EXPECT().Entitled(ctx, testFeature).Return(fmt.Errorf("api error")) - - client := newClient(mockRest, true) - err := client.IsFeatureEnabled(ctx, testFeature) - assert.Error(t, err, "expected error") - assert.False(t, errors.IsUnsupportedConfigError(err), "should not be unsupported config error") - assert.False(t, errors.IsUnlicensedError(err), "should not be unlicensed error") - }) - - t.Run("WhenFeatureIsNotSupported", func(t *testing.T) { - // Reset the package-level state after the test completes. - testFeature := FeatureSnapshotRestore - - mockCtrl := gomock.NewController(t) - mockRest := mock_acp.NewMockREST(mockCtrl) - mockRest.EXPECT().Entitled(ctx, testFeature).Return(errors.UnlicensedError("unlicensed error")) - - client := newClient(mockRest, true) - err := client.IsFeatureEnabled(ctx, testFeature) - assert.Error(t, err, "expected error") - assert.False(t, errors.IsUnsupportedConfigError(err), "should be unsupported config error") - assert.True(t, errors.IsUnlicensedError(err), "should be unlicensed error") + assert.NoError(t, err, "expected no error") }) } diff --git a/cli/cmd/version.go b/cli/cmd/version.go index 567bb1bf9..a1fa0a952 100644 --- a/cli/cmd/version.go +++ b/cli/cmd/version.go @@ -226,21 +226,14 @@ func writeVersionTable(version *api.ClientVersionResponse) { func writeVersionsTable(versions *api.VersionResponse) { table := tablewriter.NewWriter(os.Stdout) - if versions.ACPServer != nil { - table.SetHeader([]string{"Server Version", "Client Version", "ACP Version"}) - table.Append([]string{ - versions.Server.Version, - versions.Client.Version, - versions.ACPServer.Version, - }) - } else { - table.SetHeader([]string{"Server Version", "Client Version"}) - - table.Append([]string{ - versions.Server.Version, - versions.Client.Version, - }) - } + + table.SetHeader([]string{"Server Version", "Client Version"}) + + table.Append([]string{ + versions.Server.Version, + versions.Client.Version, + }) + table.Render() } diff --git a/frontend/crd/snapshot_restore_test.go b/frontend/crd/snapshot_restore_test.go index 4dfc59d00..f476ba00e 100644 --- a/frontend/crd/snapshot_restore_test.go +++ b/frontend/crd/snapshot_restore_test.go @@ -247,84 +247,6 @@ func TestHandleActionSnapshotRestore(t *testing.T) { assert.True(t, apierrors.IsNotFound(err), "TASR should not have been found") } -func TestHandleActionSnapshotRestore_Disabled(t *testing.T) { - // Reset the package-level state after the test completes. - defer acp.SetAPI(acp.API()) - - mockCtrl := gomock.NewController(t) - mockACP := mockacp.NewMockTridentACP(mockCtrl) - acp.SetAPI(mockACP) - orchestrator := mockcore.NewMockOrchestrator(mockCtrl) - - tridentNamespace := "trident" - kubeClient := GetTestKubernetesClientset() - snapClient := GetTestSnapshotClientset() - crdClient := GetTestCrdClientset() - crdController, err := newTridentCrdControllerImpl(orchestrator, tridentNamespace, kubeClient, snapClient, crdClient) - if err != nil { - t.Fatalf("cannot create Trident CRD controller frontend; %v", err) - } - - // Mock out any expected calls on the ACP API. - // This test expects the feature call to return false / unsupported. - err = errors.UnsupportedError("unsupported feature") - mockACP.EXPECT().IsFeatureEnabled(gomock.Any(), acp.FeatureSnapshotRestore).Return(err).AnyTimes() - - // Activate the CRD controller and start monitoring - if err = crdController.Activate(); err != nil { - t.Fatalf("error while activating; %v", err) - } - time.Sleep(250 * time.Millisecond) - - pvc := fakeSnapRestorePVC(snapRestorePVC1, namespace1, snapRestorePV1) - _, _ = kubeClient.CoreV1().PersistentVolumeClaims(namespace1).Create(ctx(), pvc, createOpts) - - pv := fakePV(snapRestorePVC1, namespace1, snapRestorePV1) - _, _ = kubeClient.CoreV1().PersistentVolumes().Create(ctx(), pv, createOpts) - - vs1Time := time.Now() - vs2Time := vs1Time.Add(1 * time.Second) - vs3Time := vs2Time.Add(1 * time.Second) - - vs1 := fakeVS(snapRestoreSnap1, namespace1, snapRestoreVSC1, snapRestorePVC1, vs1Time) - _, _ = snapClient.SnapshotV1().VolumeSnapshots(namespace1).Create(ctx(), vs1, createOpts) - - vsc1 := fakeVSC(snapRestoreSnap1, namespace1, snapRestoreVSC1, snapRestoreSnapHandle1, vs1Time) - _, _ = snapClient.SnapshotV1().VolumeSnapshotContents().Create(ctx(), vsc1, createOpts) - - vs2 := fakeVS(snapRestoreSnap2, namespace1, snapRestoreVSC2, snapRestorePVC1, vs2Time) - _, _ = snapClient.SnapshotV1().VolumeSnapshots(namespace1).Create(ctx(), vs2, createOpts) - - vsc2 := fakeVSC(snapRestoreSnap2, namespace1, snapRestoreVSC2, snapRestoreSnapHandle2, vs2Time) - _, _ = snapClient.SnapshotV1().VolumeSnapshotContents().Create(ctx(), vsc2, createOpts) - - vs3 := fakeVS(snapRestoreSnap3, namespace1, snapRestoreVSC3, snapRestorePVC1, vs3Time) - _, _ = snapClient.SnapshotV1().VolumeSnapshots(namespace1).Create(ctx(), vs3, createOpts) - - vsc3 := fakeVSC(snapRestoreSnap3, namespace1, snapRestoreVSC3, snapRestoreSnapHandle3, vs3Time) - _, _ = snapClient.SnapshotV1().VolumeSnapshotContents().Create(ctx(), vsc3, createOpts) - - tasr := fakeTASR(tasr1, namespace1, snapRestorePVC1, snapRestoreSnap3) - _, _ = crdClient.TridentV1().TridentActionSnapshotRestores(namespace1).Create(ctx(), tasr, createOpts) - - // Wait until the operation completes - for i := 0; i < 20; i++ { - time.Sleep(250 * time.Millisecond) - - tasr, err = crdClient.TridentV1().TridentActionSnapshotRestores(namespace1).Get(ctx(), tasr1, getOpts) - if err != nil { - if apierrors.IsNotFound(err) { - continue - } - break - } else if tasr.IsComplete() { - break - } - } - - assert.True(t, tasr.Failed(), "TASR operation did not fail") -} - func TestHandleActionSnapshotRestore_InProgressError(t *testing.T) { // Reset the package-level state after the test completes. defer acp.SetAPI(acp.API()) diff --git a/frontend/crd/trident_action_mirror_update_test.go b/frontend/crd/trident_action_mirror_update_test.go index 1865f643b..ef35820a1 100644 --- a/frontend/crd/trident_action_mirror_update_test.go +++ b/frontend/crd/trident_action_mirror_update_test.go @@ -338,61 +338,6 @@ func TestHandleActionMirrorUpdate_InProgress(t *testing.T) { assert.True(t, tamu.Succeeded(), "TAMU operation failed") } -func TestHandleActionMirrorUpdate_InProgress_Disabled(t *testing.T) { - // Reset the package-level state after the test completes. - defer acp.SetAPI(acp.API()) - - mockCtrl := gomock.NewController(t) - mockACP := mockacp.NewMockTridentACP(mockCtrl) - acp.SetAPI(mockACP) - orchestrator := mockcore.NewMockOrchestrator(mockCtrl) - - tridentNamespace := "trident" - kubeClient := GetTestKubernetesClientset() - snapClient := GetTestSnapshotClientset() - crdClient := GetTestCrdClientset() - crdController, err := newTridentCrdControllerImpl(orchestrator, tridentNamespace, kubeClient, snapClient, crdClient) - if err != nil { - t.Fatalf("cannot create Trident CRD controller frontend, error: %v", err.Error()) - } - - // Mock out any expected calls on the ACP API. - err = errors.UnsupportedError("unsupported") - mockACP.EXPECT().IsFeatureEnabled(gomock.Any(), acp.FeatureSnapshotMirrorUpdate).Return(err).AnyTimes() - - // Activate the CRD controller and start monitoring - if err = crdController.Activate(); err != nil { - t.Fatalf("error while activating: %v", err.Error()) - } - delaySeconds(1) - - pvc := fakePVC(pvc1, namespace1, pv1) - _, _ = kubeClient.CoreV1().PersistentVolumeClaims(namespace1).Create(ctx(), pvc, createOpts) - - tmr := fakeTMR(tmrName1, namespace1, pvc1) - _, _ = crdClient.TridentV1().TridentMirrorRelationships(namespace1).Create(ctx(), tmr, createOpts) - - tamu := fakeTAMU(tamu1, namespace1, tmrName1, snapHandle1) - _, _ = crdClient.TridentV1().TridentActionMirrorUpdates(namespace1).Create(ctx(), tamu, createOpts) - - // Wait until the operation completes - for i := 0; i < 5; i++ { - time.Sleep(250 * time.Millisecond) - - tamu, err = crdClient.TridentV1().TridentActionMirrorUpdates(namespace1).Get(ctx(), tamu1, getOpts) - if err != nil { - if apierrors.IsNotFound(err) { - continue - } - break - } else if tamu.IsComplete() { - break - } - } - - assert.True(t, tamu.Failed(), "TAMU operation was not disabled") -} - func TestHandleActionMirrorUpdate_InProgressAtStartup(t *testing.T) { // Reset the package-level state after the test completes. defer acp.SetAPI(acp.API()) diff --git a/frontend/rest/controller_handlers.go b/frontend/rest/controller_handlers.go index de9e52909..8f3ef04f0 100644 --- a/frontend/rest/controller_handlers.go +++ b/frontend/rest/controller_handlers.go @@ -263,8 +263,7 @@ func GetVersion(w http.ResponseWriter, r *http.Request) { response.Error = err.Error() } response.Version = version - - response.ACPVersion = GetACPVersion(ctx) + response.ACPVersion = version return httpStatusCodeForGetUpdateList(err) }, ) diff --git a/operator/controllers/orchestrator/controller.go b/operator/controllers/orchestrator/controller.go index 5671b8c98..c88c999d9 100644 --- a/operator/controllers/orchestrator/controller.go +++ b/operator/controllers/orchestrator/controller.go @@ -1364,7 +1364,7 @@ func (c *Controller) updateTridentOrchestratorCRStatus( Version: version, Namespace: namespace, CurrentInstallationParams: installParams, - ACPVersion: acpVersion, + ACPVersion: version, } if reflect.DeepEqual(tridentCR.Status, newStatusDetails) { diff --git a/storage/backend_test.go b/storage/backend_test.go index 611fd07d6..ac3414fe4 100644 --- a/storage/backend_test.go +++ b/storage/backend_test.go @@ -149,44 +149,6 @@ func TestDeleteSnapshot_NotManaged(t *testing.T) { assert.Errorf(t, err, "expected err") } -func TestCloneVolume_FeatureDisabled(t *testing.T) { - // Reset the package-level state after the test completes. - defer acp.SetAPI(acp.API()) - - mockCtrl := gomock.NewController(t) - mockACP := mockacp.NewMockTridentACP(mockCtrl) - acp.SetAPI(mockACP) - - // Mock out any expected calls on the ACP API. - err := errors.UnsupportedError("unsupported") - mockACP.EXPECT().IsFeatureEnabled(gomock.Any(), acp.FeatureReadOnlyClone).Return(err) - - volumeName := "pvc-e9748b6b-8240-4fd8-97bc-868bf064ecd4" - volumeInternalName := "trident_pvc_e9748b6b_8240_4fd8_97bc_868bf064ecd4" - volumeConfig := &VolumeConfig{ - Version: "", - Name: volumeName, - InternalName: volumeInternalName, - } - volumeConfigDest := &VolumeConfig{ - Version: "", - Name: "pvc-deadbeef-8240-4fd8-97bc-868bf064ecd4", - InternalName: "trident_pvc_deadbeef_8240_4fd8_97bc_868bf064ecd4", - ReadOnlyClone: true, - } - - backend := &StorageBackend{ - state: Offline, - } - pool := NewStoragePool(nil, "test-pool1") - - // Both volume and snapshot not managed - _, err = backend.CloneVolume(context.Background(), volumeConfig, volumeConfigDest, pool, false) - - assert.Error(t, err, "expected err") - assert.True(t, errors.IsUnsupportedError(err)) -} - func TestCloneVolume_BackendOffline(t *testing.T) { // Reset the package-level state after the test completes. defer acp.SetAPI(acp.API()) diff --git a/storage_drivers/azure/azure_anf_test.go b/storage_drivers/azure/azure_anf_test.go index d4eb42471..69b520e53 100644 --- a/storage_drivers/azure/azure_anf_test.go +++ b/storage_drivers/azure/azure_anf_test.go @@ -8723,14 +8723,6 @@ func TestUpdateSnapshotDirectory_Failure(t *testing.T) { volConfig, _, _, _, filesystem := getStructsForCreateNFSVolume(ctx, driver, storagePool) - // CASE: ACP is not enabled - mockACP.EXPECT().IsFeatureEnabled(gomock.Any(), acp.FeatureReadOnlyClone).Return(errors.New("mock error")) - result, err := driver.updateSnapshotDirectory( - ctx, volConfig.InternalName, filesystem, "TRUE", map[string]*storage.Volume{}) - - assert.Nil(t, result) - assert.NotNil(t, err) - // CASE: Error while modifying volume mockACP.EXPECT().IsFeatureEnabled(gomock.Any(), acp.FeatureReadOnlyClone).Return(nil).AnyTimes() mockAPI.EXPECT().RefreshAzureResources(ctx).Return(nil).AnyTimes() @@ -8749,7 +8741,7 @@ func TestUpdateSnapshotDirectory_Failure(t *testing.T) { }, } - result, err = driver.updateSnapshotDirectory( + result, err := driver.updateSnapshotDirectory( ctx, volConfig.InternalName, filesystem, "TRUE", allVolumes) assert.Nil(t, result) diff --git a/storage_drivers/ontap/ontap_nas_qtree_test.go b/storage_drivers/ontap/ontap_nas_qtree_test.go index e0b113ba4..511695e11 100644 --- a/storage_drivers/ontap/ontap_nas_qtree_test.go +++ b/storage_drivers/ontap/ontap_nas_qtree_test.go @@ -4506,35 +4506,6 @@ func TestCanSnapshot_InvalidSnapshotDir(t *testing.T) { assert.NotNil(t, result, "result is nil") } -func TestCreateSnapshot_Disabled(t *testing.T) { - // Reset the package-level state after the test completes. - defer acp.SetAPI(acp.API()) - - mockCtrl := gomock.NewController(t) - mockACP := mockacp.NewMockTridentACP(mockCtrl) - acp.SetAPI(mockACP) - // Mock out any expected calls on the ACP API. - err := errors.UnsupportedError("unsupported") - mockACP.EXPECT().IsFeatureEnabled(gomock.Any(), acp.FeatureReadOnlyClone).Return(err) - - _, driver := newMockOntapNasQtreeDriver(t) - volConfig := &storage.VolumeConfig{ - Size: "1g", - Encryption: "false", - FileSystem: "nfs", - InternalName: flexvol, - InternalID: volInternalID, - } - - snapConfig := &storage.SnapshotConfig{ - InternalName: "snap1", - VolumeInternalName: "vol1", - } - _, err = driver.CreateSnapshot(ctx, snapConfig, volConfig) - - assert.Error(t, err, "no error occurred") -} - func TestCreateSnapshot_Success(t *testing.T) { // Reset the package-level state after the test completes. defer acp.SetAPI(acp.API()) @@ -5327,18 +5298,10 @@ func TestNASQtreeStorageDriver_UpdateSnapshotDirectory_Failure(t *testing.T) { "pvc-99138d85-6259-4830-ada0-30e45e21f843": mockVol3, } - // CASE: ACP is not enabled for this feature - mockACP.EXPECT().IsFeatureEnabled(gomock.Any(), acp.FeatureReadOnlyClone).Return(errors.UnsupportedError("unsupported")) - - result, resultErr := driver.updateSnapshotDirectory(ctx, mockVol1.Config, "invalid", true, "", allVolumes) - - assert.Error(t, resultErr) - assert.Nil(t, result) - // CASE: Invalid snapshot dir value mockACP.EXPECT().IsFeatureEnabled(gomock.Any(), acp.FeatureReadOnlyClone).Return(nil).AnyTimes() - result, resultErr = driver.updateSnapshotDirectory(ctx, mockVol1.Config, "invalid", true, "", allVolumes) + result, resultErr := driver.updateSnapshotDirectory(ctx, mockVol1.Config, "invalid", true, "", allVolumes) assert.Error(t, resultErr) assert.True(t, errors.IsInvalidInputError(resultErr))