controllers: delete nfs driver if nfs is not enabled #500
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rohan47 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| nfsDriver := &csiopv1.Driver{} | ||
| nfsDriver.Name = templates.NfsDriverName | ||
| nfsDriver.Namespace = c.OperatorNamespace |
There was a problem hiding this comment.
We can move this from here and the if condition above, to the outside block in someway like this:
nfsDriver := &csiopv1.Driver{
Name: templates.NfsDriverName,
Namespace: c.OperatorNamespace
}
|
/hold |
| return nil | ||
| } | ||
|
|
||
| func (c *OperatorConfigMapReconciler) hasPVsWithNfsDriver() (bool, error) { |
There was a problem hiding this comment.
I don't see the need to spread the functionality across multiple functions, a single func per type would do and list with matchingfield + limit of 1.
| return nil | ||
| } | ||
|
|
||
| func (c *OperatorConfigMapReconciler) hasPVsWithNfsDriver() (bool, error) { |
There was a problem hiding this comment.
| func (c *OperatorConfigMapReconciler) hasPVsWithNfsDriver() (bool, error) { | |
| func (c *OperatorConfigMapReconciler) hasPersistentVolumesWithNfsDriver() (bool, error) { |
after the first usage your ide would autocomplete and no need to shorten it, in other controllers we aren't shortening it.
| return len(pvList.Items) > 0, nil | ||
| } | ||
|
|
||
| func (c *OperatorConfigMapReconciler) hasSnapshotsWithNfsDriver() (bool, error) { |
There was a problem hiding this comment.
| func (c *OperatorConfigMapReconciler) hasSnapshotsWithNfsDriver() (bool, error) { | |
| func (c *OperatorConfigMapReconciler) hasVolumeSnapshotContentsWithNfsDriver() (bool, error) { |
the existing one is misleading, we are checking for vsc but not for vs.
| return fmt.Errorf("failed to reconcile nfs driver: %v", err) | ||
| } | ||
| } else { | ||
| if hasPVs, err := c.hasPVsWithNfsDriver(); err != nil { |
There was a problem hiding this comment.
| if hasPVs, err := c.hasPVsWithNfsDriver(); err != nil { | |
| if hasPvs, err := c.hasPVsWithNfsDriver(); err != nil { |
| c.log.Info("NFS driver has PVs, skipping deletion") | ||
| return nil | ||
| } | ||
| if hasSnapshots, err := c.hasSnapshotsWithNfsDriver(); err != nil { |
There was a problem hiding this comment.
| if hasSnapshots, err := c.hasSnapshotsWithNfsDriver(); err != nil { | |
| if hasVscs, err := c.hasSnapshotsWithNfsDriver(); err != nil { |
| if hasSnapshots, err := c.hasSnapshotsWithNfsDriver(); err != nil { | ||
| return fmt.Errorf("failed to check if NFS driver has snapshots: %v", err) | ||
| } else if hasSnapshots { | ||
| c.log.Info("NFS driver has snapshots, skipping deletion") |
There was a problem hiding this comment.
| c.log.Info("NFS driver has snapshots, skipping deletion") | |
| c.log.Info("NFS driver has volumesnapshotcontents, skipping deletion") |
| return nil | ||
| } | ||
| if hasSnapshots, err := c.hasSnapshotsWithNfsDriver(); err != nil { | ||
| return fmt.Errorf("failed to check if NFS driver has snapshots: %v", err) |
There was a problem hiding this comment.
| return fmt.Errorf("failed to check if NFS driver has snapshots: %v", err) | |
| return fmt.Errorf("failed to check if NFS driver has volumesnapshotcontents: %v", err) |
Signed-off-by: Rohan Gupta <[email protected]>
| if hasPvs, err := c.hasPersistentVolumesWithNfsDriver(); err != nil { | ||
| return fmt.Errorf("failed to check if NFS driver has PVs: %v", err) | ||
| } else if hasPvs { | ||
| c.log.Info("NFS driver has PVs, skipping deletion") |
There was a problem hiding this comment.
Do we really need to log this info?
| return fmt.Errorf("failed to check if NFS driver has volumesnapshotcontents: %v", err) | ||
| } else if hasVscs { | ||
| c.log.Info("NFS driver has volumesnapshotcontents, skipping deletion") | ||
| return nil |
4 participants
delete nfs driver if nfs is not enabled and nfs driver exists, also there are no PVCs using nfs storageclass
DFBUGS-4704