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

userdata: add tests for write_files in containers #918

Merged
merged 1 commit into from
Nov 6, 2023

Conversation

europaul
Copy link
Contributor

@europaul europaul commented Nov 3, 2023

Since we added support for write_files in cloud-init configuration for containers - here is a test that will check that functionality, including whether the cloud-init config is reapplied at the containers's reboot

@europaul europaul force-pushed the cloud-config-container-test branch from bfc32e1 to aaa7a3e Compare November 3, 2023 21:52
eden pod deploy -n eclient --memory=512MB --networks=n1 {{template "eclient_image"}} -p {{$port}}:22 --metadata={{$userdata_file}}
test eden.app.test -test.v -timewait 20m RUNNING eclient

exec sleep 3
Copy link
Contributor

Choose a reason for hiding this comment

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

I would recommend to avoid this sleep and instead make test_injected_file.sh to retry few times (with small sleep between retries) until it succeeds or timeouts. Something like this but with smaller sleep time. Github runners can be slow, meaning that hard-coded sleep time may not be enough (at the same time we do not want to put long unconditional sleeps into tests).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks! I will do it. it also seems to solve the problem of ssh refusing the connection by simply retrying

        > exec -t 2m bash test_injected_file.sh "after_restart"
        [stdout]
        /home/paul/zededa/eden/dist/bin/eden sdn fwd eth0 2223 -- ssh -o ConnectTimeout=10 -o StrictHostKeyChecking=no -i /home/paul/zededa/eden/dist/tests/eclient/image/cert/id_rsa root@FWD_IP -p FWD_PORT grep -q "after_restart" /etc/injected_file.txt
        Try 1
        time="2023-11-06T14:15:39+01:00" level=fatal msg="command ssh failed: exit status 255"
        Try 2
        Success
        [stderr]
        Connection timed out during banner exchange
        Connection to 127.0.0.1 port 2223 timed out

Since we added support for write_files in cloud-init configuration for
containers - here is a test that will check that functionality,
including whether the cloud-init config is reapplied at the containers's
reboot

Signed-off-by: Paul Gaiduk <[email protected]>
@europaul europaul force-pushed the cloud-config-container-test branch from aaa7a3e to 0ffcc67 Compare November 6, 2023 13:38
@europaul europaul marked this pull request as ready for review November 6, 2023 13:39
@europaul europaul requested a review from uncleDecart as a code owner November 6, 2023 13:39
Copy link
Member

@uncleDecart uncleDecart left a comment

Choose a reason for hiding this comment

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

LGTM! Congratulations @europaul on your first eden test 🎉

@uncleDecart uncleDecart merged commit 8badd25 into lf-edge:master Nov 6, 2023
6 of 10 checks passed
@giggsoff
Copy link
Collaborator

giggsoff commented Nov 6, 2023

@uncleDecart seems I come quite late, but nice to check CI.

@europaul
Copy link
Contributor Author

europaul commented Nov 6, 2023

@giggsoff this is due to eden pulling eve's tag 10.11.0 here. the changes that are tested in this branch are not included in this EVE version, since they are quite new. in fact the cloud-init support for containers is not part of any EVE tag yet.

@uncleDecart
Copy link
Member

@giggsoff we will bump EVE version once there will be release including cloud-init changes

@giggsoff
Copy link
Collaborator

giggsoff commented Nov 6, 2023

Good to know, thank you. But not sure, that it is good idea to merge changes that are expected to fail the tests. If we really need them to be merged, we can point Eden onto non-tagged version (hash) of eve-os here.

@milan-zededa
Copy link
Contributor

@uncleDecart @europaul It seems this is already picked up by EVE and failing at least in this case: https://github.com/lf-edge/eve/actions/runs/6785428539/job/18443723178?pr=3567#step:3:3314
Also, if test requires newer EVE version to pass, this should be updated to point to that version.

@uncleDecart
Copy link
Member

@milan-zededa you are right, we should not merge tests which are not in EVE version we use for testing. But this tests should not have been picked up, because there is a bug in github-checkout action. I created PR #924 to fix it and if it works, we will merge this to master and backport it to 0.9.2 to avoid this problem.

Alternatively, we can put commit hash to EVE version to test in eden.

@europaul
Copy link
Contributor Author

europaul commented Nov 8, 2023

@milan-zededa thank you for noticing! it's a legitimate bug in my test in https://github.com/lf-edge/eve/actions/runs/6785428539/job/18443723178?pr=3567#step:3:3314. I'm on it!

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