Add image/upload delete lifecycle

This adds support for deleting images and uploads.  This could be
commanded in the future through the API, or automatically if there
are more than 2 valid images.

It emits a delete-image trigger event so that a job can be run to
clean up artifact storage, but complete implementation and testing
of that is left for a later change.

Change-Id: Iea6f3c9f1f235576540717a4fcfbc0243038bd26
This commit is contained in:
James E. Blair committed 2025-01-15 08:02:30 -08:00
1 parent 7c5df255ab
commit 1305081172
5 files changed
+399 -33

No files matched your search

+147 -28
View File
@@ -14,6 +14,7 @@
# under the License.
import textwrap
import time
from collections import defaultdict
from unittest import mock
@@ -143,6 +144,13 @@ class LauncherBaseTestCase(ZuulTestCase):
class TestLauncher(LauncherBaseTestCase):
def _waitForArtifacts(self, image_name, count):
for _ in iterate_timeout(30, "artifacts to settle"):
artifacts = self.launcher.image_build_registry.\
getArtifactsForImage(image_name)
if len(artifacts) == count:
return artifacts
@simple_layout('layouts/nodepool-missing-connection.yaml',
enable_nodepool=True)
def test_launcher_missing_connection(self):
@@ -193,20 +201,101 @@ class TestLauncher(LauncherBaseTestCase):
'review.example.com%2Forg%2Fcommon-config/debian-local',
'review.example.com%2Forg%2Fcommon-config/ubuntu-local',
]:
artifacts = self.launcher.image_build_registry.\
getArtifactsForImage(name)
self.assertEqual(2, len(artifacts))
self.assertEqual('qcow2', artifacts[0].format)
self.assertEqual('raw', artifacts[1].format)
artifacts = self._waitForArtifacts(name, 1)
self.assertEqual('raw', artifacts[0].format)
self.assertTrue(artifacts[0].validated)
self.assertTrue(artifacts[1].validated)
uploads = self.launcher.image_upload_registry.getUploadsForImage(
name)
self.assertEqual(1, len(uploads))
self.assertEqual(artifacts[1].uuid, uploads[0].artifact_uuid)
self.assertEqual(artifacts[0].uuid, uploads[0].artifact_uuid)
self.assertEqual("test_external_id", uploads[0].external_id)
self.assertTrue(uploads[0].validated)
@simple_layout('layouts/nodepool-image.yaml', enable_nodepool=True)
@return_data(
'build-debian-local-image',
'refs/heads/master',
LauncherBaseTestCase.debian_return_data,
)
@return_data(
'build-ubuntu-local-image',
'refs/heads/master',
LauncherBaseTestCase.ubuntu_return_data,
)
@mock.patch('zuul.driver.aws.awsendpoint.AwsProviderEndpoint.uploadImage',
return_value="test_external_id")
def test_launcher_image_expire(self, mock_uploadImage):
self.waitUntilSettled()
self.assertHistory([
dict(name='build-debian-local-image', result='SUCCESS'),
dict(name='build-ubuntu-local-image', result='SUCCESS'),
], ordered=False)
self.scheds.execute(lambda app: app.sched.reconfigure(app.config))
for _ in iterate_timeout(
30, "scheduler and launcher to have the same layout"):
if (self.scheds.first.sched.local_layout_state.get("tenant-one") ==
self.launcher.local_layout_state.get("tenant-one")):
break
# The build should not run again because the image is no
# longer missing
self.waitUntilSettled()
self.assertHistory([
dict(name='build-debian-local-image', result='SUCCESS'),
dict(name='build-ubuntu-local-image', result='SUCCESS'),
], ordered=False)
for name in [
'review.example.com%2Forg%2Fcommon-config/debian-local',
'review.example.com%2Forg%2Fcommon-config/ubuntu-local',
]:
artifacts = self._waitForArtifacts(name, 1)
self.assertEqual('raw', artifacts[0].format)
self.assertTrue(artifacts[0].validated)
uploads = self.launcher.image_upload_registry.getUploadsForImage(
name)
self.assertEqual(1, len(uploads))
self.assertEqual(artifacts[0].uuid, uploads[0].artifact_uuid)
self.assertEqual("test_external_id", uploads[0].external_id)
self.assertTrue(uploads[0].validated)
image_cname = 'review.example.com%2Forg%2Fcommon-config/ubuntu-local'
# Run another build event manually
driver = self.launcher.connections.drivers['zuul']
event = driver.getImageBuildEvent(
['ubuntu-local'], 'review.example.com',
'org/common-config', 'master')
self.launcher.trigger_events['tenant-one'].put(
event.trigger_name, event)
self.waitUntilSettled()
self.assertHistory([
dict(name='build-debian-local-image', result='SUCCESS'),
dict(name='build-ubuntu-local-image', result='SUCCESS'),
dict(name='build-ubuntu-local-image', result='SUCCESS'),
], ordered=False)
self._waitForArtifacts(image_cname, 2)
# Run another build event manually
driver = self.launcher.connections.drivers['zuul']
event = driver.getImageBuildEvent(
['ubuntu-local'], 'review.example.com',
'org/common-config', 'master')
self.launcher.trigger_events['tenant-one'].put(
event.trigger_name, event)
self.waitUntilSettled()
self.assertHistory([
dict(name='build-debian-local-image', result='SUCCESS'),
dict(name='build-ubuntu-local-image', result='SUCCESS'),
dict(name='build-ubuntu-local-image', result='SUCCESS'),
dict(name='build-ubuntu-local-image', result='SUCCESS'),
], ordered=False)
self._waitForArtifacts(image_cname, 3)
# Trigger a deletion run
self.launcher.upload_deleted_event.set()
self.launcher.wake_event.set()
self._waitForArtifacts(image_cname, 2)
@simple_layout('layouts/nodepool-image-no-validate.yaml',
enable_nodepool=True)
@return_data(
@@ -238,17 +327,13 @@ class TestLauncher(LauncherBaseTestCase):
dict(name='build-debian-local-image', result='SUCCESS'),
])
name = 'review.example.com%2Forg%2Fcommon-config/debian-local'
artifacts = self.launcher.image_build_registry.getArtifactsForImage(
name)
self.assertEqual(2, len(artifacts))
self.assertEqual('qcow2', artifacts[0].format)
self.assertEqual('raw', artifacts[1].format)
artifacts = self._waitForArtifacts(name, 1)
self.assertEqual('raw', artifacts[0].format)
self.assertFalse(artifacts[0].validated)
self.assertFalse(artifacts[1].validated)
uploads = self.launcher.image_upload_registry.getUploadsForImage(
name)
self.assertEqual(1, len(uploads))
self.assertEqual(artifacts[1].uuid, uploads[0].artifact_uuid)
self.assertEqual(artifacts[0].uuid, uploads[0].artifact_uuid)
self.assertEqual("test_external_id", uploads[0].external_id)
self.assertFalse(uploads[0].validated)
@@ -285,20 +370,58 @@ class TestLauncher(LauncherBaseTestCase):
dict(name='validate-debian-local-image', result='SUCCESS'),
])
name = 'review.example.com%2Forg%2Fcommon-config/debian-local'
artifacts = self.launcher.image_build_registry.getArtifactsForImage(
name)
self.assertEqual(2, len(artifacts))
self.assertEqual('qcow2', artifacts[0].format)
self.assertEqual('raw', artifacts[1].format)
artifacts = self._waitForArtifacts(name, 1)
self.assertEqual('raw', artifacts[0].format)
self.assertFalse(artifacts[0].validated)
self.assertFalse(artifacts[1].validated)
uploads = self.launcher.image_upload_registry.getUploadsForImage(
name)
self.assertEqual(1, len(uploads))
self.assertEqual(artifacts[1].uuid, uploads[0].artifact_uuid)
self.assertEqual(artifacts[0].uuid, uploads[0].artifact_uuid)
self.assertEqual("test_external_id", uploads[0].external_id)
self.assertTrue(uploads[0].validated)
@simple_layout('layouts/nodepool.yaml', enable_nodepool=True)
@mock.patch('zuul.driver.aws.awsendpoint.AwsProviderEndpoint.uploadImage',
return_value="test_external_id")
def test_launcher_crashed_upload(self, mock_uploadImage):
self.waitUntilSettled()
provider = self.launcher._getProvider(
'tenant-one', 'aws-us-east-1-main')
endpoint = provider.getEndpoint()
image = list(provider.images.values())[0]
# create an IBA and an upload
with self.createZKContext(None) as ctx:
# This starts with an unknown state, then
# createImageUploads will set it to ready.
iba = model.ImageBuildArtifact.new(
ctx,
uuid='iba-uuid',
name=image.name,
canonical_name=image.canonical_name,
project_canonical_name=image.project_canonical_name,
url='http://example.com/image.raw',
timestamp=time.time(),
)
with iba.locked(self.zk_client):
model.ImageUpload.new(
ctx,
uuid='upload-uuid',
artifact_uuid='iba-uuid',
endpoint_name=endpoint.canonical_name,
providers=[provider.canonical_name],
canonical_name=image.canonical_name,
timestamp=time.time(),
_state=model.ImageUpload.State.UPLOADING,
state_time=time.time(),
)
with iba.activeContext(ctx):
iba.state = iba.State.READY
self.waitUntilSettled()
pending_uploads = [
u for u in self.launcher.image_upload_registry.getItems()
if u.state == u.State.PENDING]
self.assertEqual(0, len(pending_uploads))
@simple_layout('layouts/nodepool.yaml', enable_nodepool=True)
def test_jobs_executed(self):
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
@@ -457,17 +580,13 @@ class TestLauncher(LauncherBaseTestCase):
for name in [
'review.example.com%2Forg%2Fproject/debian-local',
]:
artifacts = self.launcher.image_build_registry.\
getArtifactsForImage(name)
self.assertEqual(2, len(artifacts))
self.assertEqual('qcow2', artifacts[0].format)
self.assertEqual('raw', artifacts[1].format)
artifacts = self._waitForArtifacts(name, 1)
self.assertEqual('raw', artifacts[0].format)
self.assertTrue(artifacts[0].validated)
self.assertTrue(artifacts[1].validated)
uploads = self.launcher.image_upload_registry.getUploadsForImage(
name)
self.assertEqual(1, len(uploads))
self.assertEqual(artifacts[1].uuid, uploads[0].artifact_uuid)
self.assertEqual(artifacts[0].uuid, uploads[0].artifact_uuid)
self.assertEqual("test_external_id", uploads[0].external_id)
self.assertTrue(uploads[0].validated)
+18
View File
@@ -32,6 +32,7 @@ PARENT_CHANGE_ENQUEUED = 'parent-change-enqueued'
PROJECT_CHANGE_MERGED = 'project-change-merged'
IMAGE_BUILD = 'image-build'
IMAGE_VALIDATE = 'image-validate'
IMAGE_DELETE = 'image-delete'
class ZuulDriver(Driver, TriggerInterface, ReporterInterface):
@@ -201,6 +202,23 @@ class ZuulDriver(Driver, TriggerInterface, ReporterInterface):
event.arrived_at_scheduler_timestamp = event.timestamp
return event
def getImageDeleteEvent(self, image_names, project_hostname, project_name,
project_branch, image_build_uuid):
event = ZuulTriggerEvent()
event.type = IMAGE_DELETE
event.connection_name = "zuul"
event.trigger_name = self.name
event.image_names = image_names
event.image_build_uuid = image_build_uuid
event.project_hostname = project_hostname
event.project_name = project_name
event.branch = project_branch
event.ref = f'refs/heads/{project_branch}'
event.zuul_event_id = str(uuid4().hex)
event.timestamp = time.time()
event.arrived_at_scheduler_timestamp = event.timestamp
return event
def getTrigger(self, connection, config=None):
return zuultrigger.ZuulTrigger(self, config)
+162 -5
View File
@@ -84,6 +84,41 @@ class ProviderNodeError(Exception):
pass
class DeleteJob:
log = logging.getLogger("zuul.Launcher")
def __init__(self, launcher, image_build_artifact, upload):
self.launcher = launcher
self.image_build_artifact = image_build_artifact
self.upload = upload
def run(self):
try:
self._run()
except Exception:
self.log.exception("Error in delete job")
def _run(self):
try:
with self.launcher.createZKContext(None, self.log) as ctx:
try:
with self.upload.locked(ctx, blocking=False):
self.log.info("Deleting image upload %s", self.upload)
with self.upload.activeContext(ctx):
self.upload.state = self.upload.State.DELETING
provider_cname = self.upload.providers[0]
provider = self.launcher.\
_getProviderByCanonicalName(provider_cname)
provider.deleteImage(self.upload.external_id)
self.upload.delete(ctx)
self.launcher.upload_deleted_event.set()
self.launcher.wake_event.set()
except LockException:
return
except Exception:
self.log.exception("Unable to delete upload %s", self.upload)
class UploadJob:
log = logging.getLogger("zuul.Launcher")
@@ -116,6 +151,8 @@ class UploadJob:
self.log.debug(
"Acquired upload lock for %s",
upload)
with upload.activeContext(ctx):
upload.state = upload.State.UPLOADING
except LockException:
# We may have raced another launcher; set the
# event to try again.
@@ -148,6 +185,15 @@ class UploadJob:
except Exception:
self.log.exception("Unable to release lock for %s",
upload)
try:
with upload.activeContext(ctx):
if upload.external_id:
upload.state = upload.State.READY
else:
upload.state = upload.State.PENDING
except Exception:
self.log.exception("Unable to update state for %s",
upload)
if path:
try:
os.unlink(path)
@@ -261,6 +307,7 @@ class Launcher:
self.layout_updated_event.set()
self.image_updated_event = threading.Event()
self.upload_deleted_event = threading.Event()
self.tenant_layout_state = LayoutStateStore(
self.zk_client, self._layoutUpdatedCallback)
@@ -318,10 +365,14 @@ class Launcher:
if self.layout_updated_event.is_set():
self.layout_updated_event.clear()
if self.updateTenantProviders():
self.checkOldImages()
self.checkMissingImages()
self.checkMissingUploads()
if self.image_updated_event.is_set():
self.checkOldImages()
self.checkMissingUploads()
if self.upload_deleted_event.is_set():
self.checkOldImages()
self._processRequests()
self._processNodes()
self._processMinReady()
@@ -1006,6 +1057,18 @@ class Launcher:
tenant_name, iba.name)
self.trigger_events[tenant_name].put(event.trigger_name, event)
def addImageDeleteEvent(self, iba):
project_hostname, project_name = \
iba.project_canonical_name.split('/', 1)
tenant_name = iba.build_tenant_name
driver = self.connections.drivers['zuul']
event = driver.getImageDeleteEvent(
[iba.name], project_hostname, project_name, iba.project_branch,
iba.uuid)
self.log.info("Submitting image delete event for %s %s",
tenant_name, iba.name)
self.trigger_events[tenant_name].put(event.trigger_name, event)
def checkMissingImages(self):
for tenant_name, providers in self.tenant_providers.items():
images_by_project_branch = {}
@@ -1024,9 +1087,9 @@ class Launcher:
# this image, skip.
self.log.debug("Checking for missing images")
seen_formats = set()
for build in self.image_build_registry.getArtifactsForImage(
for iba in self.image_build_registry.getArtifactsForImage(
image.canonical_name):
seen_formats.add(build.format)
seen_formats.add(iba.format)
if image.format in seen_formats:
# We have at least one build with the required
@@ -1039,16 +1102,110 @@ class Launcher:
images = images_by_project_branch.setdefault(key, set())
images.add(image.name)
def checkOldImages(self):
self.log.debug("Checking for old images")
self.upload_deleted_event.clear()
keep_uploads = set()
for tenant_name, providers in self.tenant_providers.items():
for provider in providers:
for image in provider.images.values():
if image.type == 'zuul':
self.checkOldImage(tenant_name, provider, image,
keep_uploads)
# Get the list of ibas for which we could consider uploads
# first (to make sure that we don't race the creation of
# uploads and setting the iba to ready).
active_ibas = [iba for iba in self.image_build_registry.getItems()
if iba.state in
(iba.State.DELETING, iba.State.READY)]
uploads_by_artifact = collections.defaultdict(list)
latest_upload_timestamp = 0
for upload in self.image_upload_registry.getItems():
if upload.timestamp > latest_upload_timestamp:
latest_upload_timestamp = upload.timestamp
uploads_by_artifact[upload.artifact_uuid].append(upload)
iba = self.image_build_registry.getItem(upload.artifact_uuid)
if not iba:
self.log.warning("Unable to find artifact for upload %s",
upload.artifact_uuid)
continue
if (iba.state == iba.State.DELETING or
upload.state == upload.State.DELETING or
(upload.state == upload.State.READY and
upload not in keep_uploads)):
self.upload_executor.submit(DeleteJob(self, iba, upload).run)
for iba in active_ibas:
if (iba.timestamp > latest_upload_timestamp or
not latest_upload_timestamp):
# Ignore artifacts that are newer than the newest upload
continue
if len(uploads_by_artifact[iba.uuid]) == 0:
self.log.info("Deleting image build artifact "
"with no uploads: %s", iba)
with self.createZKContext(None, self.log) as ctx:
try:
with iba.locked(ctx, blocking=False):
iba.delete(ctx)
except LockException:
pass
def checkOldImage(self, tenant_name, provider, image,
keep_uploads):
self.log.debug("Checking for old artifacts for image %s",
image.canonical_name)
image_cname = image.canonical_name
uploads = self.image_upload_registry.getUploadsForImage(image_cname)
valid_uploads = [
upload for upload in uploads
if (provider.canonical_name in upload.providers and
upload.validated and
upload.external_id)
]
# Keep the 2 most recent validated uploads (uploads are
# already sorted by timestamp)
newest_valid_uploads = valid_uploads[-2:]
keep_uploads.update(set(newest_valid_uploads))
# And also keep any uploads (regardless of validation) newer
# than that (since they may have validation jobs running).
if newest_valid_uploads:
oldest_good_timestamp = newest_valid_uploads[0].timestamp
else:
oldest_good_timestamp = 0
new_uploads = [
upload for upload in uploads
if (provider.canonical_name in upload.providers and
upload.timestamp > oldest_good_timestamp)
]
keep_uploads.update(set(new_uploads))
def checkMissingUploads(self):
self.log.debug("Checking for missing uploads")
uploads_by_artifact_id = collections.defaultdict(list)
self.image_updated_event.clear()
for upload in self.image_upload_registry.getItems():
self.log.debug("Checking %s", upload)
if upload.external_id:
continue
if upload.endpoint_name not in self.endpoints:
continue
iba = self.image_build_registry.getItem(upload.artifact_uuid)
if not iba:
self.log.warning("Unable to find artifact for upload %s",
upload.artifact_uuid)
continue
if iba.state == iba.State.DELETING:
continue
if upload.state == upload.State.UPLOADING:
# If there is an unlocked upload in uploading state,
# it has probably crashed. Reset it.
if not upload.is_locked:
with self.createZKContext(None, self.log) as ctx:
with (upload.locked(ctx, blocking=False),
upload.activeContext(ctx)):
# Double check the state after lock.
if upload.state == upload.State.UPLOADING:
upload.state = upload.State.PENDING
if upload.state != upload.State.PENDING:
continue
upload_list = uploads_by_artifact_id[upload.artifact_uuid]
upload_list.append(upload)
+64
View File
@@ -1471,6 +1471,15 @@ class ImageBuildArtifact(zkobject.LockableZKObject):
IMAGES_PATH = "artifacts"
LOCKS_PATH = "locks"
class State(StrEnum):
READY = "ready"
DELETING = "deleting"
STATES = set([
State.READY,
State.DELETING,
])
def __init__(self):
super().__init__()
self._set(
@@ -1487,10 +1496,27 @@ class ImageBuildArtifact(zkobject.LockableZKObject):
url=None,
timestamp=None,
validated=None,
_state=None,
state_time=None,
# Attributes that are not serialized
lock=None,
is_locked=False,
)
@property
def state(self):
return self._state
@state.setter
def state(self, value):
if value not in self.STATES:
raise TypeError("'%s' is not a valid state" % value)
self._state = value
self.state_time = time.time()
def __repr__(self):
return (f"<ImageBuildArtifact {self.uuid} "
f"state: {self.state} "
f"canonical_name: {self.canonical_name} "
f"build_uuid: {self.build_uuid} "
f"validated: {self.validated}>")
@@ -1516,6 +1542,8 @@ class ImageBuildArtifact(zkobject.LockableZKObject):
url=self.url,
timestamp=self.timestamp,
validated=self.validated,
_state=self._state,
state_time=self.state_time,
)
return json.dumps(data, sort_keys=True).encode("utf-8")
@@ -1525,6 +1553,19 @@ class ImageUpload(zkobject.LockableZKObject):
UPLOADS_PATH = "uploads"
LOCKS_PATH = "locks"
class State(StrEnum):
READY = "ready"
DELETING = "deleting"
PENDING = "pending"
UPLOADING = "uploading"
STATES = set([
State.READY,
State.DELETING,
State.PENDING,
State.UPLOADING,
])
def __init__(self):
super().__init__()
self._set(
@@ -1537,10 +1578,27 @@ class ImageUpload(zkobject.LockableZKObject):
external_id=None,
timestamp=None,
validated=None,
_state=None,
state_time=None,
# Attributes that are not serialized
lock=None,
is_locked=False,
)
@property
def state(self):
return self._state
@state.setter
def state(self, value):
if value not in self.STATES:
raise TypeError("'%s' is not a valid state" % value)
self._state = value
self.state_time = time.time()
def __repr__(self):
return (f"<ImageUpload {self.uuid} "
f"state: {self.state} "
f"endpoint: {self.endpoint_name} "
f"artifact: {self.artifact_uuid} "
f"validated: {self.validated} "
@@ -1563,6 +1621,8 @@ class ImageUpload(zkobject.LockableZKObject):
external_id=self.external_id,
timestamp=self.timestamp,
validated=self.validated,
_state=self._state,
state_time=self.state_time,
)
return json.dumps(data, sort_keys=True).encode("utf-8")
@@ -5880,6 +5940,7 @@ class EventInfo:
# by the reporter.
self.image_names = None
self.image_upload_uuid = None
self.image_build_uuid = None
@classmethod
def fromEvent(cls, event, event_ref_key):
@@ -5893,6 +5954,7 @@ class EventInfo:
tinfo.ref = None
tinfo.image_names = getattr(event, 'image_names', None)
tinfo.image_upload_uuid = getattr(event, 'image_upload_uuid', None)
tinfo.image_build_uuid = getattr(event, 'image_build_uuid', None)
return tinfo
@classmethod
@@ -5905,6 +5967,7 @@ class EventInfo:
tinfo.ref = d.get("ref")
tinfo.image_names = d.get("image_names")
tinfo.image_upload_uuid = d.get("image_upload_uuid")
tinfo.image_build_uuid = d.get("image_build_uuid")
return tinfo
def toDict(self):
@@ -5915,6 +5978,7 @@ class EventInfo:
"ref": self.ref,
"image_names": self.image_names,
"image_upload_uuid": self.image_upload_uuid,
"image_build_uuid": self.image_build_uuid,
}
+8
View File
@@ -3231,6 +3231,8 @@ class Scheduler(threading.Thread):
art_uuid, image.canonical_name, image_format, build.uuid,
artifact_url)
with self.createZKContext(None, self.log) as ctx:
# This starts with an unknown state, then
# createImageUploads will set it to ready.
return ImageBuildArtifact.new(
ctx,
uuid=art_uuid,
@@ -3285,7 +3287,13 @@ class Scheduler(threading.Thread):
config_hash=config_hash,
timestamp=time.time(),
validated=iba.validated,
_state=ImageUpload.State.PENDING,
state_time=time.time(),
)
# Only mark the iba as ready once all the upload
# records for it have been created.
with iba.activeContext(ctx):
iba.state = iba.State.READY
def validateImageUpload(self, image_upload_uuid):
with self.createZKContext(None, self.log) as ctx: