[forge] rust bindings for indexer/testnet deployer #14547
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
⏱️ 1h 42m total CI duration on this PR
|
rustielin
force-pushed
the
rustielin/forge-indexer
branch
from
48a2dda
to
48b2c4f
Compare
rustielin
changed the title
rustielin
force-pushed
the
rustielin/forge-indexer
branch
from
48b2c4f
to
b9d43fa
Compare
21 tasks
rustielin
force-pushed
the
rustielin/forge-indexer
branch
5 times, most recently
from
cd07380
to
2261509
Compare
rustielin
changed the title
sherry-x
requested review from
ibalajiarun,
aluon,
igor-aptos and
sitalkedia
| RetryableError(String), | ||
| FinalError(String), | ||
| } | ||
|
|
||
| async fn create_namespace( | ||
| /// Does the same as create_namespace and handling the 409, but for any k8s resource T | ||
| pub async fn maybe_create_k8s_resource<T>( |
There was a problem hiding this comment.
We can actually use Dynamic object here and a nice little macro to get the api that you want like so
Then you can get rid of all the template T logic
macro_rules! kube_api {
($client:expr, $t:ty) => {{
let tm = kube::api::TypeMeta::resource::<$t>();
let gvk = kube::api::GroupVersionKind::try_from(&tm).unwrap();
let ar = kube::api::ApiResource::from_gvk(&gvk);
kube::api::Api::<kube::api::DynamicObject>::all_with($client.clone(), &ar)
}};
($client:expr, $t:ty, $namespace:expr) => {{
let tm = kube::api::TypeMeta::resource::<$t>();
let gvk = kube::api::GroupVersionKind::try_from(&tm).unwrap();
let ar = kube::api::ApiResource::from_gvk(&gvk);
kube::api::Api::<kube::api::DynamicObject>::namespaced_with(
$client.clone(),
&$namespace,
&ar,
)
}};
}
There was a problem hiding this comment.
discussed offline. We'll do this in another PR
testsuite/forge.env
Outdated
| @@ -0,0 +1,6 @@ | |||
| # This file is source-d when running Forge | |||
There was a problem hiding this comment.
beautiful and clear!
lets be sure to document this so that people know they can use this nice new interface
NOTE: we need to prevent people from modifying and committing this
There was a problem hiding this comment.
May be .gitignore this file but force add it to this PR, so other's cannot commit it unintentionally in the future.
There was a problem hiding this comment.
I tried to do this earlier but couldn't figure out an elegant solution. The issue is that we want folks to maybe modify and commit this (for their own tests), but never land it.
In run_forge.sh it fails forge after all the tests run, if there's any env vars set here, so I think that should cover all those cases.
rustielin
commented
Sep 13, 2024
perryjrandall
requested changes
Sep 13, 2024
rustielin
force-pushed
the
rustielin/forge-indexer
branch
from
2261509
to
02dad86
Compare
perryjrandall
approved these changes
Sep 17, 2024
rustielin
force-pushed
the
rustielin/forge-indexer
branch
from
02dad86
to
0136244
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
ibalajiarun
reviewed
Sep 30, 2024
| @@ -31,11 +31,10 @@ on: | |||
| required: false | |||
| type: string | |||
| description: The Forge k8s cluster to be used for test | |||
| FORGE_ENABLE_HAPROXY: | |||
There was a problem hiding this comment.
Why remove this option?
There was a problem hiding this comment.
I had to make space for an indexer option. workflow_dispatch has a maximum of 10 inputs unfortunately. We can continue to test HAProxy in continuous forge if necessary, but we can probably remove it from adhoc
testsuite/forge.env
Outdated
| @@ -0,0 +1,6 @@ | |||
| # This file is source-d when running Forge | |||
There was a problem hiding this comment.
May be .gitignore this file but force add it to this PR, so other's cannot commit it unintentionally in the future.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
ibalajiarun
approved these changes
Oct 1, 2024
| } else { | ||
| false | ||
| /// Check if the stateful set labels match the given labels | ||
| fn stateful_set_labels_matches(sts: &StatefulSet, labels: &BTreeMap<String, String>) -> bool { |
There was a problem hiding this comment.
Do we need to handle truncation to 63 characters here, incase we have longer labels?
There was a problem hiding this comment.
We probably won't hit this case since usually we don't autogenerate labels the same way we do with names, since we generally stick with labels for app/instance, etc. Doesn't hurt to add a check I suppose, though
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ Forge suite
|
✅ Forge suite
|
✅ Forge suite
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Extends the Forge k8s backend with
k8s_deployer, which contains rust bindings for invoking Forge Deployers.ForgeDeployerManagercreates k8s jobs using the k8s API that run the deployers, containers that take in customization values and create Forge components in the clusterFORGE_ENABLE_INDEXERAlso write a CLI utility for operators. It tests the whole forge-deployers flow e2e without having to run a test. It spins up the testnet and indexer the same way that the TestRunner does. Run this against any GCP project that has a K8s cluster set up with Forge, and also additional Forge Indexer resources
Misc changes:
kube_apis more DRY by using genericsMockK8sResourceApito track all resources of a particular type in memory, to test more complex casesforge.env.testsuite/run_forge.shwill source this file before running. Folks wanting to run highly customized adhoc Forge runs may commit their customizations toforge.envto a PR branch and run the workflow from there. NOTE: there are no protections at the moment about preventing changes toforge.envto land inmain.Type of Change
Which Components or Systems Does This Change Impact?
How Has This Been Tested?
CI, unit tests, manually via
forge operatorcommandsCanary via adhoc Forge running the workflow from this branch
rustielin/forge-indexer-canary: #14577Key Areas to Review
Checklist