add bootstrap kubernetes labels and validator management structs by gregcusack · Pull Request #9 · anza-xyz/validator-lab

gregcusack · 2024-04-01T20:53:26Z

Summary of Changes

create bootstrap validator kubernetes labels
Address @yihau 's comments from PR add and deploy bootstrap secrets #8 , hardcode scripts into rust so we avoid having to clone this repo in order to run.
Add library.rs and validator.rs to simplify the handling of various validators, their types, and their labels.
Update to latest version of rustls
Add three more spl args I had left out
Make secret creation more flexible

yihau

🪖

joncinque

Just some small comments, this is all going in the right direction!

joncinque · 2024-04-25T13:31:36Z

+        validators
+            .into_iter()
+            .collect::<Vec<_>>() // Collect into Vec and thread push
+            .par_iter()
+            .try_for_each(|validator| Self::push_image(validator.image()))


Since push_image is using Command::spawn internally, it creates a new process anyway, so the par_iter is a little overkill. You could have push_image return the process::Child and then iterate through them here with wait_with_output, and completely remove rayon

joncinque · 2024-04-25T13:35:36Z

+    let mut data: BTreeMap<String, ByteString> = BTreeMap::new();
+    for (label, value) in secrets {
+        match value {
+            SecretType::Value { v } => {
+                data.insert(label, ByteString(v.into_bytes()));
+            }
+            SecretType::File { path } => {
+                let file_content = std::fs::read(&path)
+                    .map_err(|err| format!("Failed to read file '{:?}': {}", path, err))?;
+                data.insert(label, ByteString(file_content));
+            }
+        }


nit: you might be able to do this in one statement by collecting into a BTreeMap, ie:

let data = secrets.into_iter().map(|(label_value)| ... (label, ByteString(v.into_bytes()))).collect::<BTreeMap<_, _>>();

joncinque · 2024-04-25T13:36:52Z

+pub fn create_secret(
    secret_name: &str,
-    key_files: &[(PathBuf, &str)], //[pathbuf, key type]
+    secrets: HashMap<String, SecretType>,


nit: Since Secret takes a BTreeMap, you could just pass one in here directly

joncinque · 2024-04-25T13:41:52Z

+#[derive(Debug)]
+struct DockerPushThreadError {
+    message: String,
+}
+
+impl Display for DockerPushThreadError {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.message)
+    }
+}
+
+impl Error for DockerPushThreadError {}
+
+impl From<String> for DockerPushThreadError {
+    fn from(message: String) -> Self {
+        DockerPushThreadError { message }
+    }
+}
+
+impl From<&str> for DockerPushThreadError {
+    fn from(message: &str) -> Self {
+        DockerPushThreadError {
+            message: message.to_string(),
+        }
+    }
+}


nit: It might be easier to use thiserror https://docs.rs/thiserror/latest/thiserror/ to define this error. It could become as simple as:

#[derive(Debug, Error)] #[error(transparent)] struct DockerPushThreadError(#[from] String);

i am learning so many things about rust! thanks Jon!

turns out i can actually just fully remove this whole thing based on the suggested change to push_image()

Even better, no code is the best code 🧘

