-
Notifications
You must be signed in to change notification settings - Fork 16
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 bottlerocket admin and control images, custom host and bootstrap containers support #11
Conversation
…containers support
type BottlerocketHostContainer struct { | ||
// Name is the host container name that will be given to the container in BR's `apiserver` | ||
// +kubebuilder:validation:Required | ||
Name string `json:"name"` |
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.
Name, Image, and UserData can technically be shared between the two structs as a separate one that is inline right? It will help us avoid having to change those fields in two places
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.
yea.. i don't feel strongly abt combining them just bc they share same fields - besides this is consistent w the capi BR fields. i'd prefer to leave it as it is unless we feel necessary to change it
// Bootstrap containers configured with essential = true will stop the boot process if they exit code is a non-zero value. | ||
// Default is false. | ||
// +optional | ||
Essential bool `json:"essential"` |
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.
Superpowered and Essential are terms used in BR?
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.
@@ -14,12 +14,36 @@ import ( | |||
) | |||
|
|||
const ( | |||
adminContainerInitTemplate = `{{ define "adminContainerInitSettings" -}} |
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.
What's happening to the admin container then? I see that we are introducing a field AdminImage
above so how is that related to here.
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.
admin container is part of the hostContainers. We now loop through the host containers (admin, control, kubeadm-bootstrap) and generate the template
{ | ||
Name: "admin", | ||
Superpowered: true, | ||
Image: config.BottlerocketConfig.AdminImage, |
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.
Oh I see, it is being decoupled here. Which is why we need AdminImage at a higher level separately now?
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.
yup
Issue #, if available:
aws/eks-anywhere#3283
Description of changes:
To provision unstacked Etcd node on Bottlerocket OS for snow, we need override the default admin, control image for BR AWS variant. Also need to pass in the Snow BR bootstrap container setting similar to CAPI: aws/eks-anywhere-build-tooling#1599.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.