Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

tmp: Do not remove tmp.mount unit file from the rootfs #301

Merged
merged 1 commit into from
May 31, 2019

Conversation

amshinde
Copy link
Member

We should start this unit so that systemd can mount /tmp as
tmpfs.

Fixes #300

Signed-off-by: Archana Shinde [email protected]

We should start this unit so that systemd can mount /tmp as
tmpfs.

Fixes kata-containers#300

Signed-off-by: Archana Shinde <[email protected]>
amshinde added a commit to amshinde/agent-1 that referenced this pull request May 30, 2019
Invoke tmp.mount by adding it to kata-containers.target.
This is not invoked by systemd with current rootfs setup we have
as one of the dependencies for tmp.mount is systemd-remount-fs.service
which depends on /etc/fstab file being present(it is currenty missing)
Instead of adding that file, start the tmp.mount unit by including
it in kata-containers.target

With this and change in os-builder to not delete the tmp.mount unit,
(kata-containers/osbuilder#301) /tmp should now
be writeable for systemd based images.
For initrd this is handled by the agent itself.

Fixes #kata-containers/osbuilder#300

Signed-off-by: Archana Shinde <[email protected]>
Copy link
Contributor

@jodh-intel jodh-intel left a comment

Choose a reason for hiding this comment

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

lgtm.

Note: This needs to land before kata-containers/agent#564.

@jodh-intel
Copy link
Contributor

/test

Copy link
Contributor

@grahamwhaley grahamwhaley left a comment

Choose a reason for hiding this comment

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

lgtm
@amshinde @GabyCT - do we think there is any way we can make a CI test to spot any regression of this in the future? I know that might be a little tricky, as this is outside the container workload space...

@grahamwhaley
Copy link
Contributor

FC CI looks unhappy:

Complete!
INFO: Create symlink to /tmp in /var to create private temporal directories with systemd
INFO: Configure chrony file /rootfs/etc/chrony.conf
/osbuilder/rootfs.sh: line 398: /rootfs/etc/chrony.conf: No such file or directory
Failed at 116: sudo -E AGENT_INIT="${AGENT_INIT}" AGENT_VERSION="${agent_commit}" GOPATH="$GOPATH" USE_DOCKER=true OS_VERSION=${os_version} ./rootfs-builder/rootfs.sh "${distro}"
Failed at 18: "${cidir}/install_kata_image.sh"
+ exit_handler
+ '[' 1 -eq 0 ']'
+ info 'ERROR: test failed'
+ s='ERROR: test failed'
+ echo -e 'INFO: ERROR: test failed\n'
INFO: ERROR: test failed

@amshinde
Copy link
Member Author

@grahamwhaley Just taking a look, did the FC CI just get fixed?

@devimc
Copy link

devimc commented May 31, 2019

@amshinde yup

@devimc devimc merged commit 030cd4d into kata-containers:master May 31, 2019
@amshinde
Copy link
Member Author

@grahamwhaley @GabyCT @devimc We do need to have a test for this. Yes this is going to be tricky.
Maybe we test this by generating a debug rootfs as well and verify the mini-os mounts and services using
a debug console.
We need to be generating a debug rootfs(with debug shell and tracing) in the future from user debuggability point of view as well. Any suggestions?

amshinde added a commit to amshinde/agent-1 that referenced this pull request May 31, 2019
Invoke tmp.mount by adding it to kata-containers.target.
This is not invoked by systemd with current rootfs setup we have
as one of the dependencies for tmp.mount is systemd-remount-fs.service
which depends on /etc/fstab file being present(it is currenty missing)
Instead of adding that file, start the tmp.mount unit by including
it in kata-containers.target

With this and change in os-builder to not delete the tmp.mount unit,
(kata-containers/osbuilder#301) /tmp should now
be writeable for systemd based images.
For initrd this is handled by the agent itself.

Fixes kata-containers#565

Signed-off-by: Archana Shinde <[email protected]>
@chavafg
Copy link
Contributor

chavafg commented May 31, 2019

Seems like this change broke the dmesg log test on all CI:

13:13:39 [1] 
13:13:39 [1] • Failure [13.651 seconds]
13:13:39 [1] check dmesg logs errors
13:13:39 [1] /tmp/jenkins/workspace/kata-containers-tests-ubuntu-18-04-PR/go/src/github.com/kata-containers/tests/integration/docker/run_test.go:277
13:13:39 [1]   Run to check dmesg log errors
13:13:39 [1]   /tmp/jenkins/workspace/kata-containers-tests-ubuntu-18-04-PR/go/src/github.com/kata-containers/tests/integration/docker/run_test.go:294
13:13:39 [1]     should be empty [It]
13:13:39 [1]     /tmp/jenkins/workspace/kata-containers-tests-ubuntu-18-04-PR/go/src/github.com/kata-containers/tests/integration/docker/run_test.go:295
13:13:39 [1] 
13:13:39 [1]     Expected
13:13:39 [1]         <string>: [    0.619595] systemd[72]: tmp.mount: Failed to execute command: No such file or directory
13:13:39 [1]         [    0.619844] systemd[72]: tmp.mount: Failed at step EXEC spawning /usr/bin/mount: No such file or directory
13:13:39 [1]     to be empty
13:13:39 [1] 
13:13:39 [1]     /tmp/jenkins/workspace/kata-containers-tests-ubuntu-18-04-PR/go/src/github.com/kata-containers/tests/integration/docker/run_test.go:305
13:13:39 [1] ------------------------------
13:13:39 [1] 

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not remove the tmp.mount unit from the rootfs
5 participants