Skip to content

Commit 82c07b0

Browse files
authored
[Back port] Fix concurrent publish/unpublish to handle subordinate volumes and RO…
1 parent 23dce89 commit 82c07b0

5 files changed

Lines changed: 1571 additions & 59 deletions

File tree

core/concurrent_cache/volume.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,26 @@ func ListReadOnlyCloneVolumes() Subquery {
105105
}
106106
}
107107

108+
// ListReadOnlyCloneVolumesForSources returns all read-only clone volumes whose
109+
// CloneSourceVolume matches any of the provided source volume names.
110+
func ListReadOnlyCloneVolumesForSources(sourceVolumeNames ...string) Subquery {
111+
sourceSet := make(map[string]struct{}, len(sourceVolumeNames))
112+
for _, name := range sourceVolumeNames {
113+
sourceSet[name] = struct{}{}
114+
}
115+
return Subquery{
116+
res: volume,
117+
op: list,
118+
setResults: listVolumesSetResults(func(v *storage.Volume) bool {
119+
if !v.Config.ReadOnlyClone || v.Config.CloneSourceVolume == "" {
120+
return false
121+
}
122+
_, match := sourceSet[v.Config.CloneSourceVolume]
123+
return match
124+
}),
125+
}
126+
}
127+
108128
func ListVolumesByInternalName(internalVolName string) Subquery {
109129
return Subquery{
110130
res: volume,

core/concurrent_cache/volume_test.go

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1679,3 +1679,187 @@ func TestListVolumesForPolicyReevaluation(t *testing.T) {
16791679
})
16801680
}
16811681
}
1682+
1683+
func TestListReadOnlyCloneVolumesForSources(t *testing.T) {
1684+
tests := []struct {
1685+
name string
1686+
volumes map[string]*storage.Volume
1687+
sources []string
1688+
expected int
1689+
expectedVol []string
1690+
}{
1691+
{
1692+
name: "no volumes in cache",
1693+
volumes: map[string]*storage.Volume{},
1694+
sources: []string{"sourceVol"},
1695+
expected: 0,
1696+
},
1697+
{
1698+
name: "no matching sources",
1699+
volumes: map[string]*storage.Volume{
1700+
"clone1": {
1701+
Config: &storage.VolumeConfig{
1702+
Name: "clone1",
1703+
ReadOnlyClone: true,
1704+
CloneSourceVolume: "otherSource",
1705+
},
1706+
},
1707+
},
1708+
sources: []string{"sourceVol"},
1709+
expected: 0,
1710+
},
1711+
{
1712+
name: "single matching source",
1713+
volumes: map[string]*storage.Volume{
1714+
"clone1": {
1715+
Config: &storage.VolumeConfig{
1716+
Name: "clone1",
1717+
ReadOnlyClone: true,
1718+
CloneSourceVolume: "sourceVol",
1719+
},
1720+
},
1721+
"normalVol": {
1722+
Config: &storage.VolumeConfig{
1723+
Name: "normalVol",
1724+
},
1725+
},
1726+
},
1727+
sources: []string{"sourceVol"},
1728+
expected: 1,
1729+
expectedVol: []string{"clone1"},
1730+
},
1731+
{
1732+
name: "multiple matching sources",
1733+
volumes: map[string]*storage.Volume{
1734+
"cloneA": {
1735+
Config: &storage.VolumeConfig{
1736+
Name: "cloneA",
1737+
ReadOnlyClone: true,
1738+
CloneSourceVolume: "source1",
1739+
},
1740+
},
1741+
"cloneB": {
1742+
Config: &storage.VolumeConfig{
1743+
Name: "cloneB",
1744+
ReadOnlyClone: true,
1745+
CloneSourceVolume: "source2",
1746+
},
1747+
},
1748+
"cloneC": {
1749+
Config: &storage.VolumeConfig{
1750+
Name: "cloneC",
1751+
ReadOnlyClone: true,
1752+
CloneSourceVolume: "source3",
1753+
},
1754+
},
1755+
},
1756+
sources: []string{"source1", "source2"},
1757+
expected: 2,
1758+
expectedVol: []string{"cloneA", "cloneB"},
1759+
},
1760+
{
1761+
name: "non-RO clone volumes ignored even with CloneSourceVolume set",
1762+
volumes: map[string]*storage.Volume{
1763+
"notClone": {
1764+
Config: &storage.VolumeConfig{
1765+
Name: "notClone",
1766+
ReadOnlyClone: false,
1767+
CloneSourceVolume: "sourceVol",
1768+
},
1769+
},
1770+
},
1771+
sources: []string{"sourceVol"},
1772+
expected: 0,
1773+
},
1774+
{
1775+
name: "RO clone with empty CloneSourceVolume ignored",
1776+
volumes: map[string]*storage.Volume{
1777+
"emptySource": {
1778+
Config: &storage.VolumeConfig{
1779+
Name: "emptySource",
1780+
ReadOnlyClone: true,
1781+
CloneSourceVolume: "",
1782+
},
1783+
},
1784+
},
1785+
sources: []string{"sourceVol"},
1786+
expected: 0,
1787+
},
1788+
{
1789+
name: "mixed volumes",
1790+
volumes: map[string]*storage.Volume{
1791+
"matchingClone": {
1792+
Config: &storage.VolumeConfig{
1793+
Name: "matchingClone",
1794+
ReadOnlyClone: true,
1795+
CloneSourceVolume: "sourceVol",
1796+
},
1797+
},
1798+
"nonMatchingClone": {
1799+
Config: &storage.VolumeConfig{
1800+
Name: "nonMatchingClone",
1801+
ReadOnlyClone: true,
1802+
CloneSourceVolume: "otherSource",
1803+
},
1804+
},
1805+
"normalVol": {
1806+
Config: &storage.VolumeConfig{
1807+
Name: "normalVol",
1808+
},
1809+
},
1810+
"emptySourceClone": {
1811+
Config: &storage.VolumeConfig{
1812+
Name: "emptySourceClone",
1813+
ReadOnlyClone: true,
1814+
CloneSourceVolume: "",
1815+
},
1816+
},
1817+
"falseClone": {
1818+
Config: &storage.VolumeConfig{
1819+
Name: "falseClone",
1820+
ReadOnlyClone: false,
1821+
CloneSourceVolume: "sourceVol",
1822+
},
1823+
},
1824+
},
1825+
sources: []string{"sourceVol"},
1826+
expected: 1,
1827+
expectedVol: []string{"matchingClone"},
1828+
},
1829+
}
1830+
1831+
for _, tt := range tests {
1832+
t.Run(tt.name, func(t *testing.T) {
1833+
volumes.lock()
1834+
volumes.data = make(map[string]SmartCopier)
1835+
for k, v := range tt.volumes {
1836+
volumes.data[k] = v
1837+
}
1838+
volumes.unlock()
1839+
1840+
subquery := ListReadOnlyCloneVolumesForSources(tt.sources...)
1841+
result := &Result{}
1842+
err := subquery.setResults(&subquery, result)
1843+
assert.NoError(t, err)
1844+
1845+
assert.Len(t, result.Volumes, tt.expected)
1846+
if len(tt.expectedVol) > 0 {
1847+
volNames := make([]string, len(result.Volumes))
1848+
for i, v := range result.Volumes {
1849+
volNames[i] = v.Config.Name
1850+
}
1851+
for _, expected := range tt.expectedVol {
1852+
assert.Contains(t, volNames, expected)
1853+
}
1854+
}
1855+
for _, v := range result.Volumes {
1856+
assert.True(t, v.Config.ReadOnlyClone)
1857+
assert.NotEmpty(t, v.Config.CloneSourceVolume)
1858+
}
1859+
1860+
volumes.lock()
1861+
volumes.data = make(map[string]SmartCopier)
1862+
volumes.unlock()
1863+
})
1864+
}
1865+
}

0 commit comments

Comments
 (0)