Skip to content

Commit

Permalink
ref(wadm): simplify PutResult WIP
Browse files Browse the repository at this point in the history
Signed-off-by: Márk Kővári <kovarimarkofficial@gmail.com>
  • Loading branch information
markkovari committed Jan 5, 2025
1 parent 77f5bc8 commit bbf5445
Show file tree
Hide file tree
Showing 10 changed files with 120 additions and 33 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-22.04]
nats_version: [2.10.7]
nats_version: [2.10.18]

steps:
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ CARGO_TEST_TARGET ?=

test:: ## Run tests
ifeq ($(shell nc $(NC_FLAGS) -w1 127.0.0.1 4222 || echo fail),fail)
$(DOCKER) run --rm -d --name wadm-test -p 127.0.0.1:4222:4222 nats:2.10 -js
$(DOCKER) run --rm -d --name wadm-test -p 127.0.0.1:4222:4222 nats:2.10.18-alpine -js
$(CARGO) test $(CARGO_TEST_TARGET) -- --nocapture
$(DOCKER) stop wadm-test
else
Expand Down
9 changes: 7 additions & 2 deletions crates/wadm-types/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,18 @@ pub struct PutModelResponse {
pub name: String,
}

#[derive(Debug, Serialize, Deserialize, PartialEq, Eq)]
pub enum PutResultSuccess {
Created,
NewVersion(String),
}

/// Possible outcomes of a put request
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq)]
#[serde(rename_all = "lowercase")]
pub enum PutResult {
Success(PutResultSuccess),
Error,
Created,
NewVersion,
}

/// Summary of a given model returned when listing
Expand Down
12 changes: 7 additions & 5 deletions crates/wadm/src/server/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ use wadm_types::{
api::{
DeleteModelRequest, DeleteModelResponse, DeleteResult, DeployModelRequest,
DeployModelResponse, DeployResult, GetModelRequest, GetModelResponse, GetResult,
ListModelsResponse, PutModelResponse, PutResult, Status, StatusResponse, StatusResult,
UndeployModelRequest, VersionInfo, VersionResponse,
ListModelsResponse, PutModelResponse, PutResult, PutResultSuccess, Status, StatusResponse,
StatusResult, UndeployModelRequest, VersionInfo, VersionResponse,
},
CapabilityProperties, Manifest, Properties,
};
Expand Down Expand Up @@ -139,13 +139,15 @@ impl<P: Publisher> Handler<P> {
return;
}

