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

Validate annotations that refer to binaries #3005

Merged
merged 30 commits into from Nov 11, 2020

Conversation

c3d
Copy link
Member

@c3d c3d commented Oct 9, 2020

This fixes #3004 and https://bugs.launchpad.net/katacontainers.io/+bug/1878234.

Need to add a link to code reviews once this is merged.

Forward port is kata-containers/kata-containers#902

@c3d
Copy link
Member Author

c3d commented Oct 9, 2020

/test

1 similar comment
@fidencio
Copy link
Member

fidencio commented Oct 9, 2020

/test

@c3d c3d added needs-forward-port Changes need to be applied to a newer branch / repository no-backport-needed Changed do not need to be applied to an older branch / repository labels Oct 12, 2020
@c3d
Copy link
Member Author

c3d commented Oct 12, 2020

/test

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.

Thanks @c3d !

@c3d c3d force-pushed the bug/launchpad-1878234-access branch from 9a75f86 to 09ec09e Compare October 12, 2020 13:42
@c3d
Copy link
Member Author

c3d commented Oct 12, 2020

Rebased on recent master to fix conflict

@c3d
Copy link
Member Author

c3d commented Oct 12, 2020

/retest

@c3d c3d force-pushed the bug/launchpad-1878234-access branch from 09ec09e to b73de39 Compare October 14, 2020 14:22
@c3d
Copy link
Member Author

c3d commented Oct 14, 2020

One more iteration required to add some variables that were missed in makefile: Add missing generated vars to USER_VARS

@c3d
Copy link
Member Author

c3d commented Oct 14, 2020

/retest

@c3d c3d force-pushed the bug/launchpad-1878234-access branch from b73de39 to 770709c Compare October 16, 2020 09:11
@c3d
Copy link
Member Author

c3d commented Oct 16, 2020

Found a typo in PROXYPATH variable, was spelled PROXYPAT. Fixed.

@c3d
Copy link
Member Author

c3d commented Oct 16, 2020

/retest

@jodh-intel
Copy link
Contributor

@c3d - make --warn-undefined-variables (which actually shows a few others in the current master Makefile!). Also, I found https://github.com/mrtazz/checkmake which looks interesting.

@c3d c3d force-pushed the bug/launchpad-1878234-access branch from 770709c to 6a5edcb Compare October 16, 2020 10:00
@c3d
Copy link
Member Author

c3d commented Oct 16, 2020

Last failures apparently need rebasing on master. Pushed that.

@c3d c3d force-pushed the bug/launchpad-1878234-access branch from 6a5edcb to e3722d1 Compare October 16, 2020 10:57
@c3d
Copy link
Member Author

c3d commented Oct 16, 2020

Grumble, one test had been copy-pasted, and so the removal of vc.MinHypervisorMemory was not acted in the added code. Retrying.

@c3d
Copy link
Member Author

c3d commented Oct 16, 2020

/retest

1 similar comment
@c3d
Copy link
Member Author

c3d commented Oct 19, 2020

/retest

@fidencio
Copy link
Member

@c3d, I'm seeing this error accross all the CIs:

Drop caches
/tmp/jenkins/workspace/kata-containers-runtime-opensuse-15-PR/go/src/github.com/kata-containers/runtime
Setup virtcontainers environment
Install virtcontainers
make: Entering directory '/tmp/jenkins/workspace/kata-containers-runtime-opensuse-15-PR/go/src/github.com/kata-containers/runtime/virtcontainers'
     GOBUILD  build
     GOBUILD  hook
     GOBUILD  kata-shim
make: Leaving directory '/tmp/jenkins/workspace/kata-containers-runtime-opensuse-15-PR/go/src/github.com/kata-containers/runtime/virtcontainers'
/usr/local/bin/kata-runtime
Logging kata-env information:
no service name provided
Build step 'Execute shell' marked build as failure
Performing Post build task...
Match found for :.* : True
Logical operation result is TRUE
Running script  : #!/bin/bash

