-
-
Notifications
You must be signed in to change notification settings - Fork 322
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
Allow for using Flatcar Linux instead of Container Linux #209
Conversation
Thanks! I've been thinking about some different ways of allowing Flatcar Linux (and potentially other Container Linux derivatives) around v1.11.x timeframe. Bare-metal is certainly the biggest commitment, so I'd like to explore allowing it on clouds first ideally. For bare-metal, I'm thinking neither Will Flatcar have the same notion of alpha, beta, stable? Interesting the user was kept as "core", but kernel args were renamed. |
@dghubble Thanks. Setting a flavor (flatcar / coreos) in the channel name sounds like a good idea. I'll update my PR just like you do in #211. Flatcar has exactly the same channel names, |
2d563f0
to
12c597e
Compare
Make the bare-metal provider work based on not only Container Linux but also [Flatcar Linux](https://flatcar-linux.org). To use Flatcar Linux, user needs to set a config variable `os_channel` to one of the following variables: `flatcar-stable`, `flatcar-beta`, `flatcar-alpha`. Also create a new profile `fl` for Flatcar Linux, and rename a corresponding config variable `container_linux_version` to `os_version`.
@dghubble Updated, and force-pushed. Is it ok for you? |
@@ -0,0 +1,171 @@ | |||
--- |
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.
I'd like to keep the Ignition for Container Linux derivatives unified. If I allowed Flatcar to drift, I might have to allow future derivatives and I'd like to keep everything aligned for a while
ExecStartPre=/bin/mkdir -p /var/lib/kubelet/volumeplugins | ||
ExecStartPre=/usr/bin/bash -c "grep 'certificate-authority-data' /etc/kubernetes/kubeconfig | awk '{print $2}' | base64 -d > /etc/kubernetes/ca.crt" | ||
ExecStartPre=-/usr/bin/rkt rm --uuid-file=/var/cache/kubelet-pod.uuid | ||
ExecStart=/usr/lib/flatcar/kubelet-wrapper \ |
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.
You have /usr/lib/coreos/kubelet-wrapper
in your image, let's keep using that for now.
@@ -186,8 +186,8 @@ module "bare-metal-mercury" { | |||
# bare-metal | |||
cluster_name = "mercury" | |||
matchbox_http_endpoint = "http://matchbox.example.com" | |||
container_linux_channel = "stable" | |||
container_linux_version = "1632.3.0" | |||
os_channel = "stable" |
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.
coreos-stable
@@ -1,18 +1,39 @@ | |||
locals { | |||
flavor = "${element(split("-", var.os_channel), 0)}" |
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.
One of the nice properties of #211 is that if a user inputs a value like stable
(instead of coreos-stable
), the Terraform error strongly hints at the problem showing the AMI data source and indicating that no matching images were found.
I wouldn't do any contortion to make something similar here, but worth considering.
flavor = "${element(split("-", var.os_channel), 0)}" | ||
channel = "${element(split("-", var.os_channel), 1)}" | ||
|
||
profile = "${local.flavor == "flatcar" ? "fl" : "cl"}" |
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.
Sorry. I was unclear in my review. By separate profile, I was referring to separate Matchbox Profiles. A Matchbox profile defines the initrd, kernel, args, etc. that should be used by matched machines. I was proposing instead of using variables throughout the "container-linux-install" "matchbox_profile", I'd consider defining a separate "flatcar-linux-install" profile.
Wasn't proposing making separate directories for cl and fl
container_linux_channel = "${var.container_linux_channel}" | ||
container_linux_version = "${var.container_linux_version}" | ||
os_channel = "${local.channel}" | ||
os_version = "${var.os_version}" | ||
ignition_endpoint = "${format("%s/ignition", var.matchbox_http_endpoint)}" | ||
install_disk = "${var.install_disk}" | ||
container_linux_oem = "${var.container_linux_oem}" |
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.
Vaguely unrelated to your PR, but this will probably be dropped at some point. So don't worry about the oem setting.
@@ -40,21 +61,21 @@ data "template_file" "container-linux-install-configs" { | |||
} | |||
|
|||
// Container Linux Install profile (from matchbox /assets cache) | |||
// Note: Admin must have downloaded container_linux_version into matchbox assets. | |||
// Note: Admin must have downloaded os_version into matchbox assets. | |||
resource "matchbox_profile" "cached-container-linux-install" { |
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.
You can skip making a flatcar-cached-install profile for now. Without the standard Matchbox setup having an analog of https://github.com/coreos/matchbox/blob/master/scripts/get-coreos, I don't think users will prepare those assets correctly and it'll lead to issues being filed. Let's consider it in a followup.
inline: | | ||
#!/bin/bash -ex | ||
curl --retry 10 "${ignition_endpoint}?{{.request.raw_query}}&os=installed" -o ignition.json | ||
flatcar-install \ |
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.
Hmm, I guess template in coreos vs flatcar in the common container-linux-install.yaml.tmpl (still need to think about renaming the file with all the RH news and shifting names). If the intention is to slowly convert all instances of core to flatcar within the OS, that feels like its going to be painful.
I'm also interested in Flatcar since I learnt of their fork of Container Linux in the wake of the Red Hat takeover :) I might want to switch my clusters over to it (from CL), but haven't investigated yet. |
I'm happy to take this on myself. I'd like to get this running myself and I recognize I'm pretty particular about how its implemented, having written Matchbox too. This PR does provide me excellent context for the handful of naming changes and URLs for Flatcar Linux. |
@dghubble Sorry for late reply. I have been on holiday. |
A new PR #220 will replace this PR. So I'll close this one. |
* Support bare-metal cached_install=true mode with Flatcar Linux where assets are fetched from the Matchbox assets cache instead of from the upstream Flatcar download server * Skipped in original Flatcar support to keep it simple #209
* Support bare-metal cached_install=true mode with Flatcar Linux where assets are fetched from the Matchbox assets cache instead of from the upstream Flatcar download server * Skipped in original Flatcar support to keep it simple poseidon/typhoon#209
* Support bare-metal cached_install=true mode with Flatcar Linux where assets are fetched from the Matchbox assets cache instead of from the upstream Flatcar download server * Skipped in original Flatcar support to keep it simple poseidon#209
Make the bare-metal provider work based on not only Container Linux but also Flatcar Linux. To use Flatcar Linux, user needs to set a config variable
os_channel
to one of the following variables:flatcar-stable
,flatcar-beta
,flatcar-alpha
.Also create a new profile
fl
for Flatcar Linux, and rename a corresponding config variablecontainer_linux_version
toos_version
.