let resp = PutModelResponse {
let resp: PutModelResponse = PutModelResponse {
// If we successfully insert, the given manifest version will be the new current version
current_version: current_manifests.current_version().to_string(),
result: if current_manifests.count() == 1 {
PutResult::Created
PutResult::Success(PutResultSuccess::Created)
} else {
PutResult::NewVersion
PutResult::Success(PutResultSuccess::NewVersion(
current_manifests.current_version().to_string(),
))
},
name: manifest_name.clone(),
total_versions: current_manifests.count(),
Expand Down
112 changes: 96 additions & 16 deletions tests/api_model_operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,12 @@ async fn test_crud_operations() {

println!("Response: {resp:?}");

assert_put_response(resp, PutResult::Created, "v0.0.1", 1);
assert_put_response(
resp,
PutResult::Success(PutResultSuccess::Created),
"v0.0.1",
1,
);

let raw = tokio::fs::read("./oam/simple1.yaml")
.await
Expand All @@ -165,7 +170,12 @@ async fn test_crud_operations() {
.await;
// This manifest has no version, so it's assigned on as a ULID
let example_version = resp.current_version.clone();
assert_put_response(resp, PutResult::Created, &example_version, 1);
assert_put_response(
resp,
PutResult::Success(PutResultSuccess::Created),
&example_version,
1,
);

// Check that we can get back the manifest
let resp: GetModelResponse = test_server
Expand Down Expand Up @@ -214,7 +224,12 @@ async fn test_crud_operations() {
None,
)
.await;
assert_put_response(resp, PutResult::NewVersion, "v0.0.2", 2);
assert_put_response(
resp,
PutResult::Success(PutResultSuccess::NewVersion("v0.0.2".to_string())),
"v0.0.2",
2,
);

manifest
.metadata
Expand All @@ -227,7 +242,12 @@ async fn test_crud_operations() {
None,
)
.await;
assert_put_response(resp, PutResult::NewVersion, "v0.0.3", 3);
assert_put_response(
resp,
PutResult::Success(PutResultSuccess::NewVersion("v0.0.3".to_string())),
"v0.0.3",
3,
);

// Make sure we still only have 2 manifests
let ListModelsResponse { models: resp, .. } = test_server
Expand Down Expand Up @@ -391,7 +411,12 @@ async fn test_bad_requests() {
.get_response("default.model.put", raw, None)
.await;

assert_put_response(resp, PutResult::Created, "v0.0.1", 1);
assert_put_response(
resp,
PutResult::Success(PutResultSuccess::Created),
"v0.0.1",
1,
);

// https://imgflip.com/memegenerator/195657242/Do-it-again
let raw = tokio::fs::read("./oam/sqldbpostgres.yaml")
Expand Down Expand Up @@ -489,7 +514,12 @@ async fn test_delete_noop() {
.get_response("default.model.put", raw, None)
.await;

assert_put_response(resp, PutResult::Created, "v0.0.1", 1);
assert_put_response(
resp,
PutResult::Success(PutResultSuccess::Created),
"v0.0.1",
1,
);

let resp: DeleteModelResponse = test_server
.get_response(
Expand Down Expand Up @@ -528,7 +558,12 @@ async fn test_invalid_topics() {
.get_response("default.model.put", raw, None)
.await;

assert_put_response(resp, PutResult::Created, "v0.0.1", 1);
assert_put_response(
resp,
PutResult::Success(PutResultSuccess::Created),
"v0.0.1",
1,
);

// Too short
let resp: HashMap<String, String> = test_server
Expand Down Expand Up @@ -610,7 +645,12 @@ async fn test_manifest_parsing() {
// This manifest has no version, so it's assigned on as a ULID
let version = resp.current_version.clone();

assert_put_response(resp, PutResult::Created, &version, 1);
assert_put_response(
resp,
PutResult::Success(PutResultSuccess::Created),
&version,
1,
);

// Test yaml manifest with hint
let raw = tokio::fs::read("./oam/sqldbpostgres.yaml")
Expand All @@ -624,7 +664,12 @@ async fn test_manifest_parsing() {
)
.await;

assert_put_response(resp, PutResult::Created, "v0.0.1", 1);
assert_put_response(
resp,
PutResult::Success(PutResultSuccess::Created),
"v0.0.1",
1,
);

// Test json manifest with hint
manifest
Expand All @@ -640,7 +685,12 @@ async fn test_manifest_parsing() {
)
.await;

assert_put_response(resp, PutResult::NewVersion, "v0.0.2", 2);
assert_put_response(
resp,
PutResult::Success(PutResultSuccess::NewVersion("v0.0.2".to_string())),
"v0.0.2",
2,
);

// Smoke test to make sure the server can handle the various provider config options
let raw = tokio::fs::read("./oam/config.yaml")
Expand All @@ -652,7 +702,12 @@ async fn test_manifest_parsing() {
// This manifest has no version, so it's assigned on as a ULID
let version = resp.current_version.clone();

assert_put_response(resp, PutResult::Created, &version, 1);
assert_put_response(
resp,
PutResult::Success(PutResultSuccess::Created),
&version,
1,
);
}

#[tokio::test]
Expand All @@ -675,7 +730,12 @@ async fn test_deploy() {
.get_response("default.model.put", raw, None)
.await;

assert_put_response(resp, PutResult::Created, "v0.0.1", 1);
assert_put_response(
resp,
PutResult::Success(PutResultSuccess::Created),
"v0.0.1",
1,
);
manifest
.metadata
.annotations
Expand All @@ -687,7 +747,12 @@ async fn test_deploy() {
None,
)
.await;
assert_put_response(resp, PutResult::NewVersion, "v0.0.2", 2);
assert_put_response(
resp,
PutResult::Success(PutResultSuccess::NewVersion("v0.0.2".to_string())),
"v0.0.2",
2,
);

// Try to deploy and undeploy something that doesn't exist
let resp: DeployModelResponse = test_server
Expand Down Expand Up @@ -850,7 +915,12 @@ async fn test_delete_deploy() {
.get_response("default.model.put", raw, None)
.await;

assert_put_response(resp, PutResult::Created, "v0.0.1", 1);
assert_put_response(
resp,
PutResult::Success(PutResultSuccess::Created),
"v0.0.1",
1,
);
manifest
.metadata
.annotations
Expand All @@ -862,7 +932,12 @@ async fn test_delete_deploy() {
None,
)
.await;
assert_put_response(resp, PutResult::NewVersion, "v0.0.2", 2);
assert_put_response(
resp,
PutResult::Success(PutResultSuccess::NewVersion("v0.0.2".to_string())),
"v0.0.2",
2,
);

let resp: DeployModelResponse = test_server
.get_response(
Expand Down Expand Up @@ -942,7 +1017,12 @@ async fn test_status() {
let resp: PutModelResponse = test_server
.get_response("default.model.put", raw, None)
.await;
assert_put_response(resp, PutResult::Created, "v0.0.1", 1);
assert_put_response(
resp,
PutResult::Success(PutResultSuccess::Created),
"v0.0.1",
1,
);

let resp: StatusResponse = test_server
.get_response(
Expand Down
2 changes: 1 addition & 1 deletion tests/docker-compose-e2e_multiple_hosts.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
services:
nats:
image: nats:2.10-alpine
image: nats:2.10.18-alpine
command: ['-js']
ports:
- 4222:4222
Expand Down
8 changes: 4 additions & 4 deletions tests/docker-compose-e2e_multitenant.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
services:
# Set up a NATS core node and two leaf nodes which will connect with basic auth.
nats-core:
image: nats:2.9.16-alpine
image: nats:2.10.18-alpine
ports:
- "127.0.0.1:5222:5222"
- "127.0.0.1:7422:7422"
Expand All @@ -11,7 +11,7 @@ services:
nats-leaf-wadm:
depends_on:
- nats-core
image: nats:2.9.16-alpine
image: nats:2.10.18-alpine
ports:
- "127.0.0.1:4222:4222"
command: ["-js", "-a", "0.0.0.0", "-c", "/nats/config/nats-leaf-wadm.conf"]
Expand All @@ -20,7 +20,7 @@ services:
nats-leaf-a:
depends_on:
- nats-core
image: nats:2.9.16-alpine
image: nats:2.10.18-alpine
ports:
- "127.0.0.1:4223:4222"
command: ["-js", "-a", "0.0.0.0", "-c", "/nats/config/nats-leaf-a.conf"]
Expand All @@ -29,7 +29,7 @@ services:
nats-leaf-b:
depends_on:
- nats-core
image: nats:2.9.16-alpine
image: nats:2.10.18-alpine
ports:
- "127.0.0.1:4224:4222"
command: ["-js", "-a", "0.0.0.0", "-c", "/nats/config/nats-leaf-b.conf"]
Expand Down
2 changes: 1 addition & 1 deletion tests/docker-compose-e2e_shared.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
services:
nats:
image: nats:2.10-alpine
image: nats:2.10.18-alpine
command: ['-js']
ports:
- 4222:4222
Expand Down
2 changes: 1 addition & 1 deletion tests/docker-compose-e2e_upgrades.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
services:
nats:
image: nats:2.10-alpine
image: nats:2.10.18-alpine
command: ['-js']
ports:
- 4222:4222
Expand Down
2 changes: 1 addition & 1 deletion tests/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ pub async fn setup_env() -> Result<TestEnv> {
}

async fn start_nats_server() -> Result<ContainerAsync<GenericImage>> {
GenericImage::new("nats", "2.10.18")
GenericImage::new("nats", "2.10.18-alpine")
.with_exposed_port(DEFAULT_NATS_PORT.into())
.with_wait_for(WaitFor::message_on_stderr("Server is ready"))
.with_cmd(["-js"])
Expand Down

0 comments on commit bbf5445

Please sign in to comment.