✨ Add support for Ignition-based bootstrap data and Flatcar Container Linux #2271

invidian · 2021-02-25T16:38:04Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR adds support for handling bootstrap data in Ignition format, which will hopefully be produced by CABPK v1alpha4 implemented by PR kubernetes-sigs/cluster-api#4172 (not merged yet).

As Ignition does not plan to support multi part mime for user-data (coreos/ignition#1072), this PR implements support for putting bootstrap data in Ignition format in S3 bucket, to workaround the 64kb limitation on user-data for EC2 instances.

Similarly to SSM support, bootstrap data is removed after node has successfully bootstrapped.

Right now single S3 bucket is used for each cluster, as we need a single place to control bucket policies, which are used to restrict access for control-plane and worker nodes to only be able to access their own bootstrap data.

Finally, new template is added tailored for Flatcar Container Linux, which contains references to right now unofficial AMIs, though Flatcar support is fully added in image-builder already.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1979
Refs #1875

Special notes for your reviewer:

This PR can't probably be merged at this point yet. We probably need to wait until CAPI PR is merged and released, but as this PR goes in pair with CAPI one, it would be awesome to be able to get some early feedback on the approach taken.

@dongsupark is also working on adding e2e tests for it.

Checklist:

includes documentation
adds unit tests
adds or updates e2e tests

Release note:

Added support for bootstrap data in Ignition format and for Flatcar Container Linux

k8s-ci-robot · 2021-02-25T16:38:12Z

Welcome @invidian!

It looks like this is your first PR to kubernetes-sigs/cluster-api-provider-aws 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/cluster-api-provider-aws has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2021-02-25T16:38:12Z

Hi @invidian. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

MarcelMue

I can't find the time to do a full review, have some comments for now :)

api/v1alpha3/awscluster_types.go

api/v1alpha3/awsmachine_webhook.go

go.mod

richardcase · 2021-03-01T16:29:35Z

/ok-to-test

richardcase

Really looking forward to this change!!

Some initial comments but will take a proper look

api/v1alpha3/awscluster_types.go

api/v1alpha3/awsmachine_types.go

api/v1alpha3/awsmachine_webhook.go

richardcase · 2021-03-03T14:11:03Z

@invidian - we should have some e2e tests around this. Perhaps to keep the change smaller we could do that in a follow-up PR?

invidian · 2021-03-03T15:43:46Z

@invidian - we should have some e2e tests around this. Perhaps to keep the change smaller we could do that in a follow-up PR?

From PR description:

This PR can't probably be merged at this point yet. We probably need to wait until CAPI PR is merged and released, but as this PR goes in pair with CAPI one, it would be awesome to be able to get some early feedback on the approach taken.

@dongsupark is also working on adding e2e tests for it.

However, if you think e2e tests could be added at later stage, that would be awesome :)

randomvariable · 2021-03-05T16:01:23Z

I'll take a look at this early next week. I would really like e2e tests for this as these pieces are non-trivial and have been a frequent source of bugs.

Will this work with vanilla Flatcar AMIs or do we need to build some?

dongsupark · 2021-03-05T16:45:49Z

@randomvariable
e2e tests are still in progress.

We need capa-ami-flatcar-* AMIs instead of vanilla Flatcar ones.
For example, ami-00c62700208b5b333 (capa-ami-flatcar-stable-1.19.7-00-1612279141) in eu-central-1.

randomvariable · 2021-03-05T19:43:49Z

This is all in Image Builder, isn't it? If so, we can start publishing them in the same account as the rest of the AMIs.

invidian · 2021-03-06T14:41:16Z

This is all in Image Builder, isn't it? If so, we can start publishing them in the same account as the rest of the AMIs.

Yes, it's all in upstream Image Builder.

invidian · 2021-03-11T16:43:49Z

@randomvariable I just pushed passing e2e tests. Right now they require custom CAPI images and manifests, as mentioned before. We run tests locally and they all pass.

randomvariable · 2021-03-12T16:15:34Z

@invidian Thanks. I'll take a look, we'll aim to get this into v0.6.5

vincepri

I'm a bit concerned about the amount of changes we need to make to an infrastructure provider to support a new bootstrapper. Have you all thought if this is the best way forward?

api/v1alpha3/awscluster_types.go

docs/book/src/topics/ignition-support.md

As a preparation for using it as an alternative backend to Secret Manager for OS-es, which do not support Secret Manager, like Flatcar Container Linux. Co-authored-by: Dongsu Park <[email protected]> Signed-off-by: Mateusz Gozdek <[email protected]>

It will be consumed by new S3 service implementing S3 access for creating bootstrapping data for systems, which do not support pulling user data from Secret Manager, like Ignition. Co-authored-by: Dongsu Park <[email protected]> Signed-off-by: Mateusz Gozdek <[email protected]>

