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.

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 host-container user-data setting #1244

Merged
merged 1 commit into from
Dec 18, 2020

Conversation

tjkirch
Copy link
Contributor

@tjkirch tjkirch commented Dec 16, 2020

You can set settings.host-containers.NAME.user-data to a valid base64-encoded
string to have the (decoded) data placed in a file accessible to the host
container at /.bottlerocket/host-containers/NAME/user-data.

This is a prerequisite for other upcoming work, but the basic idea is to allow easier customization of host-container agents.

Testing done:

I added this to my instance user data:

[settings.host-containers.admin]
enabled = true
user-data = "aGkgdGhlcmUKaG93IGFyZSB5b3UK"

Here's the result, the basic functionality of this change:

[ec2-user@ip-192-168-19-177 ~]$ cat /.bottlerocket/host-containers/admin/user-data
hi there
how are you

Invalid base64 data is rejected:

[ec2-user@ip-192-168-0-110 ~]$ apiclient -u /settings -m PATCH -d '{"host-containers": {"admin": {"user-data": "lkajsdf"}}}'
Failed PATCH request to '/settings': Status 400 when PATCHing /settings: Json deserialize error: Unable to deserialize into ValidBase64: Invalid base64 input: Invalid last symbol 102, offset 6. at line 1 column 54

To test the migration, I built a 1.0.4 image from the v1.0.4 tag, updated to a 1.0.5 image based on this branch, re-confirmed that the user-data setting worked, downgraded back to 1.0.4, and confirmed the user-data setting was removed and the host came back up OK.

I confirmed a pod runs OK after downgrade/upgrade cycles.

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.

README.md Show resolved Hide resolved
sources/api/host-containers/README.md Outdated Show resolved Hide resolved
You can set settings.host-containers.NAME.user-data to a valid base64-encoded
string to have the (decoded) data placed in a file accessible to the host
container at /.bottlerocket/host-containers/NAME/user-data.
@tjkirch
Copy link
Contributor Author

tjkirch commented Dec 16, 2020

^ This push tries to address @zmrow's wording concerns above.

Copy link
Contributor

@webern webern left a comment

Choose a reason for hiding this comment

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

Looks good. One probing question.


/// Older versions don't know about the user-data settings; we remove them so that old versions
/// don't see them and fail deserialization.
fn backward(&mut self, mut input: MigrationData) -> Result<MigrationData> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason to delete the user-data file(s) that might exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so, but it's an interesting question to leave up for others to consider.

It seems to me that the worst case might be downgrading and still using a host container that understands the user-data file but tries to do something with it that isn't compatible with the older Bottlerocket version..? For example, maybe you're using the user-data to configure a host-container that makes API calls back to the parent, and those calls are no longer valid with the downgraded version. I think, in that case, you would have created an implicit dependency from the container to the host version, and so should remove/downgrade the host container before downgrading Bottlerocket.

I'm hesitant to delete things from host container persistent storage, so I'd like to have a decent reason, or fall back to leaving what (naively) seems to be a harmless file.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a really interesting question and one that has some pretty good arguments both ways. For now I think we should leave the file there.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, I think it's far enough out there as an edge case that it doesn't justify deleting data from the persistent store.

Copy link
Contributor

@zmrow zmrow left a comment

Choose a reason for hiding this comment

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

🥾

@tjkirch tjkirch merged commit 7ea0f1d into bottlerocket-os:develop Dec 18, 2020
@tjkirch tjkirch deleted the host-con-user-data branch December 18, 2020 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants