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

defaults: refactor duplicate settings to shared files #1542

Merged
merged 5 commits into from
Apr 30, 2021

Conversation

tjkirch
Copy link
Contributor

@tjkirch tjkirch commented Apr 29, 2021

Issue number:

Addresses comment #1511 (comment)

Description of changes:

defaults: move containerd-config-toml_basic to shared defaults
    
Only k8s variants need to override this if we set it in the base shared defaults.
defaults: refactor common Kubernetes defaults to shared files
defaults: refactor common Docker defaults to shared files
defaults: add missing metrics settings (default off) to vmware-dev
defaults: move public NTP settings to share between non-AWS variants

This should allow #1511 to remove the need for a one-off defaults.toml file, instead linking to the shared kubernetes-containerd.toml, kubernetes-services.toml, and public-ntp.toml files, and creating a new shared kubernetes-vmware.toml file with the bits that remain.

Testing done:

Ran storewolf to generate defaults.toml files for each variant before and after. (The process orders them alphabetically, not how we order them.)

> md5sum *defaults*toml
f5e4ef9f4ed1bb602d67ff1a9277a68d  aws-dev-defaults-after.toml
f5e4ef9f4ed1bb602d67ff1a9277a68d  aws-dev-defaults-before.toml
dbcc554991a0aefcf082727feb10452a  aws-ecs-1-defaults-after.toml
dbcc554991a0aefcf082727feb10452a  aws-ecs-1-defaults-before.toml
b7af5493a5fec29e147eb48b336c4a63  aws-k8s-1.16-defaults-after.toml
b7af5493a5fec29e147eb48b336c4a63  aws-k8s-1.16-defaults-before.toml
b7af5493a5fec29e147eb48b336c4a63  aws-k8s-1.17-defaults-after.toml
b7af5493a5fec29e147eb48b336c4a63  aws-k8s-1.17-defaults-before.toml
b7af5493a5fec29e147eb48b336c4a63  aws-k8s-1.18-defaults-after.toml
b7af5493a5fec29e147eb48b336c4a63  aws-k8s-1.18-defaults-before.toml
b7af5493a5fec29e147eb48b336c4a63  aws-k8s-1.19-defaults-after.toml
b7af5493a5fec29e147eb48b336c4a63  aws-k8s-1.19-defaults-before.toml
42022fff2dfd346eda33c71a8fd32472  aws-k8s-1.20-defaults-after.toml
42022fff2dfd346eda33c71a8fd32472  aws-k8s-1.20-defaults-before.toml
27d65c85e64fe140266ae0c09e7b9685  vmware-dev-defaults-after.toml
98d7efe96740ea0d65457fe3771e7e2d  vmware-dev-defaults-before.toml

The only difference is in vmware-dev, as expected with the metrics fix:

--- vmware-dev-defaults-before.toml     2021-04-30 10:47:22.277161135 -0700
+++ vmware-dev-defaults-after.toml      2021-04-30 10:48:12.727934759 -0700
@@ -102,8 +102,8 @@
 
 [settings.metrics]
 metrics-url = "https://metrics.bottlerocket.aws/v1/metrics"
-send-metrics = true
-service-checks = ["apiserver", "chronyd", "containerd", "host-containerd"]
+send-metrics = false
+service-checks = ["apiserver", "chronyd", "containerd", "host-containerd", "docker"]
 
 [settings.ntp]
 time-servers = ["2.amazon.pool.ntp.org"]

Built and ran aws-k8s-1.19 and aws-ecs-1 variants and confirmed things were healthy.

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.

@tjkirch tjkirch requested review from webern and bcressey April 29, 2021 21:08
Only k8s variants need to override this if we set it in the base shared defaults.
@tjkirch tjkirch force-pushed the k8s-defaults-dupes branch from fce41b3 to a35809e Compare April 30, 2021 17:15
@tjkirch tjkirch changed the base branch from refactor-ctrd-configs to develop April 30, 2021 17:15
@tjkirch
Copy link
Contributor Author

tjkirch commented Apr 30, 2021

^ Now that #1538 is merged, this rebases on develop and changes the PR base branch to develop.

@tjkirch tjkirch force-pushed the k8s-defaults-dupes branch from a35809e to f9df0ab Compare April 30, 2021 17:50
@tjkirch
Copy link
Contributor Author

tjkirch commented Apr 30, 2021

^ Addresses bcressey's concerns by:

  • Removing kubernetes-{metrics,network}, putting those into kubernetes-aws
  • Adding a commit fixing vmware-dev's metrics settings to match aws-dev (default off, include docker)

Also another one discussed offline:

  • Moves vmware-dev's NTP settings to shared-defaults/public-ntp.toml, so it can be used for other VMware variants.

@tjkirch tjkirch merged commit 43a0a3b into bottlerocket-os:develop Apr 30, 2021
@tjkirch tjkirch deleted the k8s-defaults-dupes branch April 30, 2021 22:01
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.

3 participants