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

Generate settings.metrics.send-metrics based on AWS partition for AWS variants #2247

Merged
merged 4 commits into from
Jul 13, 2022

Conversation

cbgbt
Copy link
Contributor

@cbgbt cbgbt commented Jun 27, 2022

Issue number: Related to #1255

Description of changes:
This set of changes modifies settings.metrics.send-metrics to use a setting-generator for AWS variants. Specifically, we enable metrics-sending by default on AWS variants if the host is in the aws or aws-us-gov partitions, according to the IMDS.

This information is retrieved by shibaken, which required some changes to the interface, as well as the admin user-data generation which uses that interface.

Testing done:

  • Checked that admin container user-data is still generated successfully
  • Checked that send-metrics successfully uses shibaken in the aws partition
  • Checked that send-metrics successfully uses shibaken in the aws-us-gov partition
  • Tested migrations with a test TUF repo

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.

@jpculp
Copy link
Member

jpculp commented Jun 28, 2022

It might be a good idea to split metrics.toml into three: metrics-aws.toml for aws-specific defaults, metrics-public.toml for non-aws defaults, and keep metrics.toml for shared values.

@cbgbt cbgbt marked this pull request as ready for review June 29, 2022 15:28
@@ -0,0 +1,71 @@
/// This module contains utilities for populating userdata for the admin-container with user SSH keys from IMDS.
use clap::Parser;
Copy link
Contributor

Choose a reason for hiding this comment

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

Elsewhere we've tried to use argh for binary size reasons - can we do that here? structopt is also OK since it's currently in updog and metricdog.

Copy link
Contributor

@etungsten etungsten left a comment

Choose a reason for hiding this comment

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

LGMT once existing comments are addressed.

This commit moves prior shibaken functionality to generate admin
container userdata to a subcommand.
Use the new shibaken interface to generate userdata for the admin
container.
AWS variants now generate the `settings.metrics.send-metrics` setting
based on partition information gathered from shibaken.
@cbgbt
Copy link
Contributor Author

cbgbt commented Jul 13, 2022

Addresses comment by @bcressey and rebases.

Moving from clap to argh reduced the aarch64 binary size from 4.1MB to 3.7MB. Not bad!

Copy link
Contributor

@bcressey bcressey left a comment

Choose a reason for hiding this comment

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

Nice!

@cbgbt cbgbt merged commit 529ae84 into bottlerocket-os:develop Jul 13, 2022
@cbgbt cbgbt deleted the send-metrics-partition branch August 15, 2023 23:56
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.

4 participants