Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

create instances via nodegroup #177

Merged
merged 2 commits into from
Jun 24, 2022

Conversation

gthao313
Copy link
Member

@gthao313 gthao313 commented Mar 31, 2022

Issue number:
#162

Description of changes:
Currently we create bottlerocket nodes via launching instances directly, and in this PR we're going to create bottlerocket node via nodegroups. From this we can specify any arbitrary setting we want. and we can have the node-labels set as a Bottlerocket settings directly.

Tasks

  • Nodegroup creation and termination
  • Launch template creation and termination
  • One-time iam role and profile creation and deletion
  • Mapping iam role and profile to cluster

Testing done:
Test on ipv4 and ipv6 clusters

cargo run --bin integ integration-test --cluster-name bottlerocket-test --region us-west-2 --bottlerocket-version 1.5.0  --arch x86_64 --nodegroup-name=tianhg-test-x64
cargo run --bin integ clean --cluster-name bottlerocket-test --region us-west-2 --nodegroup-name=tianhg-test-x64
cargo run --bin integ integration-test --cluster-name bottlerocket-test --region us-west-2 --bottlerocket-version 1.5.0  --arch arm64 --nodegroup-name=tianhg-test-arm64
cargo run --bin integ clean --cluster-name bottlerocket-test --region us-west-2  --nodegroup-name=tianhg-test-arm64

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@gthao313 gthao313 force-pushed the integ-test-ec2-nodegroup branch 2 times, most recently from 1659c56 to 28854f7 Compare March 31, 2022 21:12
integ/src/nodegroup_provider.rs Outdated Show resolved Hide resolved
integ/src/nodegroup_provider.rs Show resolved Hide resolved
.build()
}

fn userdata(endpoint: &str, cluster_name: &str, certificate: &str) -> String {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably use a TOML serializer with serde for this, since it would be pretty easy to break the TOML formatting with the right input.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean something like below? I tried this but create_launch_template().user_data( input), the input need a valid base64 string. Can I still use TOML serializer to output base64 string?

n userdata(endpoint: &str, cluster_name: &str, certificate: &str) -> ProviderResult<String> {
    let user_data = toml::ser::to_string_pretty(&format!(
        r#"[settings.updates]
            ignore-waves = true
                
            [settings.kubernetes]
            api-server = "{}"
            cluster-name = "{}"
            cluster-certificate = "{}"
        "#,
        endpoint, cluster_name, certificate
    ))
    .context("Unable convert userdata to pretty string")?;

    Ok(user_data)

Copy link
Contributor

@cbgbt cbgbt Apr 12, 2022

Choose a reason for hiding this comment

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

I meant more like create a series of structs like:

#[derive(Serialize)]
struct Settings {
    updates: UpdateSettings,
    kubernetes: KubernetesSettings,
}

#[derive(Serialize)]
struct UpdateSettings {
    ignore_waves: bool,
}

#[derive(Serialize)]
struct KubernetesSettings {
    api_server: String,
    // etc...
}

Then you could do something like:

let settings = Settings {
    updates: UpdateSettings {
        ignore_waves: true,
    },
    kubernetes: KubernetesSettings {
        // etc...
    },
}
toml::ser::to_string_pretty(settings);

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried the method that @cbgbt mentioned above; however, the string formate of userdata which was created by toml::ser::to_string_pretty() can be recognized by 'early-boot-config'. We can't use this method and I think it's better to keep the original method.
Error

[    5.826817] early-boot-config[1483]: Provider error: Unable to serialize settings from instance user data: TOML data did not contain 'settings' section

userdata created by toml

"[\"settings.updates\"]\n
ignore-waves = true\n\n
[\"settings.kubernetes\"]\n
api-server = 'xxxx7.us-west-2.eks.amazonaws.com'\n
cluster-name = 'bottlerocket-test'\n
cluster-certificate = '=xxxxx'\n"

@@ -31,7 +31,7 @@ spec:
spec:
containers:
- name: nginx
image: k8s.gcr.io/nginx-slim:0.8
image: nginx:1.21.6
Copy link
Member Author

Choose a reason for hiding this comment

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

Update nginx image version to 1.21.6 so that able to create arm/amd pods. Old version Nginx doesn't support arm.

@gthao313
Copy link
Member Author

gthao313 commented Jun 8, 2022

Push above address comments, update third party packages.

@gthao313 gthao313 requested review from cbgbt and somnusfish June 8, 2022 00:42
@gthao313 gthao313 force-pushed the integ-test-ec2-nodegroup branch 2 times, most recently from a685435 to 8f11bf3 Compare June 13, 2022 22:43
.build()
}

fn userdata(
Copy link
Member Author

Choose a reason for hiding this comment

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

resolve conflict on this part. The code is from IPV6 PR

@gthao313
Copy link
Member Author

Push above rebase and update aws rust dependencies to 0.13.0

integ/src/main.rs Outdated Show resolved Hide resolved
integ/src/main.rs Outdated Show resolved Hide resolved
integ/src/updater.rs Outdated Show resolved Hide resolved
@@ -31,7 +31,7 @@ spec:
spec:
containers:
- name: nginx
image: k8s.gcr.io/nginx-slim:0.8
image: nginx:1.21.6

Choose a reason for hiding this comment

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

Should you have a different commit for this change?

integ/src/main.rs Outdated Show resolved Hide resolved
integ/src/updater.rs Outdated Show resolved Hide resolved
Update nginx image version to 1.21.6 so that able to create arm/amd pods.
Old version Nginx doesn't support arm.
@gthao313 gthao313 merged commit 4a4bb8a into bottlerocket-os:develop Jun 24, 2022
@gthao313 gthao313 deleted the integ-test-ec2-nodegroup branch June 27, 2022 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants