This is needed because DeleteActor cleans up all snapshots under the
actor's location.
If we were to allow repointing an actor's snapshot location, we risked
leaking historical snapshots upon actor deletion.
Fixes #1168
* Add DV for GoldenSnapshotStatus and truncate its error_message to fit
* Require ExternalSnapshot.actor_template_uid to be a UUID
* Bound ExternalVolumeTemplate.capacity to 32 characters
TagStatus.source_actor_uid is unused, we can add this back if we ever
need it.
#1168
We were also missing some of the DV for TagStatus fields:
* Added maxLength=2048 requirement for ExternalSnapshot.snapshot_uri
(matching in_progress_snapshot_uri)
* Added validation for actor_template_uid
* Added validation for storage_location, matching what we have in
SnapshotsConfig.storage_location
https://github.com/agent-substrate/substrate/issues/1378
We don't have a backwards compatibility requirement yet, so let's clean
this up for now.
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
The tag deletion logic is getting too complex, so let's move it to a
workflow, following what we do for other resources.
#1510#664
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
To build a snapshot URI, we need the external storage location (which is
stored in the actor template), actor atespace and UID (which are stored
in the actor resource). If an actorTemplate was deleted, we'd leak the
in-progress snapshot, because the storage information was gone.
Fixes#1608
- [x] Tests pass
- [ ] Appropriate changes to documentation are included in the PR
#1522 added a resyncInterval parameter to NewActorTemplateReconciler and
#1523 added this call site. They landed without seeing each other, so
`main` doesn't compile.
SEt the resyncInterval to 7s, to match what we have in other tests
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
Fixes#1539
If an actor is `CRASHED`, it can't be resumed/paused/suspended anymore,
it can only be deleted.
An actor deletion will clean up everything under the prefix of
`Actor.external_snapshot.snapshot_uri` and
`Actor.in_progress_snapshot_name` (the latter will be empty if the
underlying worker was deleted).
Before, we risked leaking a snapshot after a worker deletion because we
were clearing the `in_progress_snapshot_name` field. We don't need to
clear it up because the CRASHED state is terminal.
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
Renames the proto message, its status message and scope enum, the five
RPCs and their request and response messages, the actor_snapshot_tag
request fields, and Actor.source_snapshot_tag to source_tag. The store
interface, its Postgres table, the object-storage prefix segment and the
kubectl-ate verbs follow, so nothing keeps the old spelling.
https://github.com/agent-substrate/substrate/issues/664
> It's a good idea to open an issue first for discussion.
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
Fixes#664
This PR implements the idea described in
https://github.com/agent-substrate/substrate/issues/664#issuecomment-5499311489
It does more than Garbage Collection of snapshots, because we also got
rid of the Snapshot resource (from the DB/API).
Now, an external snapshot is owned by a single resource:
- An Actor owns the snapshot it writes at suspend
- A tag owns a copy taken at tag creation,
- An actor cloned from a tag borrows the tag's snapshot until its own
first suspend.
Garbage Collection: whoever created/owns the snapshot is the only one
who ever deletes them:
i.e., if an actor is deleted and it owns a snapshot. The underlying
snapshot is deleted with the actor.
this PR:
- Drops table actor_snapshots
- Keeps table actor_snapshot_tags
- Adds an object copy at tag creation, and an owned versus borrowed
distinction on the Actor
- Adds synchronous external snapshot deletion at actor suspend, at actor
delete, and at tag delete
> It's a good idea to open an issue first for discussion.
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
We were leaking snapshots after actor termination.
Note that this is a temporary fix: it only removes local snapshots in
one node. We should clean up the copies on any other
NodeVmsWithLocalSnapshots. This is fine *as of the day this was written*
because today NodeVmsWithLocalSnapshots has at most one item.
Related to https://github.com/agent-substrate/substrate/issues/668 and
#664
- [x] Tests pass
- [] Appropriate changes to documentation are included in the PR
https://github.com/agent-substrate/substrate/issues/1011
An update containing unknown fields during a RMW indicates there's a
version skew between client and server.
So, the server enforces a policy to fail explicitly when it receives an
unknown proto field: so that it does not silently drop fields that the
user intended to set but the current server version cannot process
This also helps to avoid behaviour drift during rolling upgrades: During
rolling upgrades, different components (like the data plane and control
plane) can be on different versions. If an older server replica . If an
older server replica processes a request coming from a newer client, it
will reject it (it doesn't know how to validate/handle it) and it's the
client's responsibility to retry.
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
* Removed field_mask from the API
* Added a new protoupdate package to handle replacing mutable fields.
This makes sure that unknown fields in the server are not dropped by an
update from a stale/old client.
#1011
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
functional_test.go was also split into one file per resource, to match
what we're doing for the RPC handlers too. See #891
I also needed to make some changes to move the tests to a separate
package:
* `NewAteletDialer` now takes `options`, and `WithDialCredentials` lets
a test build its own transport credentials. The fake atelet is reached
over insecure transport. Didn't change existing callers. This is the
only non-test change.
* Moved a few unit tests that are actually functional tests to the
respective file under functionaltest/
Fixes#891
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
RunContractTests is getting large and resource tests are not always
grouped together. So, let's start with splitting them into separate
functions. As they grow, we may consider moving them to their own files,
like we did for functional tests in
https://github.com/agent-substrate/substrate/pull/918
> It's a good idea to open an issue first for discussion.
- [x] Tests pass
- [ ] Appropriate changes to documentation are included in the PR
#891
No declaration added, removed, or renamed, import sets
unchanged, and every non-header line is identical to what it replaced.
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
No declaration added, removed, or renamed, import sets unchanged, and
every non-header line identical to what it replaced.
https://github.com/agent-substrate/substrate/issues/891
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
Since we only have ListWorkers so far, this is just a file rename. The
rest of the handlers should be added to this file, instead of new
handler-specific files, as per #891
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
This was breaking
`hack/install-ate.sh --delete-all` with
`--delete-demo-autoscaled-workerpool is not supported on GKE`
Also removed the custom usage message for
--deploy-demo-autoscaled-workerpool. This was duplicate with the default
message:
Before:
```
Demo: demo-autoscaled-workerpool
--deploy-demo-autoscaled-workerpool Deploy demo-autoscaled-workerpool
--delete-demo-autoscaled-workerpool Delete demo-autoscaled-workerpool
--deploy-demo-autoscaled-workerpool Deploy autoscaled-workerpool demo (HPA + prometheus-adapter + counter workload)
```
After:
```
Demo: demo-autoscaled-workerpool
--deploy-demo-autoscaled-workerpool Deploy demo-autoscaled-workerpool
--delete-demo-autoscaled-workerpool Delete demo-autoscaled-workerpool
```
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
This is a very similar fix to #829
We're also removing the precondition as the storage layer function
arguments and passing it as a wrapper around the closure functions.
https://github.com/agent-substrate/substrate/issues/763
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
UpdateActor now takes an ActorRef, an ActorPrecondition, and a mutate
callback. The store reads the stored actor inside the WATCH transaction,
checks the precondition against that value, and hands it to mutate,
which edits it in place.
The storage layer retries up to 5 times when a concurrent write
invalidates it, re-running mutate against the newer state
I still need to migrate the other callers of store.UpdateActor to avoid
calling GetActor outside of the closure function.
#763
> It's a good idea to open an issue first for discussion.
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
Fixes#732
* `UpdateActorSnapshot` now carries the resource itself + `update_mask`
* `scope` is now applied via the update mask
* Moved `update_mask` to a separate file, so it can be reused by other
update RPCs.
* Added `uid` and `version` as optional guards.
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
#732
* `UpdateActorRequest` now carries the resource itself + `update_mask`
* `worker_selector` is now applied via the update mask (making it
possible to clear the worker selector)
* `update_mask` is validated against an allowlist of paths. Currently,
only `worker_selector` is allowed.
* Added `uid` and `version` as optional guards.
I'll update UpdateActorSnapshotTag in a follow-up PR
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
This introduces a new demo (`demos/autoscaled-workerpool`) demonstrating
how to dynamically autoscale a WorkerPool using HPA +
`prometheus-adapter` on a local Kind cluster.
As mentioned in https://github.com/agent-substrate/substrate/issues/198,
this approach may be too slow for some use cases, but this is a good
first milestone.
Plus, there are some limitations with scaling workerpools that still
need to be addressed, for example:
- Scale-down can strand paused actors. When a worker pod is removed, a
local-snapshot paused actor keeps its node pin pointing at a node that
may no longer have (free) worker pods.
- Scale-up gives no locality guarantee: If there are multiple
local-snapshot paused actors that can't resume because no worker pods
are available on their node, upscaling will not unblock them: the added
capacity can land anywhere, so the pool can grow without ever placing a
worker where a stranded actor needs it.
- Scale-down picks victims blind to actor assignment (can kill busy
pods).
- [x] Tests pass: n/a
- [x] Appropriate changes to documentation are included in the PR
WorkerPool already served /scale, so `kubectl scale` worked, but a HPA
pointed at one did not: the subresource declared no labelSelectorPath,
and HPA needs a selector to find the pods whose metrics it is averaging
or to compute ambiguous selectors.
https://github.com/agent-substrate/substrate/issues/198
- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR
Luiz Oliveira