This commit adds initial implementation of S3 service, which will be used to store bootstrap data for nodes, which do not support pulling them from Secret Manager, like ones using Ignition as bootstrap system. This commit also adds a function for creating a real S3 client similar to other functions in this package. Co-authored-by: Dongsu Park <[email protected]> Signed-off-by: Mateusz Gozdek <[email protected]>

When S3Bucket.Enabled is true, cluster controller will create an S3 bucket, by default with cluster name as a bucket name, where machine controller will be able to put userdata for systems, which do not support pulling them from Secret Manager, like Ignition. When cluster is deleted, bucket will be removed as well. Co-authored-by: Dongsu Park <[email protected]> Signed-off-by: Mateusz Gozdek <[email protected]>

This commit adds new Ignition block to AWSMachineSpec struct, which will allow different way of handling user data. If either bootstrap data has format defined as Ignition or user explicitly specify to use Ignition as a bootstrap format, machine controller will handle things accordingly. Co-authored-by: Dongsu Park <[email protected]> Signed-off-by: Mateusz Gozdek <[email protected]>

This commit finalizes addition of Ignition support as bootstrap data format. Co-authored-by: Dongsu Park <[email protected]> Signed-off-by: Mateusz Gozdek <[email protected]>

Co-authored-by: Dongsu Park <[email protected]> Signed-off-by: Mateusz Gozdek <[email protected]>

This commit adds the feature gate BootstrapFormatIgnition that will control the usage of field `ignition` in AWSMachine & AWSMachineTemplate and `s3Bucket` in AWSCluster. If user provides `ignition` field and/or `s3Bucket` without setting the feature gate then the webhook rejects the request with a validation error. Signed-off-by: Suraj Deshmukh <[email protected]>

Signed-off-by: Mateusz Gozdek <[email protected]>

Mainly so it can be referenced in the book. Signed-off-by: Mateusz Gozdek <[email protected]>

docs/book/src/topics/ignition-support.md

Signed-off-by: Mateusz Gozdek <[email protected]>

k8s-ci-robot · 2022-03-29T10:09:59Z

@invidian: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cluster-api-provider-aws-apidiff-main	`0db23d4`	link	false	`/test pull-cluster-api-provider-aws-apidiff-main`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

richardcase · 2022-03-29T10:16:14Z

Thanks for the docs and subsequent small changes @invidian.

Based on the conversation ^^^^ and the agreement to follow up on certain items as PRs after this, from my side:

/lgtm

To make kubernetes-sigs#2271 CI pass, as current Flatcar AMIs for 1.23.0 are built using older image-builder version which we are not compatible with. Signed-off-by: Mateusz Gozdek <[email protected]>

sedefsavas · 2022-03-29T16:24:28Z

Thanks all, this could be the largest PR that is going in for a while.

/approve

k8s-ci-robot · 2022-03-29T16:24:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sedefsavas

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [sedefsavas]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

invidian · 2022-03-29T17:19:55Z

Thanks a lot for approving @sedefsavas! I'm really happy this work didn't get stale and eventually got merged.

In case you want me to address unresolved conversations, they are linked in this comment: #2271 (comment). They got wrapped by GitHub, so they are not easy to reach anymore.

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. labels Feb 25, 2021

k8s-ci-robot requested review from michaelbeaumont and vincepri February 25, 2021 16:38

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 25, 2021

invidian changed the title ~~Add support for Ignition-based bootstrap data and Flatcar Container Linux support~~ ✨ Add support for Ignition-based bootstrap data and Flatcar Container Linux support Feb 25, 2021

This was referenced Feb 25, 2021

Support userdata privacy beyond cloudinit #1875

Closed

Support Flatcar linux for nodes #1979

Closed

MarcelMue reviewed Feb 26, 2021

View reviewed changes

api/v1alpha3/awscluster_types.go Outdated Show resolved Hide resolved

api/v1alpha3/awscluster_types.go Outdated Show resolved Hide resolved

api/v1alpha3/awsmachine_webhook.go Outdated Show resolved Hide resolved

go.mod Outdated Show resolved Hide resolved

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 1, 2021

richardcase reviewed Mar 1, 2021

View reviewed changes

api/v1alpha3/awscluster_types.go Outdated Show resolved Hide resolved

api/v1alpha3/awscluster_types.go Outdated Show resolved Hide resolved

api/v1alpha3/awsmachine_types.go Outdated Show resolved Hide resolved

api/v1alpha3/awsmachine_webhook.go Outdated Show resolved Hide resolved

randomvariable added this to the v0.6.5 milestone Mar 12, 2021

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 20, 2021