export GOPATH=$WORKSPACE/go
export GOROOT="/usr/local/go"
export PATH=${GOPATH}/bin:/usr/local/go/bin:/usr/sbin:/usr/local/bin:${PATH}

cd $GOPATH/src/github.com/kata-containers/tests
.ci/teardown.sh "$WORKSPACE/artifacts"
[kata-containers-runtime-opensuse-15-PR] $ /bin/bash /tmp/jenkins546820317887890670.sh
ERROR: kata-collect-data.sh: cannot find runtime ''
/tmp/jenkins/workspace/kata-containers-runtime-opensuse-15-PR/artifacts /tmp/jenkins/workspace/kata-containers-runtime-opensuse-15-PR/go/src/github.com/kata-containers/tests
gzip: kata-collect-data_*: No such file or directory

Maybe it's worth trying to reproduce the issue locally, on a VM. If you fancy the approach, you can simply spawn a, for instance, fedora32 VM, get the tests repo, and run a ./.ci/setup.sh from the tests top directory. It'll setup the VM for you where you have a similar environment to try to reproduce the issue.

Let us know if some help is needed.

@c3d
Copy link
Member Author

c3d commented Oct 19, 2020

@c3d, I'm seeing this error accross all the CIs:

Drop caches
/tmp/jenkins/workspace/kata-containers-runtime-opensuse-15-PR/go/src/github.com/kata-containers/runtime
Setup virtcontainers environment
Install virtcontainers
make: Entering directory '/tmp/jenkins/workspace/kata-containers-runtime-opensuse-15-PR/go/src/github.com/kata-containers/runtime/virtcontainers'
     GOBUILD  build
     GOBUILD  hook
     GOBUILD  kata-shim
make: Leaving directory '/tmp/jenkins/workspace/kata-containers-runtime-opensuse-15-PR/go/src/github.com/kata-containers/runtime/virtcontainers'
/usr/local/bin/kata-runtime
Logging kata-env information:
no service name provided
Build step 'Execute shell' marked build as failure
Performing Post build task...
Match found for :.* : True
Logical operation result is TRUE
Running script  : #!/bin/bash

export GOPATH=$WORKSPACE/go
export GOROOT="/usr/local/go"
export PATH=${GOPATH}/bin:/usr/local/go/bin:/usr/sbin:/usr/local/bin:${PATH}

cd $GOPATH/src/github.com/kata-containers/tests
.ci/teardown.sh "$WORKSPACE/artifacts"
[kata-containers-runtime-opensuse-15-PR] $ /bin/bash /tmp/jenkins546820317887890670.sh
ERROR: kata-collect-data.sh: cannot find runtime ''
/tmp/jenkins/workspace/kata-containers-runtime-opensuse-15-PR/artifacts /tmp/jenkins/workspace/kata-containers-runtime-opensuse-15-PR/go/src/github.com/kata-containers/tests
gzip: kata-collect-data_*: No such file or directory

Maybe it's worth trying to reproduce the issue locally, on a VM. If you fancy the approach, you can simply spawn a, for instance, fedora32 VM, get the tests repo, and run a ./.ci/setup.sh from the tests top directory. It'll setup the VM for you where you have a similar environment to try to reproduce the issue.

Let us know if some help is needed.

Thanks. The problem is yet another @XYZ@ variable not being replaced. Here, it's @RUNTIME_NAME@ in kata-collect-data.sh. However, this time it's not because the variable is not there, nor because the file is not in GENERATED_VARS. So some new failure mode. Apparently, it is being replaced, but the variable is not set while building. So we end up creating a kata-collect.sh that has the runtime set to "".

I'm currently investigating. I can reproduce in my local build, so no need for a VM at the moment, although I am likely to try and run the CI locally after that change.

@c3d
Copy link
Member Author

c3d commented Oct 19, 2020

I'm currently investigating. I can reproduce in my local build, so no need for a VM at the moment, although I am likely to try and run the CI locally after that change.

