[Project Sync] Gate MLRun's Nuclio-deletion wait to leader=mlrun by daanvdk-mck · Pull Request #10101 · mlrun/mlrun · GitHub
Skip to content

[Project Sync] Gate MLRun's Nuclio-deletion wait to leader=mlrun - #10101

Closed
daanvdk-mck wants to merge 2 commits into
mlrun:feature/project-sync-followerfrom
daanvdk-mck:project-sync-nuclio-delete-gate
Closed

daanvdk-mck wants to merge 2 commits into
mlrun:feature/project-sync-followerfrom
daanvdk-mck:project-sync-nuclio-delete-gate

Conversation

@daanvdk-mck

@daanvdk-mck daanvdk-mck commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

📝 Description

delete_project_resources() unconditionally waits for Nuclio to report a project's deletion whenever nuclio_dashboard_url is set. That's correct in CE (leader=mlrun): MLRun's own leader.py fans delete_project out to an ordered "nuclio" follower that issues the real Nuclio DELETE before this wait ever runs.

The new Orca 2PC commit_delete_project follower hook (#10042) funnels through this same cascading path, but in the enterprise model Orca deletes the Nuclio project itself, directly, with no ordering guarantee visible to MLRun. Since commit_delete_project blocks synchronously and is timed to the full delete operation, this leftover wait can spuriously block/fail it for a step that's no longer MLRun's responsibility to verify.


🛠️ Changes Made

  • crud/projects.py: skip _wait_for_nuclio_project_deletion in delete_project_resources() when httpdb.projects.leader == "orca", matching the existing httpdb.projects.leader == "mlrun" idiom already used in framework/api/utils.py and crud/runtimes/nuclio/function.py. CE behavior is unchanged.

✅ Checklist

  • I updated the documentation (if applicable)
  • I have tested the changes in this PR
  • I confirmed whether my changes are covered by system tests
    • If yes, I ran all relevant system tests and ensured they passed before submitting this PR
    • I updated existing system tests and/or added new ones if needed to cover my changes
  • If I introduced a deprecation:
  • Please run Smoke-tests workflow, providing it the PR number as input (upon success, the workflow run will add label "Smoke tests: Pass" to the PR)

🧪 Testing

  • Added patched_delete_project_resources_dependencies fixture plus two unit tests in test_projects.py asserting the wait runs under leader=mlrun and is skipped under leader=orca.
  • ruff check / ruff format --check pass on both changed files.

🔗 References


🚨 Breaking Changes?

  • Yes (explain below)
  • No

🔍️ Additional Notes

  • This assumes Orca (or Nuclio itself) picks up an equivalent "wait for actual Nuclio cleanup" check on its own side of the 2PC flow - worth confirming that's part of the Orca-side design, since this PR only removes MLRun's half of that responsibility, it doesn't relocate it.

🤖 Generated with Claude Code

…ject leader

delete_project_resources() unconditionally waited for Nuclio to report a
project's deletion whenever nuclio_dashboard_url was set, assuming MLRun's
own leader.py had already triggered the real Nuclio delete first (true only
in CE, leader=mlrun). The new Orca 2PC commit_delete_project follower hook
funnels through this same cascading path, but Orca now deletes the Nuclio
project itself with no ordering guarantee visible to MLRun, so the wait
could spuriously block/fail commit_delete_project. Gate it on leader=orca.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

Comment thread server/py/services/api/crud/projects.py Outdated
…just any non-orca leader

The previous leader==orca check was broader than the actual invariant: the
wait is only justified when MLRun's own leader-driven fan-out is what
triggered the real Nuclio delete, which requires both self-leading
(leader in [mlrun, nop-self-leader]) and nuclio being listed in
httpdb.projects.followers. Any other external leader (iguazio included, not
just orca) never has that guarantee. Added
Config.get_configured_project_followers()/is_project_follower_configured(),
reused by leader.py's existing follower init, so the call site collapses to
one condition instead of duplicating the leader check.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Smoke Tests with the PR merged with 1 new commits from feature/project-sync-follower HEAD 48bf0946: Pass ✅

@github-actions

Copy link
Copy Markdown
Contributor

📊 Diff Coverage Report

📂 Click to view full coverage details

Diff Coverage

Diff: origin/development...HEAD, staged and unstaged changes

  • mlrun/config.py (100%)
  • server/py/framework/api/deps.py (100%)
  • server/py/framework/db/sqldb/db.py (60.0%): Missing lines 3600-3601,3604,3616,3618,3620
  • server/py/framework/utils/clients/chief.py (50.0%): Missing lines 197
  • server/py/framework/utils/clients/iguazio/v4.py (90.6%): Missing lines 371,471,493,515,517,519,534,536,538
  • server/py/framework/utils/clients/service_account_token.py (100%)
  • server/py/framework/utils/projects/follower.py (100%)
  • server/py/framework/utils/projects/follower_contract.py (100%)
  • server/py/framework/utils/projects/follower_schemas.py (100%)
  • server/py/framework/utils/projects/leader.py (100%)
  • server/py/services/api/api/api.py (100%)
  • server/py/services/api/api/endpoints/projects_follower.py (59.6%): Missing lines 48,64-65,68,73,78,89,92,103-104,107,112,119,130,136,150,154,158,164,169,174,193,200
  • server/py/services/api/crud/projects.py (94.9%): Missing lines 1167,1235,1319,1398-1399

Summary

  • Total: 366 lines
  • Missing: 44 lines
  • Coverage: 87%

server/py/framework/db/sqldb/db.py

Lines 3596-3608

  3596         updated_at_col = func.coalesce(Project.updated_at, _KEYSET_PAGINATION_EPOCH)
  3597         if updated_after is not None:
  3598             query = query.filter(updated_at_col >= updated_after)
  3599         if keyset_after is not None or limit is not None:
! 3600             if keyset_after is not None:
! 3601                 after_updated_at, after_name = keyset_after
  3602                 # Portable keyset predicate (works identically on SQLite/Postgres/MySQL)
  3603                 # — avoids row-value comparison syntax some dialects handle inconsistently.
! 3604                 query = query.filter(
  3605                     or_(
  3606                         updated_at_col > after_updated_at,
  3607                         and_(
  3608                             updated_at_col == after_updated_at,

Lines 3612-3624

  3612                 )
  3613             # A stable order is required for keyset pagination to mean anything — the
  3614             # very first page (keyset_after=None) still needs it, so the boundary row
  3615             # a caller computes its next cursor from is consistent across pages.
! 3616             query = query.order_by(updated_at_col, Project.name)
  3617         if limit is not None:
! 3618             query = query.limit(limit)
  3619         if for_update:
! 3620             query = query.with_for_update()
  3621 
  3622         project_records = query.all()
  3623         return mlrun.common.schemas.ProjectsOutput(
  3624             projects=self._format_projects(project_records, format_)

server/py/framework/utils/clients/chief.py

Lines 193-201

  193         Re-routes the /follower/projects commit-delete call to chief. Blocks until
  194         the full purge finishes, so the timeout needs to cover that, not the legacy
  195         chief-proxy default.
  196         """
! 197         return await self._proxy_request_to_chief(
  198             "DELETE",
  199             f"follower/projects/{name}",
  200             request,
  201             timeout=int(

server/py/framework/utils/clients/iguazio/v4.py

Lines 367-375

  367 
  368         if response.status_code == requests.codes.accepted:
  369             # 202: accepted, still converging - poll for completion
  370             if not wait_for_completion:
! 371                 return True
  372             op_id = response.json()["status"]["opId"]
  373             self._wait_for_op(name, op_id, auth_info)
  374 
  375         return False

Lines 467-475

  467                 auth_info,
  468                 fatal_exceptions=(_OrcaActionFailedError,),
  469             )
  470         except _OrcaActionFailedError as exc:
! 471             raise mlrun.errors.MLRunRuntimeError(str(exc)) from exc
  472 
  473     def _verify_op_terminal(
  474         self,
  475         name: str,

Lines 489-497

  489             },
  490         )
  491         items = response.json().get("items", [])
  492         if not items:
! 493             raise mlrun.errors.MLRunRuntimeError(
  494                 f"No Orca sync-project action observed yet for project {name} (op_id={op_id})"
  495             )
  496         state = items[0].get("status", {}).get("state")
  497         if state == "failed":

Lines 511-523

  511         wire = {"name": project.metadata.name}
  512         if project.spec.owner:
  513             wire["owner"] = project.spec.owner
  514         if project.spec.description:
! 515             wire["description"] = project.spec.description
  516         if project.metadata.labels:
! 517             wire["labels"] = project.metadata.labels
  518         if project.metadata.annotations:
! 519             wire["annotations"] = project.metadata.annotations
  520         return wire
  521 
  522     @staticmethod
  523     def _update_project_wire(

Lines 530-542

  530             "prevOpId": str(prev_op_id) if prev_op_id else None,
  531             "owner": project.spec.owner,
  532         }
  533         if project.spec.description:
! 534             wire["description"] = project.spec.description
  535         if project.metadata.labels:
! 536             wire["labels"] = project.metadata.labels
  537         if project.metadata.annotations:
! 538             wire["annotations"] = project.metadata.annotations
  539         return wire
  540 
  541     @staticmethod
  542     def _wire_to_project(body: dict) -> mlrun.common.schemas.Project:

server/py/services/api/api/endpoints/projects_follower.py

Lines 44-52

  44 def _to_follower_state(
  45     name: str, snapshot: mlrun.common.schemas.Project | None
  46 ) -> follower_schemas.FollowerProjectState:
  47     if snapshot is None:
! 48         return follower_schemas.FollowerProjectState(name=name)
  49     return follower_schemas.FollowerProjectState(
  50         name=snapshot.metadata.name,
  51         op_id=snapshot.status.op_id,
  52         sync_status=snapshot.status.state,

Lines 60-82

  60 async def prepare_create_project(
  61     name: str,
  62     body: follower_schemas.FollowerPrepareCreateRequest,
  63 ) -> follower_schemas.FollowerProjectState:
! 64     if body.metadata.name != name:
! 65         raise mlrun.errors.MLRunBadRequestError(
  66             "Path project name does not match metadata.name"
  67         )
! 68     project = mlrun.common.schemas.Project(
  69         metadata=body.metadata,
  70         spec=body.spec,
  71         status=mlrun.common.schemas.ProjectStatus(op_id=body.status.op_id),
  72     )
! 73     result = await mlrun.utils.run_in_threadpool(
  74         services.api.crud.Projects().prepare_create_project,
  75         project,
  76         body.status.op_id,
  77     )
! 78     return _to_follower_state(name, result)
  79 
  80 
  81 @router.post(
  82     "/follower/projects/{name}/commit-create",

Lines 85-96

  85 async def commit_create_project(
  86     name: str,
  87     body: follower_schemas.FollowerCommitCreateRequest,
  88 ) -> follower_schemas.FollowerProjectState:
! 89     result = await mlrun.utils.run_in_threadpool(
  90         services.api.crud.Projects().commit_create_project, name, body.status.op_id
  91     )
! 92     return _to_follower_state(name, result)
  93 
  94 
  95 @router.put(
  96     "/follower/projects/{name}",

Lines 99-116

   99 async def update_project(
  100     name: str,
  101     body: follower_schemas.FollowerUpdateRequest,
  102 ) -> follower_schemas.FollowerProjectState:
! 103     if body.metadata.name != name:
! 104         raise mlrun.errors.MLRunBadRequestError(
  105             "Path project name does not match metadata.name"
  106         )
! 107     project = mlrun.common.schemas.Project(
  108         metadata=body.metadata,
  109         spec=body.spec,
  110         status=mlrun.common.schemas.ProjectStatus(op_id=body.status.op_id),
  111     )
! 112     result = await mlrun.utils.run_in_threadpool(
  113         services.api.crud.Projects().update_project_follower,
  114         name,
  115         project,
  116         op_id=body.status.op_id,

Lines 115-123

  115         project,
  116         op_id=body.status.op_id,
  117         prev_op_id=body.prev_op_id,
  118     )
! 119     return _to_follower_state(name, result)
  120 
  121 
  122 @router.post(
  123     "/follower/projects/{name}/prepare-delete",

Lines 126-134

  126 async def prepare_delete_project(
  127     name: str,
  128     body: follower_schemas.FollowerPrepareDeleteRequest,
  129 ) -> follower_schemas.FollowerProjectState:
! 130     result = await mlrun.utils.run_in_threadpool(
  131         services.api.crud.Projects().prepare_delete_project,
  132         name,
  133         op_id=body.status.op_id,
  134         prev_op_id=body.prev_op_id,

Lines 132-140

  132         name,
  133         op_id=body.status.op_id,
  134         prev_op_id=body.prev_op_id,
  135     )
! 136     return _to_follower_state(name, result)
  137 
  138 
  139 @router.delete(
  140     "/follower/projects/{name}",

Lines 146-162

  146     request: fastapi.Request,
  147 ) -> follower_schemas.FollowerProjectState:
  148     # delete_project_resources deletes schedules, which run only on chief, so we
  149     # re-route to chief — same reason the legacy (non-follower) delete endpoint does.
! 150     if (
  151         mlrun.mlconf.httpdb.clusterization.role
  152         != mlrun.common.schemas.ClusterizationRole.chief
  153     ):
! 154         logger.info(
  155             "Requesting to commit-delete follower project, re-routing to chief",
  156             project=name,
  157         )
! 158         return (
  159             await framework.utils.clients.chief.Client().commit_delete_follower_project(
  160                 name=name, request=request
  161             )
  162         )

Lines 160-178

  160                 name=name, request=request
  161             )
  162         )
  163 
! 164     op_id = body.status.op_id
  165     # Validates internally (CAS/ordering/state) and, on success, purges the project's
  166     # resources and its row — a no-op if it's already gone (a previous call already
  167     # removed it) or a genuine retry with the same op_id (e.g. after a dropped
  168     # connection re-runs the purge rather than skipping it).
! 169     await mlrun.utils.run_in_threadpool(
  170         services.api.crud.Projects().commit_delete_project, name, op_id
  171     )
  172     # The row is gone at this point — sync_status reports the state it was in right
  173     # before removal, matching every other op's {name, op_id, sync_status} response.
! 174     return follower_schemas.FollowerProjectState(
  175         name=name,
  176         op_id=op_id,
  177         sync_status=mlrun.common.schemas.ProjectState.deleting,
  178     )

Lines 189-197

  189     db_session: sqlalchemy.orm.Session = fastapi.Depends(
  190         framework.api.deps.get_db_session
  191     ),
  192 ) -> follower_schemas.FollowerProjectStatesPage:
! 193     projects, next_cursor = await mlrun.utils.run_in_threadpool(
  194         services.api.crud.Projects().list_project_states,
  195         db_session,
  196         updated_after,
  197         cursor,

Lines 196-204

  196         updated_after,
  197         cursor,
  198         page_size,
  199     )
! 200     return follower_schemas.FollowerProjectStatesPage(
  201         projects=[
  202             _to_follower_state(project.metadata.name, project) for project in projects
  203         ],
  204         next_cursor=next_cursor,

server/py/services/api/crud/projects.py

Lines 1163-1171

  1163             # Not on the wire for this op (Orca's request has no updated_at field at
  1164             # all) — MLRun stamps its own local time for when it applied the change.
  1165             project.status.updated_at = datetime.datetime.now(datetime.UTC)
  1166             if existing:
! 1167                 self.store_project(session, name, project)
  1168             else:
  1169                 self.create_project(session, project)
  1170             return project
  1171         finally:

Lines 1231-1239

  1231                 incoming_op_id=op_id,
  1232                 prev_op_id=prev_op_id,
  1233             )
  1234             if outcome == follower_contract.ReplayOutcome.replay:
! 1235                 return existing
  1236             # No project payload on this call, and updated_at isn't on the wire at
  1237             # all — MLRun stamps its own local time for when it applied the change.
  1238             updated_at = datetime.datetime.now(datetime.UTC)
  1239             self.patch_project(

Lines 1315-1323

  1315                 incoming_op_id=op_id,
  1316                 prev_op_id=prev_op_id,
  1317             )
  1318             if outcome == follower_contract.ReplayOutcome.replay:
! 1319                 return existing
  1320             # No updated_at on the wire for this op either — MLRun stamps its own
  1321             # local time for when it applied the change.
  1322             updated_at = datetime.datetime.now(datetime.UTC)
  1323             # The common set only — labels/annotations/owner/description. Everything else

Lines 1394-1403

  1394     def _decode_project_states_cursor(cursor: str) -> tuple[datetime.datetime, str]:
  1395         updated_at_str, _, name = cursor.partition("|")
  1396         try:
  1397             updated_at = datetime.datetime.fromisoformat(updated_at_str)
! 1398         except ValueError as exc:
! 1399             raise mlrun.errors.MLRunBadRequestError(
  1400                 f"Invalid list-states cursor: {cursor}"
  1401             ) from exc
  1402         return updated_at, name

@liranbg liranbg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this PR is needless - we always ship mlrun + nuclio
and we expect mlrun to ensure its artifacts created on nuclio to be deleted as well, thus, when we delete a project on mlrun, we will wait to make sure nuclio deleted its project as well, that can be safe and idempotent.

@daanvdk-mck

Copy link
Copy Markdown
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants