Skip to content

Commit 8cb002a

Browse files
author
Neumoin, Konstantin
committed
fix: new logic VS/VCS
1 parent 01fbc27 commit 8cb002a

File tree

4 files changed

+302
-8
lines changed

4 files changed

+302
-8
lines changed

images/snapshot-controller/pkg/common-controller/snapshot_controller.go

Lines changed: 194 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,33 @@ func (ctrl *csiSnapshotCommonController) syncUnreadySnapshot(snapshot *crdv1.Vol
563563
}
564564

565565
// snapshot.Spec.Source.VolumeSnapshotContentName == nil - dynamically creating snapshot
566+
// CRITICAL: Check for Deckhouse-managed BEFORE attempting to create VSC
567+
// This ensures VSC is created by Deckhouse controller, not snapshot-controller
568+
_, deckhouseManaged := snapshot.Annotations[utils.AnnDeckhouseManaged]
569+
if deckhouseManaged {
570+
klog.V(5).Infof("syncUnreadySnapshot[%s]: VolumeSnapshot is managed by Deckhouse, waiting for VSC to be created externally", uniqueSnapshotName)
571+
// For Deckhouse-managed VS, we wait for VSC to be created by Deckhouse controller
572+
// Check if VSC exists with matching labels/annotations
573+
contentObj, err := ctrl.getDynamicallyProvisionedContentFromStore(snapshot)
574+
if err != nil {
575+
return err
576+
}
577+
if contentObj == nil {
578+
// VSC not created yet, wait
579+
return fmt.Errorf("Deckhouse-managed VolumeSnapshot %s waiting for VolumeSnapshotContent to be created", uniqueSnapshotName)
580+
}
581+
// VSC exists, bind and update status
582+
// Note: For Deckhouse-managed VSC, snapshotHandle may be nil - this is OK
583+
newSnapshot, err := ctrl.bindandUpdateVolumeSnapshot(contentObj, snapshot)
584+
if err != nil {
585+
klog.V(4).Infof("bindandUpdateVolumeSnapshot[%s]: failed to bind content [%s] to snapshot %v", uniqueSnapshotName, contentObj.Name, err)
586+
return err
587+
}
588+
klog.V(5).Infof("bindandUpdateVolumeSnapshot %v", newSnapshot)
589+
return nil
590+
}
591+
592+
// For non-Deckhouse snapshots, proceed with normal dynamic provisioning
566593
klog.V(5).Infof("getDynamicallyProvisionedContentFromStore for snapshot %s", uniqueSnapshotName)
567594
contentObj, err := ctrl.getDynamicallyProvisionedContentFromStore(snapshot)
568595
if err != nil {
@@ -572,10 +599,14 @@ func (ctrl *csiSnapshotCommonController) syncUnreadySnapshot(snapshot *crdv1.Vol
572599

573600
if contentObj != nil {
574601
klog.V(5).Infof("Found VolumeSnapshotContent object %s for snapshot %s", contentObj.Name, uniqueSnapshotName)
575-
if contentObj.Spec.Source.SnapshotHandle != nil {
602+
// Check if VSC is Deckhouse-managed - skip snapshotHandle validation
603+
_, deckhouseManagedVSC := contentObj.Annotations[utils.AnnDeckhouseManaged]
604+
if !deckhouseManagedVSC && contentObj.Spec.Source.SnapshotHandle != nil {
605+
// For non-Deckhouse snapshots, snapshotHandle should not be set for dynamic provisioning
576606
ctrl.updateSnapshotErrorStatusWithEvent(snapshot, true, v1.EventTypeWarning, "SnapshotHandleSet", fmt.Sprintf("Snapshot handle should not be set in content %s for dynamic provisioning", uniqueSnapshotName))
577607
return fmt.Errorf("snapshotHandle should not be set in the content for dynamic provisioning for snapshot %s", uniqueSnapshotName)
578608
}
609+
// For Deckhouse-managed VSC, snapshotHandle is nil - this is expected and OK
579610
newSnapshot, err := ctrl.bindandUpdateVolumeSnapshot(contentObj, snapshot)
580611
if err != nil {
581612
klog.V(4).Infof("bindandUpdateVolumeSnapshot[%s]: failed to bind content [%s] to snapshot %v", uniqueSnapshotName, contentObj.Name, err)
@@ -586,6 +617,7 @@ func (ctrl *csiSnapshotCommonController) syncUnreadySnapshot(snapshot *crdv1.Vol
586617
}
587618

588619
// If we reach here, it is a dynamically provisioned snapshot, and the volumeSnapshotContent object is not yet created.
620+
// Create VSC for non-Deckhouse snapshots only
589621
if snapshot.Spec.Source.PersistentVolumeClaimName == nil {
590622
ctrl.updateSnapshotErrorStatusWithEvent(snapshot, true, v1.EventTypeWarning, "SnapshotPVCSourceMissing", fmt.Sprintf("PVC source for snapshot %s is missing", uniqueSnapshotName))
591623
return fmt.Errorf("expected PVC source for snapshot %s but got nil", uniqueSnapshotName)
@@ -666,6 +698,12 @@ func (ctrl *csiSnapshotCommonController) getPreprovisionedContentFromStore(snaps
666698
// A content is considered to be a pre-provisioned one if its Spec.Source.SnapshotHandle
667699
// is not nil, or a dynamically provisioned one if its Spec.Source.VolumeHandle is not nil.
668700
func (ctrl *csiSnapshotCommonController) getDynamicallyProvisionedContentFromStore(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshotContent, error) {
701+
// For Deckhouse-managed snapshots, search by annotations/labels instead of fixed name
702+
_, deckhouseManaged := snapshot.Annotations[utils.AnnDeckhouseManaged]
703+
if deckhouseManaged {
704+
return ctrl.findDeckhouseContentForSnapshot(snapshot)
705+
}
706+
669707
contentName := utils.GetDynamicSnapshotContentNameForSnapshot(snapshot)
670708
content, err := ctrl.getContentFromStore(contentName)
671709
if err != nil {
@@ -735,6 +773,13 @@ func (ctrl *csiSnapshotCommonController) createSnapshotContent(snapshot *crdv1.V
735773
return nil, fmt.Errorf("failed to get input parameters to create snapshot %s: %q", snapshot.Name, err)
736774
}
737775

776+
// CRITICAL: Double-check that this is not Deckhouse-managed
777+
// Even though getCreateSnapshotInput should return nil class, ensure we don't create VSC
778+
_, deckhouseManaged := snapshot.Annotations[utils.AnnDeckhouseManaged]
779+
if deckhouseManaged {
780+
return nil, fmt.Errorf("createSnapshotContent called for Deckhouse-managed snapshot %s - this should not happen", utils.SnapshotKey(snapshot))
781+
}
782+
738783
// Create VolumeSnapshotContent in the database
739784
if volume.Spec.CSI == nil {
740785
return nil, fmt.Errorf("cannot find CSI PersistentVolumeSource for volume %s", volume.Name)
@@ -753,11 +798,20 @@ func (ctrl *csiSnapshotCommonController) createSnapshotContent(snapshot *crdv1.V
753798
Source: crdv1.VolumeSnapshotContentSource{
754799
VolumeHandle: &volume.Spec.CSI.VolumeHandle,
755800
},
756-
VolumeSnapshotClassName: &(class.Name),
757-
DeletionPolicy: class.DeletionPolicy,
758-
Driver: class.Driver,
759801
},
760802
}
803+
804+
// Set SnapshotClass fields only if class is not nil (not Deckhouse-managed)
805+
// CRITICAL: If class is nil, do NOT set default - this prevents fallback
806+
if class != nil {
807+
snapshotContent.Spec.VolumeSnapshotClassName = &(class.Name)
808+
snapshotContent.Spec.DeletionPolicy = class.DeletionPolicy
809+
snapshotContent.Spec.Driver = class.Driver
810+
} else {
811+
// class is nil - this should not happen for non-Deckhouse snapshots
812+
// but if it does, fail explicitly rather than using default
813+
return nil, fmt.Errorf("createSnapshotContent: no SnapshotClass available for snapshot %s", utils.SnapshotKey(snapshot))
814+
}
761815

762816
if ctrl.enableDistributedSnapshotting {
763817
nodeName, err := ctrl.getManagedByNode(volume)
@@ -824,6 +878,26 @@ func (ctrl *csiSnapshotCommonController) createSnapshotContent(snapshot *crdv1.V
824878
func (ctrl *csiSnapshotCommonController) getCreateSnapshotInput(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshotClass, *v1.PersistentVolume, string, *v1.SecretReference, error) {
825879
className := snapshot.Spec.VolumeSnapshotClassName
826880
klog.V(5).Infof("getCreateSnapshotInput [%s]", snapshot.Name)
881+
882+
// NOTE: This Deckhouse-managed check is defensive programming.
883+
// In practice, Deckhouse-managed snapshots are handled earlier in syncUnreadySnapshot(),
884+
// so createSnapshotContent() should never be called for them. This check serves as
885+
// a safety net in case createSnapshotContent() is called from elsewhere.
886+
_, deckhouseManaged := snapshot.Annotations[utils.AnnDeckhouseManaged]
887+
if deckhouseManaged {
888+
klog.V(5).Infof("getCreateSnapshotInput [%s]: Deckhouse-managed, skipping SnapshotClass", snapshot.Name)
889+
// For Deckhouse-managed snapshots, we don't need SnapshotClass
890+
// Return nil class, but we still need volume for content creation
891+
volume, err := ctrl.getVolumeFromVolumeSnapshot(snapshot)
892+
if err != nil {
893+
klog.Errorf("getCreateSnapshotInput failed to get PersistentVolume object [%s]: Error: [%#v]", snapshot.Name, err)
894+
return nil, nil, "", nil, err
895+
}
896+
contentName := utils.GetDynamicSnapshotContentNameForSnapshot(snapshot)
897+
// Return nil class for Deckhouse-managed snapshots
898+
return nil, volume, contentName, nil, nil
899+
}
900+
827901
var class *crdv1.VolumeSnapshotClass
828902
var err error
829903
if className != nil {
@@ -1141,6 +1215,19 @@ func (ctrl *csiSnapshotCommonController) checkandBindSnapshotContent(snapshot *c
11411215
// This routine sets snapshot.Spec.Source.VolumeSnapshotContentName
11421216
func (ctrl *csiSnapshotCommonController) bindandUpdateVolumeSnapshot(snapshotContent *crdv1.VolumeSnapshotContent, snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshot, error) {
11431217
klog.V(5).Infof("bindandUpdateVolumeSnapshot for snapshot [%s]: snapshotContent [%s]", snapshot.Name, snapshotContent.Name)
1218+
1219+
// CRITICAL: For Deckhouse-managed VSC, snapshotHandle is nil - this is expected
1220+
// Never treat missing snapshotHandle as error for Deckhouse-managed objects
1221+
_, deckhouseManagedVSC := snapshotContent.Annotations[utils.AnnDeckhouseManaged]
1222+
if deckhouseManagedVSC {
1223+
// Ensure snapshotHandle is nil (it should be already, but make it explicit)
1224+
if snapshotContent.Status != nil && snapshotContent.Status.SnapshotHandle != nil {
1225+
// This shouldn't happen, but if it does, clear it
1226+
klog.V(4).Infof("bindandUpdateVolumeSnapshot[%s]: Deckhouse-managed VSC has snapshotHandle, clearing it", snapshot.Name)
1227+
snapshotContent.Status.SnapshotHandle = nil
1228+
}
1229+
}
1230+
11441231
snapshotObj, err := ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshot.Namespace).Get(context.TODO(), snapshot.Name, metav1.GetOptions{})
11451232
if err != nil {
11461233
return nil, fmt.Errorf("error get snapshot %s from api server: %v", utils.SnapshotKey(snapshot), err)
@@ -1222,6 +1309,12 @@ func (ctrl *csiSnapshotCommonController) updateSnapshotStatus(snapshot *crdv1.Vo
12221309
if content.Status != nil && content.Status.ReadyToUse != nil {
12231310
readyToUse = *content.Status.ReadyToUse
12241311
}
1312+
// For Deckhouse-managed VSC, ReadyToUse should be true even without snapshotHandle
1313+
// This ensures proper mirroring from VSC to VS
1314+
_, deckhouseManagedVSC := content.Annotations[utils.AnnDeckhouseManaged]
1315+
if deckhouseManagedVSC && content.Status != nil && content.Status.ReadyToUse != nil && *content.Status.ReadyToUse {
1316+
readyToUse = true
1317+
}
12251318
var volumeSnapshotErr *crdv1.VolumeSnapshotError
12261319
if content.Status != nil && content.Status.Error != nil {
12271320
volumeSnapshotErr = content.Status.Error.DeepCopy()
@@ -1254,6 +1347,20 @@ func (ctrl *csiSnapshotCommonController) updateSnapshotStatus(snapshot *crdv1.Vo
12541347
if err != nil {
12551348
return nil, fmt.Errorf("error get snapshot %s from api server: %v", utils.SnapshotKey(snapshot), err)
12561349
}
1350+
1351+
// Add Deckhouse proxy label if VSC is Deckhouse-managed
1352+
// deckhouseManagedVSC already declared above, reuse it
1353+
labelsUpdated := false
1354+
if deckhouseManagedVSC {
1355+
if snapshotObj.Labels == nil {
1356+
snapshotObj.Labels = make(map[string]string)
1357+
labelsUpdated = true
1358+
}
1359+
if snapshotObj.Labels[utils.LabelDeckhouseProxy] != "true" {
1360+
snapshotObj.Labels[utils.LabelDeckhouseProxy] = "true"
1361+
labelsUpdated = true
1362+
}
1363+
}
12571364

12581365
var newStatus *crdv1.VolumeSnapshotStatus
12591366
updated := false
@@ -1306,9 +1413,13 @@ func (ctrl *csiSnapshotCommonController) updateSnapshotStatus(snapshot *crdv1.Vo
13061413
}
13071414
}
13081415

1309-
if updated {
1416+
if updated || labelsUpdated {
13101417
snapshotClone := snapshotObj.DeepCopy()
13111418
snapshotClone.Status = newStatus
1419+
// Update labels if needed
1420+
if labelsUpdated {
1421+
snapshotClone.Labels = snapshotObj.Labels
1422+
}
13121423

13131424
// We need to record metrics before updating the status due to a bug causing cache entries after a failed UpdateStatus call.
13141425
// Must meet the following criteria to emit a successful CreateSnapshot status
@@ -1332,7 +1443,13 @@ func (ctrl *csiSnapshotCommonController) updateSnapshotStatus(snapshot *crdv1.Vo
13321443
ctrl.eventRecorder.Event(snapshot, v1.EventTypeNormal, "SnapshotReady", msg)
13331444
}
13341445

1335-
newSnapshotObj, err := ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshotClone.Namespace).UpdateStatus(context.TODO(), snapshotClone, metav1.UpdateOptions{})
1446+
// If labels need to be updated, use Update instead of UpdateStatus
1447+
var newSnapshotObj *crdv1.VolumeSnapshot
1448+
if labelsUpdated {
1449+
newSnapshotObj, err = ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshotClone.Namespace).Update(context.TODO(), snapshotClone, metav1.UpdateOptions{})
1450+
} else {
1451+
newSnapshotObj, err = ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshotClone.Namespace).UpdateStatus(context.TODO(), snapshotClone, metav1.UpdateOptions{})
1452+
}
13361453
if err != nil {
13371454
return nil, newControllerUpdateError(utils.SnapshotKey(snapshot), err.Error())
13381455
}
@@ -1343,6 +1460,77 @@ func (ctrl *csiSnapshotCommonController) updateSnapshotStatus(snapshot *crdv1.Vo
13431460
return snapshotObj, nil
13441461
}
13451462

1463+
// findDeckhouseContentForSnapshot finds VolumeSnapshotContent for Deckhouse-managed VolumeSnapshot
1464+
// by searching through annotations, labels, or owner references
1465+
func (ctrl *csiSnapshotCommonController) findDeckhouseContentForSnapshot(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshotContent, error) {
1466+
klog.V(5).Infof("findDeckhouseContentForSnapshot[%s]: searching for Deckhouse-managed VSC", utils.SnapshotKey(snapshot))
1467+
1468+
// Get all VolumeSnapshotContents
1469+
allContents, err := ctrl.contentLister.List(labels.Everything())
1470+
if err != nil {
1471+
return nil, fmt.Errorf("failed to list VolumeSnapshotContents: %v", err)
1472+
}
1473+
1474+
// Search by annotation AnnDeckhouseSourceSnapshotContent (if set on VS)
1475+
if sfcName, ok := snapshot.Annotations[utils.AnnDeckhouseSourceSnapshotContent]; ok {
1476+
for _, content := range allContents {
1477+
if content.Annotations != nil {
1478+
if contentSFCName, ok := content.Annotations[utils.AnnDeckhouseSourceSnapshotContent]; ok && contentSFCName == sfcName {
1479+
// Check if content is Deckhouse-managed
1480+
if _, managed := content.Annotations[utils.AnnDeckhouseManaged]; managed {
1481+
// Verify it points to this snapshot
1482+
ref := content.Spec.VolumeSnapshotRef
1483+
if ref.Name == snapshot.Name && ref.Namespace == snapshot.Namespace {
1484+
klog.V(5).Infof("findDeckhouseContentForSnapshot[%s]: found VSC %s by SFC annotation", utils.SnapshotKey(snapshot), content.Name)
1485+
return content, nil
1486+
}
1487+
}
1488+
}
1489+
}
1490+
}
1491+
}
1492+
1493+
// Search by PVC annotation/label
1494+
if snapshot.Spec.Source.PersistentVolumeClaimName != nil {
1495+
pvcName := *snapshot.Spec.Source.PersistentVolumeClaimName
1496+
for _, content := range allContents {
1497+
if content.Annotations != nil {
1498+
if contentPVCName, ok := content.Annotations[utils.AnnDeckhouseSourcePVC]; ok && contentPVCName == pvcName {
1499+
// Check if content is Deckhouse-managed
1500+
if _, managed := content.Annotations[utils.AnnDeckhouseManaged]; managed {
1501+
// Verify it points to this snapshot
1502+
ref := content.Spec.VolumeSnapshotRef
1503+
if ref.Name == snapshot.Name && ref.Namespace == snapshot.Namespace {
1504+
klog.V(5).Infof("findDeckhouseContentForSnapshot[%s]: found VSC %s by PVC annotation", utils.SnapshotKey(snapshot), content.Name)
1505+
return content, nil
1506+
}
1507+
}
1508+
}
1509+
}
1510+
}
1511+
}
1512+
1513+
// Search by owner reference (if VS has ownerRef to VSC)
1514+
for _, ownerRef := range snapshot.OwnerReferences {
1515+
if ownerRef.Kind == "VolumeSnapshotContent" {
1516+
content, err := ctrl.getContentFromStore(ownerRef.Name)
1517+
if err == nil && content != nil {
1518+
// Verify it's Deckhouse-managed and points to this snapshot
1519+
if _, managed := content.Annotations[utils.AnnDeckhouseManaged]; managed {
1520+
ref := content.Spec.VolumeSnapshotRef
1521+
if ref.Name == snapshot.Name && ref.Namespace == snapshot.Namespace {
1522+
klog.V(5).Infof("findDeckhouseContentForSnapshot[%s]: found VSC %s by owner reference", utils.SnapshotKey(snapshot), content.Name)
1523+
return content, nil
1524+
}
1525+
}
1526+
}
1527+
}
1528+
}
1529+
1530+
klog.V(5).Infof("findDeckhouseContentForSnapshot[%s]: no matching Deckhouse-managed VSC found", utils.SnapshotKey(snapshot))
1531+
return nil, nil
1532+
}
1533+
13461534
func (ctrl *csiSnapshotCommonController) getVolumeFromVolumeSnapshot(snapshot *crdv1.VolumeSnapshot) (*v1.PersistentVolume, error) {
13471535
pvc, err := ctrl.getClaimFromVolumeSnapshot(snapshot)
13481536
if err != nil {

images/snapshot-controller/pkg/common-controller/snapshot_controller_base.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,34 @@ func (ctrl *csiSnapshotCommonController) syncContentByKey(key string) error {
497497
// On error, it must return the original snapshot, not nil, because the caller syncContentByKey
498498
// needs to check snapshot's timestamp.
499499
func (ctrl *csiSnapshotCommonController) checkAndUpdateSnapshotClass(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshot, error) {
500+
// CRITICAL: Skip SnapshotClass resolution for Deckhouse-managed snapshots
501+
// Deckhouse creates VSC directly, no SnapshotClass needed
502+
// This prevents fallback to default SnapshotClass
503+
_, deckhouseManaged := snapshot.Annotations[utils.AnnDeckhouseManaged]
504+
if deckhouseManaged {
505+
klog.V(5).Infof("checkAndUpdateSnapshotClass [%s]: Deckhouse-managed, skipping SnapshotClass resolution", snapshot.Name)
506+
// Ensure VolumeSnapshotClassName is nil to prevent any fallback
507+
if snapshot.Spec.VolumeSnapshotClassName != nil {
508+
snapshotClone := snapshot.DeepCopy()
509+
snapshotClone.Spec.VolumeSnapshotClassName = nil
510+
patches := []utils.PatchOp{
511+
{
512+
Op: "replace",
513+
Path: "/spec/volumeSnapshotClassName",
514+
Value: nil,
515+
},
516+
}
517+
newSnapshot, err := utils.PatchVolumeSnapshot(snapshotClone, patches, ctrl.clientset)
518+
if err != nil {
519+
klog.V(4).Infof("checkAndUpdateSnapshotClass [%s]: failed to clear VolumeSnapshotClassName: %v", snapshot.Name, err)
520+
// Don't fail, just return original snapshot
521+
return snapshot, nil
522+
}
523+
return newSnapshot, nil
524+
}
525+
return snapshot, nil
526+
}
527+
500528
className := snapshot.Spec.VolumeSnapshotClassName
501529
var class *crdv1.VolumeSnapshotClass
502530
var err error

0 commit comments

Comments
 (0)