Skip to content

Commit 7c9ceb0

Browse files
gfouilletgfouillet
committed
fix(cloudimagemetdata): custom metadata may be considered as expired when querying dqlite state
- Before this commit, all expired metadata, regardless of source, were excluded from queries. - After this commit, only non-custom expired metadata will be excluded from queries. This is a fix since it wasn't consistent with the expiration policy which states "only non-custom" metadata expires. Custom metadata should always be retrieved through querying, as long as they are not filtered out by the provided criteria. Signed-off-by: Guillaume Fouillet <[email protected]>
1 parent 6ee57ec commit 7c9ceb0

File tree

2 files changed

+43
-26
lines changed

2 files changed

+43
-26
lines changed

‎domain/cloudimagemetadata/state/state.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ func (s *State) SaveMetadata(ctx context.Context, metadata []cloudimagemetadata.
101101
if err := s.tryCleanUpExpiredMetadata(ctx); err != nil {
102102
s.logger.Warningf("cannot cleanup expired metadata: %s", err)
103103
}
104+
s.logger.Debugf("saving %d metadata", len(metadata))
104105

105106
db, err := s.DB()
106107
if err != nil {
@@ -247,12 +248,15 @@ func (s *State) FindMetadata(ctx context.Context, criteria cloudimagemetadata.Me
247248
expirationTime := ttl{
248249
ExpiresAt: s.clock.Now().Add(-ExpirationDelay),
249250
}
250-
inputArgs := []any{expirationTime}
251+
customSource := inputMetadata{
252+
Source: cloudimagemetadata.CustomSource,
253+
}
254+
inputArgs := []any{expirationTime, customSource}
251255

252256
// clauses will collect all required clauses to build the final WHERE ... AND ... clause
253257
clauses := []string{
254-
// Ignores expired metadata
255-
fmt.Sprintf(`cloud_image_metadata.created_at >= $ttl.expires_at`),
258+
// Ignores expired metadata in case of non custom source.
259+
`source = $inputMetadata.source OR cloud_image_metadata.created_at >= $ttl.expires_at`,
256260
}
257261

258262
// declareEqualsClause is an helper function to add a sqlair clause with the format cloud_image_metadata.<field> = $metadataFilter.<field>,

‎domain/cloudimagemetadata/state/state_test.go‎

Lines changed: 36 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -294,15 +294,15 @@ func (s *stateSuite) TestFindMetadata(c *gc.C) {
294294
err := s.runQuery(`
295295
INSERT INTO cloud_image_metadata (uuid,created_at,source,stream,region,version,architecture_id,virt_type,root_storage_type,priority,image_id)
296296
VALUES
297-
('a',datetime('now','localtime'), 'custom', 'stream', 'region', '08.04',0, 'virtType', 'storage', 42, 'id'),
298-
('b',datetime('now','localtime'), 'custom', 'unique', 'region', '10.04',1, 'virtType', 'storage', 42, 'id'),
299-
('c',datetime('now','localtime'), 'custom', 'stream', 'unique', '12.04',2, 'virtType', 'storage', 42, 'id'),
300-
('d',datetime('now','localtime'), 'custom', 'stream', 'region', '14.00',3, 'virtType', 'storage', 42, 'id'),
301-
('e',datetime('now','localtime'), 'custom', 'stream', 'region', '16.04',4, 'virtType', 'storage', 42, 'id'),
302-
('f',datetime('now','localtime'), 'custom', 'stream', 'region', '18.04',0, 'unique', 'storage', 42, 'id'),
303-
('g',datetime('now','localtime'), 'custom', 'stream', 'region', '20.04',1, 'virtType', 'unique', 42, 'id'),
304-
('h',datetime('now','localtime'), 'custom', 'stream', 'region', '22.04',2, 'virtType', 'storage', 1, 'id'),
305-
('i',datetime('now','localtime'), 'custom', 'stream', 'region', '24.04',3, 'virtType', 'storage', 42, 'unique');
297+
('a',datetime('now','localtime'), 'non-custom', 'stream', 'region', '08.04',0, 'virtType', 'storage', 42, 'id'),
298+
('b',datetime('now','localtime'), 'non-custom', 'unique', 'region', '10.04',1, 'virtType', 'storage', 42, 'id'),
299+
('c',datetime('now','localtime'), 'non-custom', 'stream', 'unique', '12.04',2, 'virtType', 'storage', 42, 'id'),
300+
('d',datetime('now','localtime'), 'non-custom', 'stream', 'region', '14.00',3, 'virtType', 'storage', 42, 'id'),
301+
('e',datetime('now','localtime'), 'non-custom', 'stream', 'region', '16.04',4, 'virtType', 'storage', 42, 'id'),
302+
('f',datetime('now','localtime'), 'non-custom', 'stream', 'region', '18.04',0, 'unique', 'storage', 42, 'id'),
303+
('g',datetime('now','localtime'), 'non-custom', 'stream', 'region', '20.04',1, 'virtType', 'unique', 42, 'id'),
304+
('h',datetime('now','localtime'), 'non-custom', 'stream', 'region', '22.04',2, 'virtType', 'storage', 1, 'id'),
305+
('i',datetime('now','localtime'), 'non-custom', 'stream', 'region', '24.04',3, 'virtType', 'storage', 42, 'unique');
306306
`)
307307
c.Assert(err, jc.ErrorIsNil)
308308
expectedBase, err := s.retrieveMetadataFromDB()
@@ -371,8 +371,8 @@ func (s *stateSuite) TestFindMetadataNotFound(c *gc.C) {
371371
err := s.runQuery(`
372372
INSERT INTO cloud_image_metadata (uuid,created_at,source,stream,region,version,architecture_id,virt_type,root_storage_type,priority,image_id)
373373
VALUES
374-
('a',datetime('now','localtime'), 'custom', 'stream', 'unique', '08.04',0, 'virtType', 'storage', 42, 'id'),
375-
('b',datetime('now','localtime'), 'custom', 'unique', 'region', '10.04',1, 'virtType', 'storage', 42, 'id');
374+
('a',datetime('now','localtime'), 'non-custom', 'stream', 'unique', '08.04',0, 'virtType', 'storage', 42, 'id'),
375+
('b',datetime('now','localtime'), 'non-custom', 'unique', 'region', '10.04',1, 'virtType', 'storage', 42, 'id');
376376
`)
377377
c.Assert(err, jc.ErrorIsNil)
378378

@@ -383,15 +383,16 @@ VALUES
383383
c.Assert(err, jc.ErrorIs, errors.NotFound)
384384
}
385385

386-
// TestFindMetadataExpired checks that expired metadata entries are correctly excluded from query results.
386+
// TestFindMetadataExpired checks that non custom expired metadata entries are correctly excluded from query results.
387387
func (s *stateSuite) TestFindMetadataExpired(c *gc.C) {
388388
// Arrange
389389
err := s.runQuery(`
390390
INSERT INTO cloud_image_metadata (uuid,created_at,source,stream,region,version,architecture_id,virt_type,root_storage_type,priority,image_id)
391391
VALUES
392-
('a',datetime('now','-3 days'), 'custom', 'stream', 'region', '08.04',0, 'virtType', 'storage', 42, 'id'),
393-
('b',datetime('now','localtime'), 'custom', 'stream', 'region', '10.04',1, 'virtType', 'storage', 42, 'id'),
394-
('c',datetime('now','-3 days'), 'custom', 'stream', 'region', '12.04',1, 'virtType', 'storage', 42, 'id');
392+
('a',datetime('now','-3 days'), 'non-custom', 'stream', 'region', '08.04',0, 'virtType', 'storage', 42, 'id'),
393+
('b',datetime('now','localtime'), 'non-custom', 'stream', 'region', '10.04',1, 'virtType', 'storage', 42, 'id'),
394+
('c',datetime('now','-3 days'), 'non-custom', 'stream', 'region', '12.04',1, 'virtType', 'storage', 42, 'id'),
395+
('d',datetime('now','-2 days'), 'custom', 'stream', 'region', '14.04',1, 'virtType', 'storage', 42, 'id');
395396
`)
396397
c.Assert(err, jc.ErrorIsNil)
397398

@@ -411,6 +412,18 @@ VALUES
411412
Arch: "arm64",
412413
VirtType: "virtType",
413414
RootStorageType: "storage",
415+
Source: "non-custom",
416+
},
417+
Priority: 42,
418+
ImageID: "id",
419+
}, {
420+
MetadataAttributes: cloudimagemetadata.MetadataAttributes{
421+
Stream: "stream",
422+
Region: "region",
423+
Version: "14.04",
424+
Arch: "arm64",
425+
VirtType: "virtType",
426+
RootStorageType: "storage",
414427
Source: "custom",
415428
},
416429
Priority: 42,
@@ -426,10 +439,10 @@ func (s *stateSuite) TestAllCloudImageMetadata(c *gc.C) {
426439
err := s.runQuery(`
427440
INSERT INTO cloud_image_metadata (uuid,created_at,source,stream,region,version,architecture_id,virt_type,root_storage_type,priority,image_id)
428441
VALUES
429-
('a',datetime('now','-3 days'), 'custom', 'stream', 'region', '08.04',0, 'virtType', 'storage', 42, 'id'),
430-
('b',datetime('now','localtime'), 'custom', 'stream', 'region', '10.04',1, 'virtType', 'storage', 42, 'id'),
431-
('c',datetime('now','-3 days'), 'custom', 'stream', 'region', '12.04',1, 'virtType', 'storage', 42, 'id'),
432-
('d',datetime('now','localtime'), 'custom', 'stream', 'region', '16.04',1, 'virtType', 'storage', 42, 'id');
442+
('a',datetime('now','-3 days'), 'non-custom', 'stream', 'region', '08.04',0, 'virtType', 'storage', 42, 'id'),
443+
('b',datetime('now','localtime'), 'non-custom', 'stream', 'region', '10.04',1, 'virtType', 'storage', 42, 'id'),
444+
('c',datetime('now','-3 days'), 'non-custom', 'stream', 'region', '12.04',1, 'virtType', 'storage', 42, 'id'),
445+
('d',datetime('now','localtime'), 'non-custom', 'stream', 'region', '16.04',1, 'virtType', 'storage', 42, 'id');
433446
`)
434447
c.Assert(err, jc.ErrorIsNil)
435448

@@ -450,7 +463,7 @@ VALUES
450463
Arch: "arm64",
451464
VirtType: "virtType",
452465
RootStorageType: "storage",
453-
Source: "custom",
466+
Source: "non-custom",
454467
},
455468
Priority: 42,
456469
ImageID: "id",
@@ -463,7 +476,7 @@ VALUES
463476
Arch: "arm64",
464477
VirtType: "virtType",
465478
RootStorageType: "storage",
466-
Source: "custom",
479+
Source: "non-custom",
467480
},
468481
Priority: 42,
469482
ImageID: "id",
@@ -478,8 +491,8 @@ func (s *stateSuite) TestAllCloudImageMetadataNoMetadata(c *gc.C) {
478491
err := s.runQuery(`
479492
INSERT INTO cloud_image_metadata (uuid,created_at,source,stream,region,version,architecture_id,virt_type,root_storage_type,priority,image_id)
480493
VALUES
481-
('a',datetime('now','-3 days'), 'custom', 'stream', 'region', '08.04',0, 'virtType', 'storage', 42, 'id'),
482-
('b',datetime('now','-3 days'), 'custom', 'stream', 'region', '12.04',1, 'virtType', 'storage', 42, 'id');
494+
('a',datetime('now','-3 days'), 'non-custom', 'stream', 'region', '08.04',0, 'virtType', 'storage', 42, 'id'),
495+
('b',datetime('now','-3 days'), 'non-custom', 'stream', 'region', '12.04',1, 'virtType', 'storage', 42, 'id');
483496
`)
484497
c.Assert(err, jc.ErrorIsNil)
485498

0 commit comments

Comments
 (0)