joncinque · 2024-04-25T13:43:15Z

 ) -> Result<(), Box<dyn std::error::Error>> {
    let so_filename = format!("spl_{}-{}.so", name.replace('-', "_"), version);
    let so_path = solana_root_path.join(&so_filename);
+    let so_name = format!("spl_{}.so", name.replace('-', "_"));


Sorry this is all a bit confusing!

ya it's a little weird since the url we are getting uses the GenesisProgram name with both hyphens and underscores

Sorry, I meant that more as "sorry that we made this confusing for you" 😅

e.g.: https://github.com/solana-labs/solana-program-library/releases/download/token-2022-v1.0.0/spl_token_2022.so uses the name token-2022 and token_2022

ohhhhh haha ok nvm lol

joncinque · 2024-04-25T13:46:42Z

+// 3) RPC Node -> One image for each RPC node (not implemented yet)
+// 4) Clients -> Each client has its own image (not implemented yet)
+
+pub struct Library {


nit: why the name Library? Seems like this is just all the machines / nodes / validators, so maybe a clearer name would help?

ya def not the best name. i will change.

joncinque · 2024-04-25T13:47:17Z

+// 4) Clients -> Each client has its own image (not implemented yet)
+
+pub struct Library {
+    validators: Vec<Option<Validator>>,


This is a bit strange... how about separating the bootstrap validator into an Option<Validator> and then having validators as a Vec<Validator>?

I agree it is a little weird. but it's basically used to store images and associated data. bootstrap, standard, rpc validators all use their own image for all validator types deployed. So a bootstrap, standard validator, and RPC would each only appear once in this Library. But for a Client, each client uses a different image, so that is why there is a vector of clients. This was the desired state with everything implemented: https://github.com/anza-xyz/validator-lab/pull/36/files#diff-47af127d675e3dc21b43c34222f42b9290334e88eb5c7ae0fc8b7539e3ecb794. But it may not be super clear.

i can change this to something like:

pub struct ValidatorImages { //struct name tbd bootstrap: Option<Validator>, standard: Option<Validator>, rpc: Option<Validator>, client: Vec<Validator>, }

Note: we don't need a vector for standard or rpc since we only need one image per set of validators. Although i could change this if we want to store refactor a little more.

Do something like:

pub struct Cluster { //struct name tbd bootstrap: Option<Validator>, standard: Vec<Validator>, rpc: Vec<Validator>, client: Vec<Validator>, }

and then import the kubernetes.rs functionality here. so in main we could do something like:

cluster.create_and_deploy_validators()

^ and this would create all the labels, selectors, secrets, replicasets, etc for the non-bootstrap validators. and then would deploy all the validators.

And can do the same for bootstrap, rpc, clients.

I like that first option, it seems clearest to me at least.

And maybe we can split the difference on the name, and go with ClusterImages since the client isn't a validator? But I like both of the options 😄

ok cool sounds good! and yes true

joncinque · 2024-04-25T13:48:59Z

+
+pub struct Library {
+    validators: Vec<Option<Validator>>,
+    _clients: Option<Vec<Validator>>,


nit: it's up to you, but the Option part might be confusing, since there are two ways to represent no clients. Meaning, what's the difference between None and Some(vec![])?

… and update struct

* wip. adding replicaset but need validator config flags * add validator config * add pod requests. need for scheduling * create bootstrap validator replicaset * update token-2022 to v1.0.0: anza-xyz/agave#995 * address jon nits in #9 part 1. * jon comments from: #9 part 2. rename Library to ClusterImages and update struct * address Jon nits

gregcusack force-pushed the bootstrap-labels branch from 9d1d622 to 5495c3a Compare April 1, 2024 21:01

gregcusack mentioned this pull request Apr 1, 2024

Create bootstrap replicaset #10

Merged

gregcusack force-pushed the bootstrap-labels branch from 5495c3a to 5402d0e Compare April 10, 2024 04:45

add bootstrap validator selector

f025ae7

gregcusack force-pushed the bootstrap-labels branch from 5402d0e to a4b6943 Compare April 18, 2024 22:41

gregcusack mentioned this pull request Apr 18, 2024

add and deploy bootstrap secrets #8

Merged

address chido: hard code startup scripts

9596709

gregcusack force-pushed the bootstrap-labels branch from a4b6943 to 9596709 Compare April 18, 2024 23:21

gregcusack requested review from joncinque and yihau April 18, 2024 23:22

add library and validator structs

8d5f9a3

gregcusack changed the title ~~add bootstrap kubernetes labels~~ add bootstrap kubernetes labels and validator management structs Apr 19, 2024

gregcusack added 4 commits April 19, 2024 19:05

upgrade rustls

d5a071a

address Jon comments from PR anza-xyz#7

68ac58f

Jon nit from PR anza-xyz#8. rm build_path

565c8f0

chido. change up secret handling to be more flexible

146a546

gregcusack requested review from joncinque and yihau and removed request for joncinque and yihau April 22, 2024 16:53

yihau approved these changes Apr 23, 2024

View reviewed changes

gregcusack merged commit 87712b9 into anza-xyz:main Apr 23, 2024

joncinque reviewed Apr 25, 2024

View reviewed changes

gregcusack added a commit to gregcusack/validator-lab that referenced this pull request Apr 25, 2024

jon comments from: anza-xyz#9 part 2. rename Library to ClusterImages…

9f72e80

… and update struct

gregcusack added a commit to gregcusack/validator-lab that referenced this pull request Apr 25, 2024

address jon nits in anza-xyz#9 part 1.

6b69de3

gregcusack added a commit to gregcusack/validator-lab that referenced this pull request Apr 25, 2024

jon comments from: anza-xyz#9 part 2. rename Library to ClusterImages…

3351061

… and update struct

gregcusack mentioned this pull request Apr 29, 2024

Create Validator struct and refactor #11

Closed

gregcusack deleted the bootstrap-labels branch May 24, 2024 19:05

Conversation

gregcusack commented Apr 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of Changes

Uh oh!

yihau left a comment

Choose a reason for hiding this comment

Uh oh!

joncinque left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gregcusack Apr 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gregcusack commented Apr 1, 2024 •

edited

Loading

gregcusack Apr 25, 2024 •

edited

Loading