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 support for writing files to containers from cloud-init config #3520

Merged

Conversation

europaul
Copy link
Contributor

@europaul europaul commented Oct 24, 2023

We are extending the support for the cloud-init config for containers. While the old config format with

ENV1=value1
ENV2=value2

still remains available we also introduce the support for the original cloud-init config syntax with support for fields runcmd (only for defining envs) and write_files, like in the following example

#cloud-config
runcmd:
  - ENV1=value1
  - ENV2=value2

write_files:
 - path: /etc/injected_file.txt
   owner: root:root
   permissions: '0644'
   encoding: b64
   content: YmxhYmxh

The section runcmd will only be used for providing envs, while the new section write_files can be used to write files to the containers in the usual cloud-init way. The option to change the owner is however not supported at the moment.

The rest of the field of the cloud-init configuration will be ignored.

pkg/pillar/go.mod Outdated Show resolved Hide resolved
Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

We do need to support the old format since the users have existing cloud-init configured for their edge applications.
So need to detect which format is in use to handle both.

@europaul europaul force-pushed the write-files-to-containers-form-cloud-init branch from 9b7ee46 to fe0d37e Compare October 25, 2023 19:28
@europaul
Copy link
Contributor Author

brought back the support for old cloud-init format and fixed unit tests.
I also intend to add integration tests in eden to test the new functionality

@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

Attention: 107 lines in your changes are missing coverage. Please review.

Comparison is base (ccbcc7d) 20.51% compared to head (e71e15e) 20.53%.
Report is 35 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3520      +/-   ##
==========================================
+ Coverage   20.51%   20.53%   +0.02%     
==========================================
  Files         208      209       +1     
  Lines       45381    45512     +131     
==========================================
+ Hits         9308     9345      +37     
- Misses      35390    35479      +89     
- Partials      683      688       +5     
Files Coverage Δ
pkg/pillar/types/domainmgrtypes.go 0.00% <ø> (ø)
pkg/pillar/utils/cloudconfig/cloudconfig.go 57.62% <57.62%> (ø)
pkg/pillar/cmd/domainmgr/domainmgr.go 0.40% <2.38%> (-0.05%) ⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@eriknordmark
Copy link
Contributor

You can ignore those yetus complaints. It is supposed to only look at lines/functions which were changed in your PR but that doesn't always work.

@europaul europaul force-pushed the write-files-to-containers-form-cloud-init branch from fe0d37e to f3f5f19 Compare October 27, 2023 11:57
Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

Still some minor issues, but approving so we can get some test results.

@europaul europaul force-pushed the write-files-to-containers-form-cloud-init branch from f3f5f19 to f9b4bc7 Compare October 30, 2023 14:01
@europaul
Copy link
Contributor Author

added tests for the new cloudconfig package

@europaul europaul force-pushed the write-files-to-containers-form-cloud-init branch 2 times, most recently from b14e87d to 24407e5 Compare October 30, 2023 15:37
We are adding the support for "write_files" section in cloud-init config
for containers. Also the definition of envs is moved to the section
"runcmd" with lines like "- VAR=value",
however the old syntax of "VAR=value"
is still supported. The rest of cloud-init config is ignored.

Signed-off-by: Paul Gaiduk <paulg@zededa.com>
@europaul europaul force-pushed the write-files-to-containers-form-cloud-init branch from 24407e5 to b34b102 Compare October 30, 2023 17:40
Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

Can you update the PR description to capture that we do handle the old format.

Also, is there any file in docs or pkg/pillar/docs which specifies the env=value format? Any one discuss cloud-init or userdata where we should add such specification?
It might make sense to rewrite/expand the first paragraph in docs/ECO-METADATA.md for this.

Feel free to merge this and do that doc update in a separate PR if you'd like.

@europaul
Copy link
Contributor Author

@eriknordmark I updated the PR description, but I don't have the write access to the repo to merge.

I think we should create a new doc file docs/CLOUD-INIT.md where we describe how our ECOs (VMs and containers) support cloud-init configuration.

Signed-off-by: Paul Gaiduk <paulg@zededa.com>
@europaul europaul force-pushed the write-files-to-containers-form-cloud-init branch from 44f6b02 to e71e15e Compare October 31, 2023 12:39
@eriknordmark
Copy link
Contributor

I think we should create a new doc file docs/CLOUD-INIT.md where we describe how our ECOs (VMs and containers) support cloud-init configuration.

Makes sense. I'll merge this for now

@eriknordmark eriknordmark merged commit 5efd826 into lf-edge:master Oct 31, 2023
15 of 16 checks passed
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.

2 participants