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.

Sign up for GitHub

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

Jump to bottom

Migrate to schnauzer v2 for Bottlerocket Configuration Templates #3476

Merged
merged 10 commits into from
Sep 22, 2023
Merged

Migrate to schnauzer v2 for Bottlerocket Configuration Templates #3476

merged 10 commits into from
Sep 22, 2023

Conversation

cbgbt
Copy link
Contributor

@cbgbt cbgbt commented Sep 19, 2023

Issue number:
#3133

Description of changes:
This merges features which have been reviewed on the ootb-settings-extensions feature branch into develop for the next release cycle:

The only changes since those PRs has been a rebase onto develop, which included these modifications:

diff --git a/packages/nvidia-container-toolkit/nvidia-oci-hooks-json b/packages/nvidia-container-toolkit/nvidia-oci-hooks-json
index 647e3345..fad6e2c6 100644
--- a/packages/nvidia-container-toolkit/nvidia-oci-hooks-json
+++ b/packages/nvidia-container-toolkit/nvidia-oci-hooks-json
@@ -1,5 +1,3 @@
-[required-extensions]
-oci-hooks = "v1"
 +++
 {
   "hooks": {
diff --git a/packages/os/oci-default-hooks-json b/packages/os/oci-default-hooks-json
index f8d08de2..ab08bab5 100644
--- a/packages/os/oci-default-hooks-json
+++ b/packages/os/oci-default-hooks-json
@@ -1,5 +1,3 @@
-[required-extensions]
-oci-hooks = "v1"
 +++
 {
   "hooks": {

This also enables CI builds for that feature branch for continued feature development.

Testing done:
See the related PRs for details, but in summary:

  • Large-scale fuzz testing of template rendering on randomly generated settings objects
  • Smoke testing instance launches
  • Extensive unit tests in new code branches.

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.

@bcressey
Copy link
Contributor

The rebase is a bit deceptive in this case because we've added some new templates:

  • the copies in packages/kubernetes-1.28
  • the deprecated setting template in e3bc71d

Not sure if that's an exhaustive list. #3460 is potentially a mid-air collision depending on merge order.

bcressey
bcressey reviewed Sep 19, 2023
View reviewed changes
.github/workflows/build.yml Outdated
@@ -1,7 +1,7 @@
name: Build
on:
pull_request:
branches: [develop]
branches: [develop, ootb-settings-extensions]
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we back out the workflow changes since the branch is going away?

packages/docker-engine/daemon-json Show resolved Hide resolved
@cbgbt
Copy link
Contributor Author

cbgbt commented Sep 20, 2023

@bcressey Nice catch on the rebase misses, I had been looking for changes to files that I had touched.

  • I did my testing using the new oci_defaults, so that should be fine.
  • I'll add commits to handle k8s 1.28 and the deprecated settings template.

@cbgbt
Copy link
Contributor Author

cbgbt commented Sep 20, 2023 •
edited
Loading

I ran the following check for template additions, which were missed by my previous scan for changes during the rebase:

$ git diff -U0 2f7124a52685a342098e5b37c9f4744332f47bd3..origin/develop -Scross_templatedir

(2f7124a was the commit that my branch was previously based on)

The only changes that turned up were the ones mentioned by @bcressey:

  • k8s-1.28 template copies
  • deprecated settings
+%{_cross_templatedir}/kubelet-env
+%{_cross_templatedir}/kubelet-config
+%{_cross_templatedir}/kubelet-kubeconfig
+%{_cross_templatedir}/kubelet-bootstrap-kubeconfig
+%{_cross_templatedir}/kubelet-exec-start-conf
+%{_cross_templatedir}/kubernetes-ca-crt
+%{_cross_templatedir}/kubelet-server-crt
+%{_cross_templatedir}/kubelet-server-key
+%{_cross_templatedir}/credential-provider-config-yaml
+%{_cross_templatedir}/log4j-hotpatch-enabled

I'll chat with @foersleo about the midair collision with #3460.

@cbgbt cbgbt force-pushed the ootb-settings-extensions branch from 06b6f35 to 7e11db2 Compare September 20, 2023 21:56
@cbgbt
Copy link
Contributor Author

cbgbt commented Sep 20, 2023 •
edited
Loading

Ugh, I ruined the diff by rebasing again. But ^ rebases on develop once more, then implements the mentioned changes.

I ran the following on the k8s templates as a check:

$ for f in kubelet-env kubelet-config kubelet-kubeconfig kubelet-bootstrap-kubeconfig kubelet-exec-start-conf kubernetes-ca-crt kubelet-server-crt kubelet-server-key credential-provider-config-yaml; do
> echo "checking $f"
> diff packages/kubernetes-1.28/$f packages/kubernetes-1.27/$f
> done
checking kubelet-env
checking kubelet-config
134a135,136
> featureGates:
>   RotateKubeletServerCertificate: true
checking kubelet-kubeconfig
checking kubelet-bootstrap-kubeconfig
checking kubelet-exec-start-conf
checking kubernetes-ca-crt
checking kubelet-server-crt
checking kubelet-server-key
checking credential-provider-config-yaml

EDIT: I tested these changes by launching a aws-k8s-1.28 and confirming that templates all render successfully, then modifying settings for log4j-hotpatch-enabled and checking the associated config file.

cbgbt added 10 commits September 21, 2023 20:50
@cbgbt
schnauzer: move legacy api behind v1 namespace
2fd9e78
@cbgbt
schnauzer: add schnauzer-v2 settings generator
7cbbc97
@cbgbt
schnauzer: update README
0ba9c3a
@cbgbt
Migrate setting-generators to schnauzer v2
b3b0e1b 
This change also uses a POSIX-shell-like lexer to tokenize
setting-generators in sundog, rather than the previous approach of
splitting on whitespace.
@cbgbt
Add schnauzer-v2 settings generator migrations
d5f623e
@cbgbt
schnauzer: allow optional extensions
5a03c83
@cbgbt
Migrate config file rendering to schnauzer v2
2b3682c 
Bottlerocket invokes thar-be-settings whenever it needs to re-render
configuration file templates defined in the Bottlerocket API. This
change modifies thar-be-settings to use the schnauzer v2 renderer, and
also modifies all existing configuration templates in Bottlerocket to
use the new template format.
@cbgbt
schnauzer: simplify template implementation
496ffd4
@cbgbt
schnauzer: move any_enabled helper to std
9f3e345
@cbgbt
schnauzer: fix helper composition bug
aae7b08 
In order for helpers to be composable, they must implement the
`call_inner` method for the `HelperDef` trait. By failing to implement
this for `SettingExtensionTemplateHelper`, helpers used e.g. in an `if`
block would not return correct results.
@cbgbt cbgbt force-pushed the ootb-settings-extensions branch from 7e11db2 to aae7b08 Compare September 21, 2023 21:50
@cbgbt
Copy link
Contributor Author

cbgbt commented Sep 21, 2023 •
edited
Loading

^ Rebases atop the changes from #3460

Tested by launching an aws-k8s-1.28 node and enabling autoload of a kernel module.

@cbgbt cbgbt requested a review from etungsten September 22, 2023 17:36
@cbgbt cbgbt merged commit a0f0173 into develop Sep 22, 2023
@cbgbt cbgbt deleted the ootb-settings-extensions branch September 22, 2023 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bcressey bcressey bcressey approved these changes

@etungsten etungsten etungsten approved these changes

@stmcginnis stmcginnis Awaiting requested review from stmcginnis

@webern webern Awaiting requested review from webern

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cbgbt @bcressey @etungsten