From 8a9f7e740e95362110e31d3a81e121643497210d Mon Sep 17 00:00:00 2001 From: Raunak Pradip Shah Date: Thu, 16 Nov 2023 11:23:43 +0530 Subject: [PATCH] Address log comments --- cmd/csi-snapshotter/main.go | 2 +- cmd/snapshot-controller/main.go | 2 +- pkg/common-controller/groupsnapshot_controller_helper.go | 5 +++-- pkg/common-controller/snapshot_controller.go | 4 +++- pkg/sidecar-controller/snapshot_controller.go | 2 +- 5 files changed, 9 insertions(+), 6 deletions(-) diff --git a/cmd/csi-snapshotter/main.go b/cmd/csi-snapshotter/main.go index 80ee8dd34..a53a0378e 100644 --- a/cmd/csi-snapshotter/main.go +++ b/cmd/csi-snapshotter/main.go @@ -85,7 +85,7 @@ var ( retryIntervalStart = flag.Duration("retry-interval-start", time.Second, "Initial retry interval of failed volume snapshot creation or deletion. It doubles with each failure, up to retry-interval-max. Default is 1 second.") retryIntervalMax = flag.Duration("retry-interval-max", 5*time.Minute, "Maximum retry interval of failed volume snapshot creation or deletion. Default is 5 minutes.") enableNodeDeployment = flag.Bool("node-deployment", false, "Enables deploying the sidecar controller together with a CSI driver on nodes to manage snapshots for node-local volumes.") - enableVolumeGroupSnapshots = flag.Bool("enable-volume-group-snapshots", false, "Enables the volume group snapshot feature, allowing the user to create snapshots of groups of volumes.") + enableVolumeGroupSnapshots = flag.Bool("enable-volume-group-snapshots", false, "Enables the volume group snapshot feature, allowing the user to create a snapshot of a group of volumes.") groupSnapshotNamePrefix = flag.String("groupsnapshot-name-prefix", "groupsnapshot", "Prefix to apply to the name of a created group snapshot") groupSnapshotNameUUIDLength = flag.Int("groupsnapshot-name-uuid-length", -1, "Length in characters for the generated uuid of a created group snapshot. Defaults behavior is to NOT truncate.") diff --git a/cmd/snapshot-controller/main.go b/cmd/snapshot-controller/main.go index 7443defac..e31b19b87 100644 --- a/cmd/snapshot-controller/main.go +++ b/cmd/snapshot-controller/main.go @@ -72,7 +72,7 @@ var ( retryIntervalMax = flag.Duration("retry-interval-max", 5*time.Minute, "Maximum retry interval of failed volume snapshot creation or deletion. Default is 5 minutes.") enableDistributedSnapshotting = flag.Bool("enable-distributed-snapshotting", false, "Enables each node to handle snapshotting for the local volumes created on that node") preventVolumeModeConversion = flag.Bool("prevent-volume-mode-conversion", false, "Prevents an unauthorised user from modifying the volume mode when creating a PVC from an existing VolumeSnapshot.") - enableVolumeGroupSnapshots = flag.Bool("enable-volume-group-snapshots", false, "Enables the volume group snapshot feature, allowing the user to create snapshots of groups of volumes.") + enableVolumeGroupSnapshots = flag.Bool("enable-volume-group-snapshots", false, "Enables the volume group snapshot feature, allowing the user to create a snapshot of a group of volumes.") retryCRDIntervalMax = flag.Duration("retry-crd-interval-max", 5*time.Second, "Maximum retry interval to wait for CRDs to appear. The default is 5 seconds.") ) diff --git a/pkg/common-controller/groupsnapshot_controller_helper.go b/pkg/common-controller/groupsnapshot_controller_helper.go index d68635e4e..543638eb3 100644 --- a/pkg/common-controller/groupsnapshot_controller_helper.go +++ b/pkg/common-controller/groupsnapshot_controller_helper.go @@ -1153,13 +1153,14 @@ func (ctrl *csiSnapshotCommonController) processGroupSnapshotWithDeletionTimesta } } - klog.V(5).Infof("processGroupSnapshotWithDeletionTimestamp[%s]: Delete individual snapshots part of group snapshot", utils.GroupSnapshotKey(groupSnapshot)) + klog.V(5).Infof("processGroupSnapshotWithDeletionTimestamp[%s]: Delete individual snapshots that are part of the group snapshot", utils.GroupSnapshotKey(groupSnapshot)) // Delete the individual snapshots part of the group snapshot for _, snapshot := range groupSnapshot.Status.VolumeSnapshotRefList { err := ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshot.Namespace).Delete(context.TODO(), snapshot.Name, metav1.DeleteOptions{}) if err != nil { - msg := fmt.Sprintf("failed to delete snapshot API object part of group snapshot %s: %v", utils.GroupSnapshotKey(groupSnapshot), err) + msg := fmt.Sprintf("failed to delete snapshot API object %s/%s part of group snapshot %s: %v", snapshot.Namespace, snapshot.Name, utils.GroupSnapshotKey(groupSnapshot), err) + klog.Error(msg) ctrl.eventRecorder.Event(groupSnapshot, v1.EventTypeWarning, "SnapshotDeleteError", msg) return fmt.Errorf(msg) } diff --git a/pkg/common-controller/snapshot_controller.go b/pkg/common-controller/snapshot_controller.go index d3d1132f5..d509fb296 100644 --- a/pkg/common-controller/snapshot_controller.go +++ b/pkg/common-controller/snapshot_controller.go @@ -290,11 +290,13 @@ func (ctrl *csiSnapshotCommonController) processSnapshotWithDeletionTimestamp(sn if snapshot.Status != nil && snapshot.Status.VolumeGroupSnapshotName != nil { groupSnapshot, err := ctrl.groupSnapshotLister.VolumeGroupSnapshots(snapshot.Namespace).Get(*snapshot.Status.VolumeGroupSnapshotName) if err == nil { - msg := fmt.Sprintf("failed to delete snapshot API %s object as it belongs to group snapshot %s", utils.SnapshotKey(snapshot), utils.GroupSnapshotKey(groupSnapshot)) + msg := fmt.Sprintf("deletion of the individual volume snapshot %s is not allowed as it belongs to group snapshot %s. Deleting the group snapshot will trigger the deletion of all the individual volume snapshots that are part of the group.", utils.SnapshotKey(snapshot), utils.GroupSnapshotKey(groupSnapshot)) + klog.Error(msg) ctrl.eventRecorder.Event(snapshot, v1.EventTypeWarning, "SnapshotDeleteError", msg) return fmt.Errorf(msg) } if !apierrs.IsNotFound(err) { + klog.Errorf("failed to delete snapshot %s: %v", utils.SnapshotKey(snapshot), err) return err } } diff --git a/pkg/sidecar-controller/snapshot_controller.go b/pkg/sidecar-controller/snapshot_controller.go index e80890a36..2954cd34c 100644 --- a/pkg/sidecar-controller/snapshot_controller.go +++ b/pkg/sidecar-controller/snapshot_controller.go @@ -272,7 +272,7 @@ func (ctrl *csiSnapshotSideCarController) checkandUpdateContentStatusOperation(c driverName = content.Spec.Driver snapshotID = *content.Spec.Source.SnapshotHandle - klog.V(5).Infof("checkandUpdateContentStatusOperation: driver %s, snapshotId %s, creationTime %v, size %d, readyToUse %t groupSnapshotID %s", driverName, snapshotID, creationTime, size, readyToUse, groupSnapshotID) + klog.V(5).Infof("checkandUpdateContentStatusOperation: driver %s, snapshotId %s, creationTime %v, size %d, readyToUse %t, groupSnapshotID %s", driverName, snapshotID, creationTime, size, readyToUse, groupSnapshotID) if creationTime.IsZero() { creationTime = time.Now()