vincepri suggested changes Mar 23, 2021

View reviewed changes

api/v1alpha3/awscluster_types.go Outdated Show resolved Hide resolved

api/v1alpha3/awscluster_types.go Outdated Show resolved Hide resolved

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 23, 2021

richardcase reviewed Mar 29, 2022

View reviewed changes

docs/book/src/topics/ignition-support.md Outdated Show resolved Hide resolved

invidian and others added 11 commits March 29, 2022 11:52

controllers: add Ignition support

a37ebbe

This commit finalizes addition of Ignition support as bootstrap data format. Co-authored-by: Dongsu Park <[email protected]> Signed-off-by: Mateusz Gozdek <[email protected]>

templates: add Flatcar Container Linux template

bdc7ade

Co-authored-by: Dongsu Park <[email protected]> Signed-off-by: Mateusz Gozdek <[email protected]>

test/e2e: add tests for S3 Bucket and Flatcar support

3156e2e

Co-authored-by: Dongsu Park <[email protected]> Signed-off-by: Mateusz Gozdek <[email protected]>

test/e2e/data/e2e_conf.yaml: enable Ignition feature gate

9502110

Signed-off-by: Mateusz Gozdek <[email protected]>

Add S3 policy as part of the release

933a0e4

Mainly so it can be referenced in the book. Signed-off-by: Mateusz Gozdek <[email protected]>

richardcase reviewed Mar 29, 2022

View reviewed changes

docs/book/src/topics/ignition-support.md Show resolved Hide resolved

docs/book/src: add Ignition support documentation

0db23d4

Signed-off-by: Mateusz Gozdek <[email protected]>

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 29, 2022

invidian requested a review from sedefsavas March 29, 2022 10:17

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 29, 2022

sedefsavas merged commit 4355df7 into kubernetes-sigs:main Mar 29, 2022

k8s-ci-robot modified the milestones: v1.4.0, v1.x Mar 29, 2022

invidian deleted the invidian/flatcar-support-with-unit-tests-and-cloudformation-test branch March 29, 2022 17:19

sedefsavas mentioned this pull request Apr 11, 2022

AWSMachine Reconciler fails with error "invalid secret backend" if insecureSkipSecretsManager: true with latest version 1.4.0 #3402

Closed

bryan-cox mentioned this pull request Oct 28, 2022

Retrieve CAPI/CAPA from release image openshift/hypershift#1826

Merged

4 tasks

t-lo mentioned this pull request Jan 4, 2023

CAPA support flatcar/Flatcar#925

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Add support for Ignition-based bootstrap data and Flatcar Container Linux #2271

✨ Add support for Ignition-based bootstrap data and Flatcar Container Linux #2271

invidian commented Feb 25, 2021 •

edited

Loading

k8s-ci-robot commented Feb 25, 2021

k8s-ci-robot commented Feb 25, 2021

MarcelMue left a comment

richardcase commented Mar 1, 2021

richardcase left a comment

richardcase commented Mar 3, 2021

invidian commented Mar 3, 2021 •

edited

Loading

randomvariable commented Mar 5, 2021

dongsupark commented Mar 5, 2021

randomvariable commented Mar 5, 2021

invidian commented Mar 6, 2021

invidian commented Mar 11, 2021

randomvariable commented Mar 12, 2021

vincepri left a comment

k8s-ci-robot commented Mar 29, 2022

richardcase commented Mar 29, 2022

sedefsavas commented Mar 29, 2022

k8s-ci-robot commented Mar 29, 2022

invidian commented Mar 29, 2022

✨ Add support for Ignition-based bootstrap data and Flatcar Container Linux #2271

✨ Add support for Ignition-based bootstrap data and Flatcar Container Linux #2271

Conversation

invidian commented Feb 25, 2021 • edited Loading

k8s-ci-robot commented Feb 25, 2021

k8s-ci-robot commented Feb 25, 2021

MarcelMue left a comment

Choose a reason for hiding this comment

richardcase commented Mar 1, 2021

richardcase left a comment

Choose a reason for hiding this comment

richardcase commented Mar 3, 2021

invidian commented Mar 3, 2021 • edited Loading

randomvariable commented Mar 5, 2021

dongsupark commented Mar 5, 2021

randomvariable commented Mar 5, 2021

invidian commented Mar 6, 2021

invidian commented Mar 11, 2021

randomvariable commented Mar 12, 2021

vincepri left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Mar 29, 2022

richardcase commented Mar 29, 2022

sedefsavas commented Mar 29, 2022

k8s-ci-robot commented Mar 29, 2022

invidian commented Mar 29, 2022

invidian commented Feb 25, 2021 •

edited

Loading

invidian commented Mar 3, 2021 •

edited

Loading