Skip to content

Commit a2b16d7

Browse files
committed
cri: fix update of pinned label for images
Signed-off-by: Iceber Gu <[email protected]> (cherry picked from commit 2e014fa) Signed-off-by: Iceber Gu <[email protected]>
1 parent 8dc8618 commit a2b16d7

2 files changed

Lines changed: 171 additions & 8 deletions

File tree

pkg/cri/store/image/image.go

Lines changed: 100 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/containerd/containerd/pkg/cri/labels"
2727
"github.com/containerd/containerd/pkg/cri/util"
2828
"github.com/containerd/containerd/reference/docker"
29+
"k8s.io/apimachinery/pkg/util/sets"
2930

3031
imagedigest "github.com/opencontainers/go-digest"
3132
"github.com/opencontainers/go-digest/digestset"
@@ -67,8 +68,9 @@ func NewStore(client *containerd.Client) *Store {
6768
refCache: make(map[string]string),
6869
client: client,
6970
store: &store{
70-
images: make(map[string]Image),
71-
digestSet: digestset.NewSet(),
71+
images: make(map[string]Image),
72+
digestSet: digestset.NewSet(),
73+
pinnedRefs: make(map[string]sets.Set[string]),
7274
},
7375
}
7476
}
@@ -106,7 +108,13 @@ func (s *Store) update(ref string, img *Image) error {
106108
}
107109
if oldExist {
108110
if oldID == img.ID {
109-
return nil
111+
if s.store.isPinned(img.ID, ref) == img.Pinned {
112+
return nil
113+
}
114+
if img.Pinned {
115+
return s.store.pin(img.ID, ref)
116+
}
117+
return s.store.unpin(img.ID, ref)
110118
}
111119
// Updated. Remove tag from old image.
112120
s.store.delete(oldID, ref)
@@ -178,9 +186,10 @@ func (s *Store) List() []Image {
178186
}
179187

180188
type store struct {
181-
lock sync.RWMutex
182-
images map[string]Image
183-
digestSet *digestset.Set
189+
lock sync.RWMutex
190+
images map[string]Image
191+
digestSet *digestset.Set
192+
pinnedRefs map[string]sets.Set[string]
184193
}
185194

186195
func (s *store) list() []Image {
@@ -205,6 +214,14 @@ func (s *store) add(img Image) error {
205214
}
206215
}
207216

217+
if img.Pinned {
218+
if refs := s.pinnedRefs[img.ID]; refs == nil {
219+
s.pinnedRefs[img.ID] = sets.New(img.References...)
220+
} else {
221+
refs.Insert(img.References...)
222+
}
223+
}
224+
208225
i, ok := s.images[img.ID]
209226
if !ok {
210227
// If the image doesn't exist, add it.
@@ -218,6 +235,73 @@ func (s *store) add(img Image) error {
218235
return nil
219236
}
220237

238+
func (s *store) isPinned(id, ref string) bool {
239+
s.lock.RLock()
240+
defer s.lock.RUnlock()
241+
digest, err := s.digestSet.Lookup(id)
242+
if err != nil {
243+
return false
244+
}
245+
refs := s.pinnedRefs[digest.String()]
246+
return refs != nil && refs.Has(ref)
247+
}
248+
249+
func (s *store) pin(id, ref string) error {
250+
s.lock.Lock()
251+
defer s.lock.Unlock()
252+
digest, err := s.digestSet.Lookup(id)
253+
if err != nil {
254+
if err == digestset.ErrDigestNotFound {
255+
err = errdefs.ErrNotFound
256+
}
257+
return err
258+
}
259+
i, ok := s.images[digest.String()]
260+
if !ok {
261+
return errdefs.ErrNotFound
262+
}
263+
264+
if refs := s.pinnedRefs[digest.String()]; refs == nil {
265+
s.pinnedRefs[digest.String()] = sets.New(ref)
266+
} else {
267+
refs.Insert(ref)
268+
}
269+
i.Pinned = true
270+
s.images[digest.String()] = i
271+
return nil
272+
}
273+
274+
func (s *store) unpin(id, ref string) error {
275+
s.lock.Lock()
276+
defer s.lock.Unlock()
277+
digest, err := s.digestSet.Lookup(id)
278+
if err != nil {
279+
if err == digestset.ErrDigestNotFound {
280+
err = errdefs.ErrNotFound
281+
}
282+
return err
283+
}
284+
i, ok := s.images[digest.String()]
285+
if !ok {
286+
return errdefs.ErrNotFound
287+
}
288+
289+
refs := s.pinnedRefs[digest.String()]
290+
if refs == nil {
291+
return nil
292+
}
293+
if refs.Delete(ref); len(refs) > 0 {
294+
return nil
295+
}
296+
297+
// delete unpinned image, we only need to keep the pinned
298+
// entries in the map
299+
delete(s.pinnedRefs, digest.String())
300+
i.Pinned = false
301+
s.images[digest.String()] = i
302+
return nil
303+
}
304+
221305
func (s *store) get(id string) (Image, error) {
222306
s.lock.RLock()
223307
defer s.lock.RUnlock()
@@ -249,10 +333,20 @@ func (s *store) delete(id, ref string) {
249333
}
250334
i.References = util.SubtractStringSlice(i.References, ref)
251335
if len(i.References) != 0 {
336+
if refs := s.pinnedRefs[digest.String()]; refs != nil {
337+
if refs.Delete(ref); len(refs) == 0 {
338+
i.Pinned = false
339+
// delete unpinned image, we only need to keep the pinned
340+
// entries in the map
341+
delete(s.pinnedRefs, digest.String())
342+
}
343+
}
344+
252345
s.images[digest.String()] = i
253346
return
254347
}
255348
// Remove the image if it is not referenced any more.
256349
s.digestSet.Remove(digest)
257350
delete(s.images, digest.String())
351+
delete(s.pinnedRefs, digest.String())
258352
}

pkg/cri/store/image/image_test.go

Lines changed: 71 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"testing"
2323

2424
"github.com/containerd/containerd/errdefs"
25+
"k8s.io/apimachinery/pkg/util/sets"
2526

2627
"github.com/opencontainers/go-digest/digestset"
2728
assertlib "github.com/stretchr/testify/assert"
@@ -58,8 +59,9 @@ func TestInternalStore(t *testing.T) {
5859
genTruncIndex := func(normalName string) string { return normalName[:(len(normalName)+1)/2] }
5960

6061
s := &store{
61-
images: make(map[string]Image),
62-
digestSet: digestset.NewSet(),
62+
images: make(map[string]Image),
63+
digestSet: digestset.NewSet(),
64+
pinnedRefs: make(map[string]sets.Set[string]),
6365
}
6466

6567
t.Logf("should be able to add image")
@@ -137,6 +139,73 @@ func TestInternalStore(t *testing.T) {
137139
}
138140
}
139141

142+
func TestInternalStorePinnedImage(t *testing.T) {
143+
assert := assertlib.New(t)
144+
s := &store{
145+
images: make(map[string]Image),
146+
digestSet: digestset.NewSet(),
147+
pinnedRefs: make(map[string]sets.Set[string]),
148+
}
149+
150+
ref1 := "containerd.io/ref-1"
151+
image := Image{
152+
ID: "sha256:1123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
153+
ChainID: "test-chain-id-1",
154+
References: []string{ref1},
155+
Size: 10,
156+
}
157+
158+
t.Logf("add unpinned image ref, image should be unpinned")
159+
assert.NoError(s.add(image))
160+
i, err := s.get(image.ID)
161+
assert.NoError(err)
162+
assert.False(i.Pinned)
163+
assert.False(s.isPinned(image.ID, ref1))
164+
165+
t.Logf("add pinned image ref, image should be pinned")
166+
ref2 := "containerd.io/ref-2"
167+
image.References = []string{ref2}
168+
image.Pinned = true
169+
assert.NoError(s.add(image))
170+
i, err = s.get(image.ID)
171+
assert.NoError(err)
172+
assert.True(i.Pinned)
173+
assert.False(s.isPinned(image.ID, ref1))
174+
assert.True(s.isPinned(image.ID, ref2))
175+
176+
t.Logf("pin unpinned image ref, image should be pinned, all refs should be pinned")
177+
assert.NoError(s.pin(image.ID, ref1))
178+
i, err = s.get(image.ID)
179+
assert.NoError(err)
180+
assert.True(i.Pinned)
181+
assert.True(s.isPinned(image.ID, ref1))
182+
assert.True(s.isPinned(image.ID, ref2))
183+
184+
t.Logf("unpin one of image refs, image should be pinned")
185+
assert.NoError(s.unpin(image.ID, ref2))
186+
i, err = s.get(image.ID)
187+
assert.NoError(err)
188+
assert.True(i.Pinned)
189+
assert.True(s.isPinned(image.ID, ref1))
190+
assert.False(s.isPinned(image.ID, ref2))
191+
192+
t.Logf("unpin the remaining one image ref, image should be unpinned")
193+
assert.NoError(s.unpin(image.ID, ref1))
194+
i, err = s.get(image.ID)
195+
assert.NoError(err)
196+
assert.False(i.Pinned)
197+
assert.False(s.isPinned(image.ID, ref1))
198+
assert.False(s.isPinned(image.ID, ref2))
199+
200+
t.Logf("pin one of image refs, then delete this, image should be unpinned")
201+
assert.NoError(s.pin(image.ID, ref1))
202+
s.delete(image.ID, ref1)
203+
i, err = s.get(image.ID)
204+
assert.NoError(err)
205+
assert.False(i.Pinned)
206+
assert.False(s.isPinned(image.ID, ref2))
207+
}
208+
140209
func TestImageStore(t *testing.T) {
141210
id := "sha256:1123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
142211
newID := "sha256:9923456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"

0 commit comments

Comments
 (0)