OK, found the problem. For some reason, this was a special case for the replacement in the original makefile, which I missed. The replacement is RUNTIME_NAME -> $(TARGET).

A smart Emacs grep regexp search (s|@\([A-Z0-9_]+]\)@|\$(\1)\g if you are curious) allowed me to see that this is the only legitimate case, but that there were one typo:

		-e "s|@DEFENABLEMSWAP@|$(DEFENABLESWAP)|g" \

So this should be the last one, hopefully, Will check if the problem exists on the 2.0 dev branch.

@c3d
Copy link
Member Author

c3d commented Oct 19, 2020

/retest

@codecov
Copy link

codecov bot commented Oct 19, 2020

Codecov Report

Merging #3005 (38fc74c) into master (687ff39) will increase coverage by 0.12%.
The diff coverage is 63.30%.

@@            Coverage Diff             @@
##           master    #3005      +/-   ##
==========================================
+ Coverage   50.28%   50.40%   +0.12%     
==========================================
  Files         120      120              
  Lines       15842    15910      +68     
==========================================
+ Hits         7966     8020      +54     
- Misses       6790     6801      +11     
- Partials     1086     1089       +3     

@c3d
Copy link
Member Author

c3d commented Oct 19, 2020

@fidencio It now looks good. @devimc, @bergwolf, @GabyCT, one last quick look?

@fidencio
Copy link
Member

  • jenkins-ci-ubuntu-1804-cloud-hypervisor-k8s-e2e-containderd-minimal:
18:31:19 running: sonobuoy run --wait=180 --e2e-focus="ConfigMap should be consumable from pods in volume|InitContainer \[NodeConformance\] should invoke init containers|Kubelet when scheduling a busybox command in a pod should printk|Projected secret should be consumable in multiple volumes|Secrets should be consumable via the environment|\[sig-storage\] Downward API volume should update annotations on modification \[NodeConformance\] \[Conformance\]"
18:31:19 time="2020-11-10T17:31:19Z" level=info msg="created object" name=sonobuoy namespace= resource=namespaces
18:31:19 time="2020-11-10T17:31:19Z" level=info msg="created object" name=sonobuoy-serviceaccount namespace=sonobuoy resource=serviceaccounts
18:31:19 time="2020-11-10T17:31:19Z" level=info msg="created object" name=sonobuoy-serviceaccount-sonobuoy namespace= resource=clusterrolebindings
18:31:19 time="2020-11-10T17:31:19Z" level=info msg="created object" name=sonobuoy-serviceaccount-sonobuoy namespace= resource=clusterroles
18:31:19 time="2020-11-10T17:31:19Z" level=info msg="created object" name=sonobuoy-config-cm namespace=sonobuoy resource=configmaps
18:31:19 time="2020-11-10T17:31:19Z" level=info msg="created object" name=sonobuoy-plugins-cm namespace=sonobuoy resource=configmaps
18:31:20 time="2020-11-10T17:31:20Z" level=info msg="created object" name=sonobuoy namespace=sonobuoy resource=pods
18:31:20 time="2020-11-10T17:31:20Z" level=info msg="created object" name=sonobuoy-master namespace=sonobuoy resource=services
18:36:20 Build timed out (after 5 minutes). Marking the build as aborted.
18:36:20 Build was aborted

@amshinde, @jcvenegas, @devimc, have you seen this one before? Any tip of what it may be related to?

@fidencio
Copy link
Member

  • jenkins-ci-Power8-ubuntu-18-04-initrd:
Install os-tree
Job for docker.service failed because the control process exited with error code.
See "systemctl status docker.service" and "journalctl -xe" for details

@bpradipt, @Amulyam24 @clnperez, @jing-wang4, do you have some idea of what may be the issue here?

@fidencio
Copy link
Member

  • jenkins-ci-opensuse-15
    Failing while performing a yum update due to:
error: unpacking of archive failed on file /usr/bin/newgidmap;5faad0a7: cpio: cap_set_file failed - Inappropriate ioctl for device
error: shadow-utils-2:4.6-9.fc30.x86_64: install failed

