platform/aws: add user manipulable raw ign config via user-data local files#3090
platform/aws: add user manipulable raw ign config via user-data local files#3090enxebre wants to merge 1 commit intocoreos:masterfrom
Conversation
|
Can one of the admins verify this patch? |
|
cc @crawford |
enxebre
changed the title
enxebre
changed the title
| { | ||
| "filesystem": "root", | ||
| "group": {}, | ||
| "path": "/etc/kubernetes/kubeconfig", |
There was a problem hiding this comment.
This doesn't seem right. The kubeconfig should be dynamically generated by TNC. This PR doesn't change the interaction though, but I think this should be removed eventually.
There was a problem hiding this comment.
thanks! I agree. It's unrelated to this PR. That would need to go hand in hand with the bootstrap token generation. Is there a place where we track what the TNC should do, and what it actually currently does?
|
I do not think this is the right approach. As t is currently implemented, users edit the very ignition config that we design. If they change the append, or the permissions on the kubeconfig then we are broken. Instead, uses should be allowed to provide an entire ignition config by themselves. If they do, then we append it and our own ignition to the stub. If they do not, then we only append our ignition to the stub. Exposing our ignition is the wrong interface for this and is inviting more problems IMO. Further, this append should really be performed in a step in the Tectonic CLI so that we can use the igntition golang packages to validate the config before sending to user data. Otherwise we will be booting nodes with ignition that was never going to work and this will be much harder to debug. |
|
Why not append using inline url-encoded string? |
|
using So using base64 here which also matches the default format that we are generating via tf for e.g filesystems/files/content |
|
This seems to be caused by discrepancies between tf |
squat
left a comment
squat
left a comment
There was a problem hiding this comment.
Looks fine overall. Just a few questions
| @@ -0,0 +1,3 @@ | |||
| { | |||
| "ignition": { "version": "2.1.0" } | |||
There was a problem hiding this comment.
Rather than make this a file that is always in the repo, shouldn't this be a top level input that users can provide optionally? this would allow us to perform validation in the CLI as well rather than just appending whatever is given.
| subnet_ids = "${module.vpc.master_subnet_ids}" | ||
| ec2_ami = "${var.tectonic_aws_ec2_ami_override}" | ||
| kubeconfig_content = "${local.kubeconfig_kubelet_content}" | ||
| user_ign = "${data.template_file.user_ign_master.rendered}" |
There was a problem hiding this comment.
And here we would do something like
"${var.tectonic_user_ignition_master}"| user_ign = "${data.template_file.user_ign_worker.rendered}" | ||
| } | ||
|
|
||
| data "template_file" "user_ign_master" { |
There was a problem hiding this comment.
and we would not need to template anything here.
There was a problem hiding this comment.
Even if we do not go with the top level config option, this should be
data "local_file" "foo" {
filename = "${path.module}/foo.bar"
}rather than template_file
https://www.terraform.io/docs/providers/local/d/file.html
|
closing in favour of #3120 |
4 participants
add user manipulable raw ign config via user-data local files. Fix INST-906