-
Notifications
You must be signed in to change notification settings - Fork 519
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
Add aws-ecs-1-nvidia variant #2128
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
logdog.aws-ecs-1.conf |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
# ECS | ||
[services.ecs] | ||
restart-commands = ["/usr/bin/ecs-settings-applier", "/bin/systemctl try-reload-or-restart ecs.service"] | ||
configuration-files = ["ecs-config"] | ||
|
||
[configuration-files.ecs-config] | ||
path = "/etc/ecs/ecs.config" | ||
template-path = "/usr/share/templates/ecs.config" | ||
|
||
[metadata.settings.ecs] | ||
affected-services = ["ecs"] | ||
|
||
[settings.ecs] | ||
allow-privileged-containers = false | ||
logging-drivers = ["json-file", "awslogs", "none"] | ||
loglevel = "info" | ||
|
||
# Metrics | ||
[settings.metrics] | ||
service-checks = ["apiserver", "chronyd", "containerd", "host-containerd", "docker", "ecs"] | ||
|
||
# Network | ||
[metadata.settings.network] | ||
affected-services = ["containerd", "docker", "ecs", "host-containerd", "host-containers"] | ||
|
||
# Image registry credentials | ||
[metadata.settings.container-registry.credentials] | ||
affected-services = ["ecs", "host-containers", "bootstrap-containers"] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
../../../shared-defaults/defaults.toml |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
../../../shared-defaults/aws-host-containers.toml |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
../../../shared-defaults/cf-signal.toml |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
../../../shared-defaults/metrics.toml |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
../../../shared-defaults/docker-services.toml | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file has four parts:
We end up overriding (2) in 53-docker-daemon.toml, and (4) in 52-aws-ecs-1.toml. It works in the sense that we're ending up with the right values, but I wonder if there's a way to refactor this that's easier to follow. If nothing comes to mind, I'm OK with this going in. However, it's close to the ceiling of acceptable complexity because it makes migrations in this area even harder to reason about than usual. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll try to propose something to safely improve these configuration files 👍 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
../../../shared-defaults/ecs.toml |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
../../../shared-defaults/docker-daemon-nvidia.toml |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
../../../shared-defaults/lockdown-none.toml |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
../../../shared-defaults/nvidia-oci-hooks-docker.toml |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
use model_derive::model; | ||
use serde::{Deserialize, Serialize}; | ||
use std::collections::HashMap; | ||
|
||
use crate::modeled_types::Identifier; | ||
use crate::{ | ||
AwsSettings, BootstrapContainer, CloudFormationSettings, ECSSettings, HostContainer, | ||
KernelSettings, MetricsSettings, NetworkSettings, NtpSettings, OciHooks, PemCertificate, | ||
RegistrySettings, UpdatesSettings, | ||
}; | ||
|
||
// Note: we have to use 'rename' here because the top-level Settings structure is the only one | ||
// that uses its name in serialization; internal structures use the field name that points to it | ||
#[model(rename = "settings", impl_default = true)] | ||
struct Settings { | ||
motd: String, | ||
updates: UpdatesSettings, | ||
host_containers: HashMap<Identifier, HostContainer>, | ||
bootstrap_containers: HashMap<Identifier, BootstrapContainer>, | ||
ntp: NtpSettings, | ||
network: NetworkSettings, | ||
kernel: KernelSettings, | ||
aws: AwsSettings, | ||
ecs: ECSSettings, | ||
metrics: MetricsSettings, | ||
pki: HashMap<Identifier, PemCertificate>, | ||
container_registry: RegistrySettings, | ||
oci_hooks: OciHooks, | ||
cloudformation: CloudFormationSettings, | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
../../../shared-defaults/ecs.toml |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
[package] | ||
name = "aws-ecs-1-nvidia" | ||
version = "0.1.0" | ||
edition = "2018" | ||
publish = false | ||
build = "build.rs" | ||
|
||
[package.metadata.build-variant.image-layout] | ||
os-image-size-gib = 4 | ||
|
||
[package.metadata.build-variant] | ||
kernel-parameters = [ | ||
"console=tty0", | ||
"console=ttyS0,115200n8", | ||
] | ||
included-packages = [ | ||
# core | ||
"release", | ||
"kernel-5.10", | ||
# docker | ||
"docker-cli", | ||
"docker-engine", | ||
"docker-init", | ||
"docker-proxy", | ||
# ecs | ||
"ecs-agent", | ||
# NVIDIA support | ||
"ecs-gpu-init", | ||
"nvidia-container-toolkit", | ||
"kmod-5.10-nvidia-tesla-470" | ||
] | ||
|
||
[lib] | ||
path = "lib.rs" | ||
|
||
[build-dependencies] | ||
# core | ||
release = { path = "../../packages/release" } | ||
kernel-5_10 = { path = "../../packages/kernel-5.10" } | ||
# docker | ||
docker-cli = { path = "../../packages/docker-cli" } | ||
docker-engine = { path = "../../packages/docker-engine" } | ||
docker-init = { path = "../../packages/docker-init" } | ||
docker-proxy = { path = "../../packages/docker-proxy" } | ||
# ecs | ||
ecs-agent = { path = "../../packages/ecs-agent" } | ||
# NVIDIA | ||
ecs-gpu-init = { path = "../../packages/ecs-gpu-init" } | ||
nvidia-container-toolkit = { path = "../../packages/nvidia-container-toolkit" } | ||
kmod-5_10-nvidia = { path = "../../packages/kmod-5.10-nvidia" } |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
use std::process::{exit, Command}; | ||
|
||
fn main() -> Result<(), std::io::Error> { | ||
let ret = Command::new("buildsys").arg("build-variant").status()?; | ||
if !ret.success() { | ||
exit(1); | ||
} | ||
Ok(()) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
// not used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking but I'm slightly concerned about expanding this since users who create custom variants with names that contain any of these substrings will accidentally include packages they might not need. I wonder if we should tokenize the variant tuple into fields with
awk
then checking the fields.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great suggestion! Maybe we should have shared macros with your idea so that we don't increase the verbosity here.