This is our good and old issue about not having -o xattrs passed to virtiofsd.
My suggestion is to always add that to our CI, regardless of the security implications, till we have a proper fix upstream. By doing this we'll be able to unblock CI.

@fidencio
Copy link
Member

  • jenkins-ubuntu-18-04-vfio:
    @devimc, I'm not capable to look at the errors and figure out whether it's expected or not. May I borrow your brain on this one? :-)

@devimc
Copy link

devimc commented Nov 10, 2020

vfio CI failed testing clh, restarting CI

@likebreath
Copy link
Contributor

@fidencio You are right that the jenkins-ci-podman-clh-fedora-31 CI is an known issue (kata-containers/ci#340). I think we are fine to move on with this CI failure as it is unrelated to this PR. (@jcvenegas had a similar idea as your solution 1 and was experimenting it in a debugging PR kata-containers/tests#2922)

Also, I noticed the other two clh failing CIs were both passing yesterday: clh-k8s-containerd job #159 and clh-k8s-e2e-containerd-mini job #164. So I just restarted those two CI jobs to see whether we got sporadic failures.

@amshinde
Copy link
Member

for Firecracker, most of the CI has passed, but failed at the last step at :

END OF POST BUILD TASK : 0
Archiving artifacts
ERROR: Step ‘Archive the artifacts’ aborted due to exception: 
java.io.IOException: No space left on device

We can ignore this as this is an unrelated error. @chavafg Have you seen this before?

@fidencio
Copy link
Member

  • jenkins-ci-ubuntu-1804-cloudhypervisor-docker:
19:28:55 POST BUILD TASK : FAILURE
19:28:55 END OF POST BUILD TASK : 0
19:28:55 Archiving artifacts
19:29:00 ERROR: Step ‘Archive the artifacts’ aborted due to exception: 
19:29:00 java.io.IOException: No space left on device
19:29:00 	at sun.nio.ch.FileDispatcherImpl.write0(Native Method)
19:29:00 	at sun.nio.ch.FileDispatcherImpl.write(FileDispatcherImpl.java:60)
19:29:00 	at sun.nio.ch.IOUtil.writeFromNativeBuffer(IOUtil.java:93)
19:29:00 	at sun.nio.ch.IOUtil.write(IOUtil.java:65)
19:29:00 	at sun.nio.ch.FileChannelImpl.write(FileChannelImpl.java:211)
19:29:00 	at java.nio.channels.Channels.writeFullyImpl(Channels.java:78)
19:29:00 	at java.nio.channels.Channels.writeFully(Channels.java:101)
19:29:00 	at java.nio.channels.Channels.access$000(Channels.java:61)

Same error as the one for firecracker.

@fidencio
Copy link
Member

  • jenkins-ci-ubuntu-18-04-inited:
    There are a bunch of failures there, including the "No space left on device", seen on both Firecracker and CLH-docker CIs.

@amshinde
Copy link
Member

@c3d Can you backport this PR to stable 1.12 as well?

@amshinde
Copy link
Member

I restarted
jenkins-ci-ubuntu-1804-cloud-hypervisor-k8s-e2e-containerd-minimal job started by Fabiano, instead of the latest run. That has passed but the status not yet reported here.

Ci run that passed:
http://jenkins.katacontainers.io/job/kata-containers-runtime-ubuntu-PR-cloud-hypeprvisor-k8s-e2e-containerd-minimal/174/

Finally vfio failure is unrelated and will be fixed with : kata-containers/tests#3044

I think this PR is good to go, would like to see this backported to 1.12.

@likebreath
Copy link
Contributor

@amshinde @fidencio All required CIs are passing, and the PR is ready to go. Besides the known issue on podman-clh-fedora-31, do we want to land the PR with the failures on opensuse-15 and Power8-ubuntu-18-04-initrd?

@amshinde amshinde merged commit 36d541c into kata-containers:master Nov 11, 2020
amshinde pushed a commit to amshinde/documentation that referenced this pull request Nov 11, 2020
Document restricted annotations, as implemented in
kata-containers/runtime#3005

Fixes: kata-containers#754

Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
(cherry picked from commit 4809aee)
This was referenced Nov 11, 2020
fidencio pushed a commit to fidencio/kata-runtime that referenced this pull request Nov 11, 2020
The build was setting a `FCVALIDPATHS` variable for firecracker, but
that was never being used. Conversely, the firecracker configuration
template was expecting a `FCVALIDHYPERVISORPATHS`, but that variable was
never being set.

Resolve by only setting the `FCVALIDHYPERVISORPATHS` variable to ensure
the generated firecracker config is valid once again.

Fixes: kata-containers#3005

From kata-containers/kata-containers#1002
Fixing kata-containers/kata-containers#1001

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
fidencio pushed a commit to fidencio/kata-runtime that referenced this pull request Nov 11, 2020
During review, Julio Montes pointed out that there was some missing text in the
comments for configuration-xyz.toml.in relative to the virtiofs version.

Fixes: kata-containers#3005

Suggested-by: Julio Montes <julio.montes@intel.com>
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
fidencio pushed a commit to fidencio/kata-runtime that referenced this pull request Nov 11, 2020
The changes introduced in PR kata-containers#3031 require additional changes.
Code lifted from kata-containers/kata-containers#1086.

Fixes: kata-containers#3005

Suggested-by: James O.D. Hunt <james.o.hunt@intel.com>
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
fidencio pushed a commit to fidencio/kata-runtime that referenced this pull request Nov 11, 2020
The build was setting a `FCVALIDPATHS` variable for firecracker, but
that was never being used. Conversely, the firecracker configuration
template was expecting a `FCVALIDHYPERVISORPATHS`, but that variable was
never being set.

Resolve by only setting the `FCVALIDHYPERVISORPATHS` variable to ensure
the generated firecracker config is valid once again.

Fixes: kata-containers#3005

From kata-containers/kata-containers#1002
Fixing kata-containers/kata-containers#1001

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
fidencio pushed a commit to fidencio/kata-runtime that referenced this pull request Nov 11, 2020
During review, Julio Montes pointed out that there was some missing text in the
comments for configuration-xyz.toml.in relative to the virtiofs version.

Fixes: kata-containers#3005

Suggested-by: Julio Montes <julio.montes@intel.com>
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
fidencio pushed a commit to fidencio/kata-runtime that referenced this pull request Nov 11, 2020
The changes introduced in PR kata-containers#3031 require additional changes.
Code lifted from kata-containers/kata-containers#1086.

Fixes: kata-containers#3005

Suggested-by: James O.D. Hunt <james.o.hunt@intel.com>
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
amshinde added a commit that referenced this pull request Nov 11, 2020
amshinde added a commit that referenced this pull request Nov 11, 2020
amshinde pushed a commit to amshinde/documentation that referenced this pull request Nov 11, 2020
Document restricted annotations, as implemented in
kata-containers/runtime#3005

Fixes: kata-containers#754

Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
(cherry picked from commit 4809aee)
c3d added a commit to kata-containers/kata-containers that referenced this pull request Apr 12, 2021
Due to a bad edit / fixup in commit be6ee25, the variable
QEMUVIRTIOFSPATH was incorrectly removed from the makefile.

This problem was found by the 1.x CI checks, see
kata-containers/runtime#3005 (comment)

Fixes: #1017

Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
c3d added a commit to c3d/kata-containers that referenced this pull request Apr 12, 2021
Due to a bad edit / fixup in commit be6ee25, the variable
QEMUVIRTIOFSPATH was incorrectly removed from the makefile.

This problem was found by the 1.x CI checks, see
kata-containers/runtime#3005 (comment)

Fixes: kata-containers#1017

Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs-forward-port Changes need to be applied to a newer branch / repository no-backport-needed Changed do not need to be applied to an older branch / repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validate annotations that refer to binaries
8 participants