From fcde8a5e143a8ccaca0b55dcf7543d728c4b25da Mon Sep 17 00:00:00 2001 From: Kishen V Date: Thu, 23 May 2024 09:51:18 +0530 Subject: [PATCH] Refactor small sections of code and tests --- Makefile | 3 ++ README.md | 4 +-- .../controllers/nodeupdate_controller.go | 16 ++++------ adhoc-controllers/main.go | 9 +++--- cmd/main.go | 4 +-- cmd/options.go | 6 ++-- go.mod | 2 +- pkg/cloud/cloud_interface.go | 4 +-- pkg/cloud/metadata.go | 4 +-- pkg/cloud/mocks/mock_cloud.go | 14 ++++----- pkg/cloud/powervs.go | 30 +++++-------------- pkg/cloud/powervs_node.go | 11 ++++--- pkg/driver/controller.go | 7 ++--- pkg/driver/controller_test.go | 8 ++--- pkg/driver/sanity_test.go | 8 ++--- pkg/driver/validation.go | 4 ++- pkg/driver/validation_test.go | 4 +-- tests/e2e/README.md | 17 +++++++---- tests/e2e/pre_provisioning.go | 24 ++++++--------- tests/e2e/suite_test.go | 3 +- tests/e2e/testsuites/testsuites.go | 6 ++-- tests/remote/pi_resources.go | 10 ++++--- 22 files changed, 93 insertions(+), 105 deletions(-) diff --git a/Makefile b/Makefile index 5150a9155..15c6dd423 100644 --- a/Makefile +++ b/Makefile @@ -129,3 +129,6 @@ init-buildx: test-integration: go test -v -timeout 100m sigs.k8s.io/ibm-powervs-block-csi-driver/tests/it -run ^TestIntegration$ + +test-e2e: + go test -v -timeout 100m sigs.k8s.io/ibm-powervs-block-csi-driver/tests/e2e -run ^TestE2E$ diff --git a/README.md b/README.md index 80bb91899..d1a949c69 100644 --- a/README.md +++ b/README.md @@ -76,7 +76,7 @@ Please see the compatibility matrix above before you deploy the driver To deploy the CSI driver: ```sh -kubectl apply -k "https://github.com/kubernetes-sigs/ibm-powervs-block-csi-driver/deploy/kubernetes/overlays/stable/?ref=v0.3.0" +kubectl apply -k "https://github.com/kubernetes-sigs/ibm-powervs-block-csi-driver/deploy/kubernetes/overlays/stable/?ref=v0.6.0" ``` Verify driver is running: @@ -87,7 +87,7 @@ kubectl get pods -n kube-system #### Deploy driver with debug mode To view driver debug logs, run the CSI driver with `-v=5` command line option -To enable powervs debug logs, run the CSI driver with `debug=true` command line option. +To enable PowerVS debug logs, run the CSI driver with `-debug=true` command line option. ## Examples Make sure you follow the [Prerequisites](README.md#Prerequisites) before the examples: diff --git a/adhoc-controllers/controllers/nodeupdate_controller.go b/adhoc-controllers/controllers/nodeupdate_controller.go index ba68747a5..e3bf54969 100644 --- a/adhoc-controllers/controllers/nodeupdate_controller.go +++ b/adhoc-controllers/controllers/nodeupdate_controller.go @@ -20,7 +20,6 @@ import ( "context" "fmt" - "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -63,7 +62,7 @@ func (r *NodeUpdateReconciler) Reconcile(_ context.Context, req ctrl.Request) (c klog.Infof("PROVIDER-ID: %s", node.Spec.ProviderID) metadata, err := cloud.TokenizeProviderID(node.Spec.ProviderID) if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to tokenize the providerID and err: %v", err) + return ctrl.Result{}, fmt.Errorf("failed to tokenize the providerID. err: %v", err) } nodeUpdateScope, err := cloud.NewNodeUpdateScope(cloud.NodeUpdateScopeParams{ @@ -73,12 +72,12 @@ func (r *NodeUpdateReconciler) Reconcile(_ context.Context, req ctrl.Request) (c }) if err != nil { - return ctrl.Result{}, errors.Errorf("failed to create nodeUpdateScope: %+v", err) + return ctrl.Result{}, fmt.Errorf("failed to create nodeUpdateScope: %w", err) } instance, err := nodeUpdateScope.Cloud.GetPVMInstanceDetails(nodeUpdateScope.InstanceId) if err != nil { - klog.Infof("Unable to fetch Instance Details %v", err) + klog.Errorf("unable to fetch instance details. err: %v", err) return ctrl.Result{}, nil } @@ -92,8 +91,8 @@ func (r *NodeUpdateReconciler) Reconcile(_ context.Context, req ctrl.Request) (c } else { err := r.getOrUpdate(nodeUpdateScope) if err != nil { - klog.Infof("unable to update instance StoragePoolAffinity %v", err) - return ctrl.Result{}, errors.Wrapf(err, "failed to reconcile VSI for IBMPowerVSMachine %s/%s", node.Namespace, node.Name) + klog.Errorf("unable to update instance StoragePoolAffinity. err: %v", err) + return ctrl.Result{}, fmt.Errorf("failed to reconcile VSI for IBMPowerVSMachine %s/%s. err: %w", node.Namespace, node.Name, err) } } default: @@ -107,10 +106,7 @@ func (r *NodeUpdateReconciler) Reconcile(_ context.Context, req ctrl.Request) (c } func (r *NodeUpdateReconciler) getOrUpdate(scope *cloud.NodeUpdateScope) error { - if err := scope.Cloud.UpdateStoragePoolAffinity(scope.InstanceId); err != nil { - return err - } - return nil + return scope.Cloud.UpdateStoragePoolAffinity(scope.InstanceId) } // SetupWithManager sets up the controller with the Manager. diff --git a/adhoc-controllers/main.go b/adhoc-controllers/main.go index 27119d322..eb83cc734 100644 --- a/adhoc-controllers/main.go +++ b/adhoc-controllers/main.go @@ -49,10 +49,11 @@ func init() { } func main() { - var metricsAddr string - var enableLeaderElection bool - var probeAddr string - var webhookPort int + var ( + metricsAddr, probeAddr string + enableLeaderElection bool + webhookPort int + ) flag.StringVar(&metricsAddr, "metrics-bind-address", ":8081", "The address the metric endpoint binds to.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8082", "The address the probe endpoint binds to.") diff --git a/cmd/main.go b/cmd/main.go index 964b28a41..890d9982f 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -39,9 +39,9 @@ func main() { driver.WithCloudConfig(options.ServerOptions.Cloudconfig), ) if err != nil { - klog.Fatalln(err) + klog.Fatalf("unable to create CSI driver. err: %v", err) } if err := drv.Run(); err != nil { - klog.Fatalln(err) + klog.Fatalf("failed to run the CSI driver. err: %v", err) } } diff --git a/cmd/options.go b/cmd/options.go index 45d946d00..59f578b1a 100644 --- a/cmd/options.go +++ b/cmd/options.go @@ -89,15 +89,15 @@ func GetOptions(fs *flag.FlagSet) *Options { } if err := fs.Parse(args); err != nil { - klog.Fatal(err) + klog.Fatalf("Error while parsing flag options. err: %v", err) } if *version { info, err := driver.GetVersionJSON() if err != nil { - klog.Fatalln(err) + klog.Fatalf("error while retriving the CSI driver version. err: %v", err) } - klog.Infoln(info) + klog.Info(info) osExit(0) } diff --git a/go.mod b/go.mod index ab7d9c06a..faa0e4216 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,6 @@ require ( github.com/kubernetes-csi/csi-test v2.2.0+incompatible github.com/onsi/ginkgo/v2 v2.19.0 github.com/onsi/gomega v1.33.1 - github.com/pkg/errors v0.9.1 go.uber.org/mock v0.4.0 golang.org/x/sys v0.22.0 google.golang.org/grpc v1.65.0 @@ -95,6 +94,7 @@ require ( github.com/opencontainers/go-digest v1.0.0 // indirect github.com/opencontainers/selinux v1.11.0 // indirect github.com/opentracing/opentracing-go v1.2.0 // indirect + github.com/pkg/errors v0.9.1 // indirect github.com/prometheus/client_golang v1.17.0 // indirect github.com/prometheus/client_model v0.6.1 // indirect github.com/prometheus/common v0.44.0 // indirect diff --git a/pkg/cloud/cloud_interface.go b/pkg/cloud/cloud_interface.go index 0851e37aa..05d2f90f0 100644 --- a/pkg/cloud/cloud_interface.go +++ b/pkg/cloud/cloud_interface.go @@ -22,7 +22,7 @@ import ( type Cloud interface { CreateDisk(volumeName string, diskOptions *DiskOptions) (disk *Disk, err error) - DeleteDisk(volumeID string) (success bool, err error) + DeleteDisk(volumeID string) (err error) AttachDisk(volumeID string, nodeID string) (err error) DetachDisk(volumeID string, nodeID string) (err error) ResizeDisk(volumeID string, reqSize int64) (newSize int64, err error) @@ -33,5 +33,5 @@ type Cloud interface { GetPVMInstanceByID(instanceID string) (instance *PVMInstance, err error) GetPVMInstanceDetails(instanceID string) (*models.PVMInstance, error) UpdateStoragePoolAffinity(instanceID string) error - IsAttached(volumeID string, nodeID string) (attached bool, err error) + IsAttached(volumeID string, nodeID string) (err error) } diff --git a/pkg/cloud/metadata.go b/pkg/cloud/metadata.go index db0331e45..aecb2b4e3 100644 --- a/pkg/cloud/metadata.go +++ b/pkg/cloud/metadata.go @@ -87,10 +87,10 @@ func TokenizeProviderID(providerID string) (*Metadata, error) { // Get New Metadata Service func NewMetadataService(k8sAPIClient KubernetesAPIClient, kubeconfig string) (MetadataService, error) { - klog.Infof("retrieving instance data from kubernetes API") + klog.Info("Retrieving instance data from Kubernetes API") clientset, err := k8sAPIClient(kubeconfig) if err != nil { - klog.Warningf("error creating kubernetes api client: %v", err) + klog.Errorf("error creating Kubernetes API client: %v", err) return nil, fmt.Errorf("an error occured during creation of k8s API client: %w", err) } klog.Info("kubernetes API is available") diff --git a/pkg/cloud/mocks/mock_cloud.go b/pkg/cloud/mocks/mock_cloud.go index ce139eed1..4506fef82 100644 --- a/pkg/cloud/mocks/mock_cloud.go +++ b/pkg/cloud/mocks/mock_cloud.go @@ -70,12 +70,11 @@ func (mr *MockCloudMockRecorder) CreateDisk(volumeName, diskOptions any) *gomock } // DeleteDisk mocks base method. -func (m *MockCloud) DeleteDisk(volumeID string) (bool, error) { +func (m *MockCloud) DeleteDisk(volumeID string) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "DeleteDisk", volumeID) - ret0, _ := ret[0].(bool) - ret1, _ := ret[1].(error) - return ret0, ret1 + ret0, _ := ret[0].(error) + return ret0 } // DeleteDisk indicates an expected call of DeleteDisk. @@ -174,12 +173,11 @@ func (mr *MockCloudMockRecorder) GetPVMInstanceDetails(instanceID any) *gomock.C } // IsAttached mocks base method. -func (m *MockCloud) IsAttached(volumeID, nodeID string) (bool, error) { +func (m *MockCloud) IsAttached(volumeID, nodeID string) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "IsAttached", volumeID, nodeID) - ret0, _ := ret[0].(bool) - ret1, _ := ret[1].(error) - return ret0, ret1 + ret0, _ := ret[0].(error) + return ret0 } // IsAttached indicates an expected call of IsAttached. diff --git a/pkg/cloud/powervs.go b/pkg/cloud/powervs.go index 51254d49a..2b0e08531 100644 --- a/pkg/cloud/powervs.go +++ b/pkg/cloud/powervs.go @@ -159,38 +159,28 @@ func (p *powerVSCloud) CreateDisk(volumeName string, diskOptions *DiskOptions) ( return &Disk{CapacityGiB: capacityGiB, VolumeID: *v.VolumeID, DiskType: v.DiskType, WWN: strings.ToLower(v.Wwn)}, nil } -func (p *powerVSCloud) DeleteDisk(volumeID string) (success bool, err error) { - err = p.volClient.DeleteVolume(volumeID) - if err != nil { - return false, err - } - - return true, nil +func (p *powerVSCloud) DeleteDisk(volumeID string) (err error) { + return p.volClient.DeleteVolume(volumeID) } func (p *powerVSCloud) AttachDisk(volumeID string, nodeID string) (err error) { - err = p.volClient.Attach(nodeID, volumeID) - if err != nil { + if err = p.volClient.Attach(nodeID, volumeID); err != nil { return err } return p.WaitForVolumeState(volumeID, VolumeInUseState) - } func (p *powerVSCloud) DetachDisk(volumeID string, nodeID string) (err error) { - err = p.volClient.Detach(nodeID, volumeID) - if err != nil { + if err = p.volClient.Detach(nodeID, volumeID); err != nil { return err } return p.WaitForVolumeState(volumeID, VolumeAvailableState) } -func (p *powerVSCloud) IsAttached(volumeID string, nodeID string) (attached bool, err error) { +// IsAttached returns an error if a volume isn't attached to a node, else nil. +func (p *powerVSCloud) IsAttached(volumeID string, nodeID string) (err error) { _, err = p.volClient.CheckVolumeAttach(nodeID, volumeID) - if err != nil { - return false, err - } - return true, nil + return err } func (p *powerVSCloud) ResizeDisk(volumeID string, reqSize int64) (newSize int64, err error) { @@ -214,7 +204,7 @@ func (p *powerVSCloud) ResizeDisk(volumeID string, reqSize int64) (newSize int64 func (p *powerVSCloud) WaitForVolumeState(volumeID, state string) error { ctx := context.Background() - err := wait.PollUntilContextTimeout(ctx, PollInterval, PollTimeout, true, func(ctx context.Context) (bool, error) { + return wait.PollUntilContextTimeout(ctx, PollInterval, PollTimeout, true, func(ctx context.Context) (bool, error) { v, err := p.volClient.Get(volumeID) if err != nil { return false, err @@ -222,10 +212,6 @@ func (p *powerVSCloud) WaitForVolumeState(volumeID, state string) error { spew.Dump(v) return v.State == state, nil }) - if err != nil { - return err - } - return nil } func (p *powerVSCloud) GetDiskByName(name string) (disk *Disk, err error) { diff --git a/pkg/cloud/powervs_node.go b/pkg/cloud/powervs_node.go index 502aa68ef..711c43367 100644 --- a/pkg/cloud/powervs_node.go +++ b/pkg/cloud/powervs_node.go @@ -25,9 +25,12 @@ import ( ) const ( - PowerVSInstanceStateSHUTOFF = "SHUTOFF" - PowerVSInstanceStateACTIVE = "ACTIVE" - StoragePoolAffinity = false + PowerVSInstanceStateSHUTOFF = "SHUTOFF" + PowerVSInstanceStateACTIVE = "ACTIVE" + PowerVSInstanceStateERROR = "ERROR" + PowerVSInstanceHealthWARNING = "WARNING" + PowerVSInstanceHealthOK = "OK" + StoragePoolAffinity = false ) type NodeUpdateScopeParams struct { @@ -78,7 +81,7 @@ func (p *powerVSCloud) GetPVMInstanceDetails(instanceID string) (*models.PVMInst func (p *powerVSCloud) UpdateStoragePoolAffinity(instanceID string) error { _, err := p.pvmInstancesClient.Update(instanceID, &models.PVMInstanceUpdate{ - StoragePoolAffinity: ptr.To[bool](StoragePoolAffinity), + StoragePoolAffinity: ptr.To(StoragePoolAffinity), }) return err } diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index 01f09921f..fff129536 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -230,7 +230,7 @@ func (d *controllerService) DeleteVolume(ctx context.Context, req *csi.DeleteVol } } - if _, err := d.cloud.DeleteDisk(volumeID); err != nil { + if err := d.cloud.DeleteDisk(volumeID); err != nil { return nil, status.Errorf(codes.Internal, "Could not delete volume ID %q: %v", volumeID, err) } @@ -282,8 +282,7 @@ func (d *controllerService) ControllerPublishVolume(ctx context.Context, req *cs pvInfo := map[string]string{WWNKey: disk.WWN} - attached, _ := d.cloud.IsAttached(volumeID, nodeID) - if attached { + if err = d.cloud.IsAttached(volumeID, nodeID); err == nil { klog.V(5).Infof("ControllerPublishVolume: volume %s already attached to node %s, returning success", volumeID, nodeID) return &csi.ControllerPublishVolumeResponse{PublishContext: pvInfo}, nil } @@ -324,7 +323,7 @@ func (d *controllerService) ControllerUnpublishVolume(ctx context.Context, req * } } - if attached, err := d.cloud.IsAttached(volumeID, nodeID); !attached { + if err := d.cloud.IsAttached(volumeID, nodeID); err != nil { klog.V(4).Infof("ControllerUnpublishVolume: volume %s is not attached to %s, err: %v, returning with success", volumeID, nodeID, err) return &csi.ControllerUnpublishVolumeResponse{}, nil } diff --git a/pkg/driver/controller_test.go b/pkg/driver/controller_test.go index 09a97d3f7..c61bde3e4 100644 --- a/pkg/driver/controller_test.go +++ b/pkg/driver/controller_test.go @@ -747,7 +747,7 @@ func TestDeleteVolume(t *testing.T) { defer mockCtl.Finish() mockCloud := mocks.NewMockCloud(mockCtl) - mockCloud.EXPECT().DeleteDisk(gomock.Eq(req.VolumeId)).Return(true, nil) + mockCloud.EXPECT().DeleteDisk(gomock.Eq(req.VolumeId)).Return(nil) mockCloud.EXPECT().GetDiskByID(gomock.Eq(req.VolumeId)).Return(nil, nil) powervsDriver := controllerService{ cloud: mockCloud, @@ -811,7 +811,7 @@ func TestDeleteVolume(t *testing.T) { defer mockCtl.Finish() mockCloud := mocks.NewMockCloud(mockCtl) - mockCloud.EXPECT().DeleteDisk(gomock.Eq(req.VolumeId)).Return(false, fmt.Errorf("DeleteDisk could not delete volume")) + mockCloud.EXPECT().DeleteDisk(gomock.Eq(req.VolumeId)).Return(fmt.Errorf("DeleteDisk could not delete volume")) mockCloud.EXPECT().GetDiskByID(gomock.Eq(req.VolumeId)).Return(nil, nil) powervsDriver := controllerService{ cloud: mockCloud, @@ -906,7 +906,7 @@ func TestControllerPublishVolume(t *testing.T) { mockCloud := mocks.NewMockCloud(mockCtl) mockCloud.EXPECT().GetPVMInstanceByID(gomock.Eq(expInstanceID)).Return(nil, nil) mockCloud.EXPECT().GetDiskByID(gomock.Eq(volumeName)).Return(&cloud.Disk{WWN: expDevicePath}, nil) - mockCloud.EXPECT().IsAttached(gomock.Eq(volumeName), gomock.Eq(expInstanceID)).Return(false, nil) + mockCloud.EXPECT().IsAttached(gomock.Eq(volumeName), gomock.Eq(expInstanceID)).Return(fmt.Errorf("the disk is unattached")) mockCloud.EXPECT().AttachDisk(gomock.Eq(volumeName), gomock.Eq(expInstanceID)).Return(nil) powervsDriver := controllerService{ @@ -1199,7 +1199,7 @@ func TestControllerUnpublishVolume(t *testing.T) { mockCloud := mocks.NewMockCloud(mockCtl) mockCloud.EXPECT().GetDiskByID(gomock.Eq("vol-test")).Return(&cloud.Disk{WWN: expDevicePath}, nil) - mockCloud.EXPECT().IsAttached(gomock.Eq("vol-test"), gomock.Eq(expInstanceID)).Return(true, nil) + mockCloud.EXPECT().IsAttached(gomock.Eq("vol-test"), gomock.Eq(expInstanceID)).Return(nil) mockCloud.EXPECT().DetachDisk(req.VolumeId, req.NodeId).Return(nil) powervsDriver := controllerService{ diff --git a/pkg/driver/sanity_test.go b/pkg/driver/sanity_test.go index 220f940c1..8cea7e0b3 100644 --- a/pkg/driver/sanity_test.go +++ b/pkg/driver/sanity_test.go @@ -186,13 +186,13 @@ func (c *fakeCloudProvider) CreateDisk(volumeName string, diskOptions *cloud.Dis return d.Disk, nil } -func (c *fakeCloudProvider) DeleteDisk(volumeID string) (bool, error) { +func (c *fakeCloudProvider) DeleteDisk(volumeID string) error { for volName, f := range c.disks { if f.Disk.VolumeID == volumeID { delete(c.disks, volName) } } - return true, nil + return nil } func (c *fakeCloudProvider) AttachDisk(volumeID, nodeID string) error { @@ -207,8 +207,8 @@ func (c *fakeCloudProvider) DetachDisk(volumeID, nodeID string) error { return nil } -func (c *fakeCloudProvider) IsAttached(volumeID string, nodeID string) (attached bool, err error) { - return true, nil +func (c *fakeCloudProvider) IsAttached(volumeID string, nodeID string) (err error) { + return nil } func (c *fakeCloudProvider) WaitForVolumeState(volumeID, expectedState string) error { diff --git a/pkg/driver/validation.go b/pkg/driver/validation.go index d0e5c8c33..f6f12ca0d 100644 --- a/pkg/driver/validation.go +++ b/pkg/driver/validation.go @@ -20,6 +20,8 @@ import ( "fmt" ) +var supportedModes = []Mode{AllMode, ControllerMode, NodeMode} + func ValidateDriverOptions(options *Options) error { if err := validateMode(options.mode); err != nil { return fmt.Errorf("invalid mode: %v", err) @@ -29,7 +31,7 @@ func ValidateDriverOptions(options *Options) error { func validateMode(mode Mode) error { if mode != AllMode && mode != ControllerMode && mode != NodeMode { - return fmt.Errorf("Mode is not supported (actual: %s, supported: %v)", mode, []Mode{AllMode, ControllerMode, NodeMode}) + return fmt.Errorf("Mode is not supported (actual: %s, supported: %v)", mode, supportedModes) } return nil } diff --git a/pkg/driver/validation_test.go b/pkg/driver/validation_test.go index 62a205842..e83da2976 100644 --- a/pkg/driver/validation_test.go +++ b/pkg/driver/validation_test.go @@ -43,7 +43,7 @@ func TestValidateMode(t *testing.T) { { name: "invalid mode: unknown", mode: Mode("unknown"), - expErr: fmt.Errorf("Mode is not supported (actual: unknown, supported: %v)", []Mode{AllMode, ControllerMode, NodeMode}), + expErr: fmt.Errorf("Mode is not supported (actual: unknown, supported: %v)", supportedModes), }, } @@ -72,7 +72,7 @@ func TestValidateDriverOptions(t *testing.T) { { name: "fail because validateMode fails", mode: Mode("unknown"), - expErr: fmt.Errorf("invalid mode: Mode is not supported (actual: unknown, supported: %v)", []Mode{AllMode, ControllerMode, NodeMode}), + expErr: fmt.Errorf("invalid mode: Mode is not supported (actual: unknown, supported: %v)", supportedModes), }, } diff --git a/tests/e2e/README.md b/tests/e2e/README.md index c8eb3ad72..f40b2f5c1 100644 --- a/tests/e2e/README.md +++ b/tests/e2e/README.md @@ -2,16 +2,23 @@ E2E test verifies the funcitonality of IBM PowerVS Block CSI Driver in the context of Kubernetes. It exercises driver feature e2e including static provisioning, dynamic provisioning, volume scheduling, mount options, etc. ### Notes -Some tests marked with `[env]` require specific environmental variables to be set, if not set these tests will be skipped. +Tests marked with `[env]` require specific environmental variables to be set, if not set these tests will be skipped. ``` export IBMCLOUD_API_KEY=XXXXXXXXXXXXXXXXXXXXXXXXXXXXXX ``` -Some tests marked with `[labels]` require specific labels to be set for each node of the cluster, if not set these tests will be skipped. +Tests marked with `[labels]` requires specific labels to be set for each node of the cluster, if not set these tests will be skipped. + +``` +kubectl label nodes powervs.kubernetes.io/cloud-instance-id= +kubectl label nodes powervs.kubernetes.io/pvm-instance-id= ``` -kubectl label nodes multinode-kube-master powervs.kubernetes.io/cloud-instance-id=7845d372-d4e1-46b8-91fc-41051c984601 -kubectl label nodes multinode-kube-master powervs.kubernetes.io/pvm-instance-id=638667f8-a4d3-46d0-9fa6-ddc621100407 + +Set the ProviderID on the cluster nodes as: `ibmpowervs://///`, for example: +```sh +spec: + providerID: ibmpowervs://syd/syd05/862032d5-xxxx-xxxx-xxxx-c18594456427/2c6cbaec-xxxx-xxxx-xxxx-a6aa35315596 ``` -- The **[Node Controller](https://kubernetes.io/docs/concepts/architecture/cloud-controller/#node-controller)** of the the **[CloudControllerManager](https://kubernetes.io/docs/concepts/architecture/cloud-controller)** is responsible for labelling the nodes. The above one is used as temporary fix. \ No newline at end of file +- The **[Node Controller](https://kubernetes.io/docs/concepts/architecture/cloud-controller/#node-controller)** of the the **[CloudControllerManager](https://kubernetes.io/docs/concepts/architecture/cloud-controller)** is responsible for labelling the nodes. \ No newline at end of file diff --git a/tests/e2e/pre_provisioning.go b/tests/e2e/pre_provisioning.go index d1802a5e8..fbd4be2aa 100644 --- a/tests/e2e/pre_provisioning.go +++ b/tests/e2e/pre_provisioning.go @@ -23,10 +23,13 @@ import ( "time" . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + v1 "k8s.io/api/core/v1" clientset "k8s.io/client-go/kubernetes" "k8s.io/kubernetes/test/e2e/framework" admissionapi "k8s.io/pod-security-admission/api" + powervscloud "sigs.k8s.io/ibm-powervs-block-csi-driver/pkg/cloud" powervscsidriver "sigs.k8s.io/ibm-powervs-block-csi-driver/pkg/driver" "sigs.k8s.io/ibm-powervs-block-csi-driver/tests/e2e/driver" @@ -69,26 +72,21 @@ var _ = Describe("[powervs-csi-e2e]Pre-Provisioned", func() { Shareable: false, } - // setup Power VS volume + // setup PowerVS volume if os.Getenv(apiKeyEnv) == "" { Skip(fmt.Sprintf("env %q not set", apiKeyEnv)) } - var err error metadata, err := testsuites.GetInstanceMetadataFromNodeSpec(cs) if err != nil { Skip(fmt.Sprintf("Could not get cloudInstanceId : %v", err)) } cloud, err = powervscloud.NewPowerVSCloud(metadata.GetCloudInstanceId(), metadata.GetZone(), debug) - if err != nil { - Fail(fmt.Sprintf("could not get NewCloud: %v", err)) - } + Expect(err).NotTo(HaveOccurred(), fmt.Sprintf("Client creation for instances and volumes has failed. err: %v", err)) r1 := rand.New(rand.NewSource(time.Now().UnixNano())) disk, err := cloud.CreateDisk(fmt.Sprintf("pvc-%d", r1.Uint64()), diskOptions) - if err != nil { - Fail(fmt.Sprintf("Create Disk failed: %v", err)) - } + Expect(err).NotTo(HaveOccurred(), fmt.Sprintf("Disk creation has failed. err: %v", err)) volumeID = disk.VolumeID diskSize = fmt.Sprintf("%dGi", defaultDiskSize) @@ -99,13 +97,9 @@ var _ = Describe("[powervs-csi-e2e]Pre-Provisioned", func() { skipManuallyDeletingVolume = true if !skipManuallyDeletingVolume { err := cloud.WaitForVolumeState(volumeID, "detached") - if err != nil { - Fail(fmt.Sprintf("could not detach volume %q: %v", volumeID, err)) - } - ok, err := cloud.DeleteDisk(volumeID) - if err != nil || !ok { - Fail(fmt.Sprintf("could not delete volume %q: %v", volumeID, err)) - } + Expect(err).NotTo(HaveOccurred(), fmt.Sprintf("Detach volume has failed. err: %v", err)) + err = cloud.DeleteDisk(volumeID) + Expect(err).NotTo(HaveOccurred(), fmt.Sprintf("Disk deletion has failed. err: %v", err)) } }) diff --git a/tests/e2e/suite_test.go b/tests/e2e/suite_test.go index 027e25c17..451e70c33 100644 --- a/tests/e2e/suite_test.go +++ b/tests/e2e/suite_test.go @@ -54,7 +54,6 @@ func TestE2E(t *testing.T) { RegisterFailHandler(Fail) // Run tests through the Ginkgo runner with output to console + JUnit for Jenkins - var r []Reporter if framework.TestContext.ReportDir != "" { // Create the directory if it doesn't already exists // NOTE: junit report can be created with new --junit-report flag @@ -64,5 +63,5 @@ func TestE2E(t *testing.T) { } } - RunSpecsWithDefaultAndCustomReporters(t, "IBM PowerVS Block CSI Driver End-to-End Tests", r) + RunSpecs(t, "IBM PowerVS Block CSI Driver End-to-End Tests") } diff --git a/tests/e2e/testsuites/testsuites.go b/tests/e2e/testsuites/testsuites.go index 4492f5590..5ab5c8c41 100644 --- a/tests/e2e/testsuites/testsuites.go +++ b/tests/e2e/testsuites/testsuites.go @@ -288,10 +288,8 @@ func (t *TestPersistentVolumeClaim) DeleteBoundPersistentVolume() { func (t *TestPersistentVolumeClaim) DeleteBackingVolume(cloud powervscloud.Cloud) { volumeID := t.persistentVolume.Spec.CSI.VolumeHandle By(fmt.Sprintf("deleting PowerVS volume %q", volumeID)) - ok, err := cloud.DeleteDisk(volumeID) - if err != nil || !ok { - Fail(fmt.Sprintf("could not delete volume %q: %v", volumeID, err)) - } + err := cloud.DeleteDisk(volumeID) + framework.ExpectNoError(err) } type TestDeployment struct { diff --git a/tests/remote/pi_resources.go b/tests/remote/pi_resources.go index 409b48be4..182cab164 100644 --- a/tests/remote/pi_resources.go +++ b/tests/remote/pi_resources.go @@ -26,6 +26,8 @@ import ( "github.com/IBM-Cloud/power-go-client/power/models" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/klog/v2" + + "sigs.k8s.io/ibm-powervs-block-csi-driver/pkg/cloud" ) func (r *Remote) createPVSResources() (err error) { @@ -244,14 +246,14 @@ func waitForInstanceHealth(insID string, ic *instance.IBMPIInstanceClient) (*mod if err != nil { return false, err } - if *pvm.Status == "ERROR" { + if *pvm.Status == cloud.PowerVSInstanceStateERROR { if pvm.Fault != nil { - return false, fmt.Errorf("failed to create the lpar: %s", pvm.Fault.Message) + return false, fmt.Errorf("failed to create a PowerVS instance: %s", pvm.Fault.Message) } - return false, fmt.Errorf("failed to create the lpar") + return false, fmt.Errorf("failed to create a PowerVS instance") } // Check for `instanceReadyStatus` health status and also the final health status "OK" - if *pvm.Status == "ACTIVE" && (pvm.Health.Status == "WARNING" || pvm.Health.Status == "OK") { + if *pvm.Status == cloud.PowerVSInstanceStateACTIVE && (pvm.Health.Status == cloud.PowerVSInstanceHealthWARNING || pvm.Health.Status == cloud.PowerVSInstanceHealthOK) { return true, nil }