-
Notifications
You must be signed in to change notification settings - Fork 374
Conversation
118066d
to
c76fb86
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @gabibeyer. Added a bunch of comments.
Thank you @jodh-intel for all of the feedback and help; working on making those changes :) |
724493b
to
50c5c55
Compare
@jodh-intel @ganeshmaharaj @GabyCT @amshinde @jcvenegas
hwName is a []byte. The fmt does an uninterpreted print, and |
Hi @gabibeyer - you need to annotate with a magic |
@jodh-intel Thank you so much!! 👐 💃 |
0091cf5
to
e9fd02f
Compare
1a5fa52
to
fcd40f7
Compare
/test |
Codecov Report
@@ Coverage Diff @@
## master #1875 +/- ##
==========================================
- Coverage 51.56% 51.48% -0.09%
==========================================
Files 108 109 +1
Lines 14660 14778 +118
==========================================
+ Hits 7560 7608 +48
- Misses 6196 6256 +60
- Partials 904 914 +10 |
/test-nemu |
virtcontainers/kata_agent.go
Outdated
@@ -132,6 +129,40 @@ const ( | |||
grpcStopTracingRequest = "grpc.StopTracingRequest" | |||
) | |||
|
|||
// The function is declared this way for mocking in unit tests | |||
var kataHostSharedDir = func() string { | |||
path := "/run/kata-containers/shared/sandboxes/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to find another solution here as we shouldn't be hard-coding paths like this in non-test code. Maybe add some sort of test setup function? Same comment for functions below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that today its hardcoded in this same file already, right? See https://github.com/kata-containers/runtime/blob/master/virtcontainers/kata_agent.go#L59-L62
WDYT @jodh-intel ?
Since it isn't any worse AFAICT compared to what is there today, I'm okay with doing an improvement to address this in a follow on PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we already have kataHostSharedDir
and kataGuestSharedDir
globals for these paths, so why can't the mocking code either use or change those variables for testing purposes? I don't understand the need for the duplication - added to which it looks very odd to me.
Any thoughts @marcov?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment on the variables is wrong. The reason to have functions in place of const is to have those directories placed in a path accessible by regular users when running rootless.
As alternative, we can leave the const declaration as before, and wherever they are used do something like:
rootless.ResolvePath(kataHostSharedDir)
But I like more how it is done now.
c676a6d
to
d9efd46
Compare
/test |
Hi @gabibeyer, this need a rebase (again) |
d9efd46
to
0ac45b5
Compare
/test |
The jenkins-ci-ubuntu-nemu build history has plenty of failure for all recent PRs. When running docker intergration tests, we get this:
Wondering what we can do, or if this can be ignored 🤔 |
8760344
to
d27a99f
Compare
/test |
There are just a couple small/quick issues that are still outstanding. @marcov if you are willing to address, that'd be great. Otherwise I can see if we can find someone... |
@egernst sure, I can start working on that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really don't like all those hard-coded paths littered throughout the new functions. But once that issue has been resolved, this should be good to land I think.
virtcontainers/kata_agent.go
Outdated
@@ -132,6 +129,40 @@ const ( | |||
grpcStopTracingRequest = "grpc.StopTracingRequest" | |||
) | |||
|
|||
// The function is declared this way for mocking in unit tests | |||
var kataHostSharedDir = func() string { | |||
path := "/run/kata-containers/shared/sandboxes/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we already have kataHostSharedDir
and kataGuestSharedDir
globals for these paths, so why can't the mocking code either use or change those variables for testing purposes? I don't understand the need for the duplication - added to which it looks very odd to me.
Any thoughts @marcov?
f5155c3
to
ab29a4b
Compare
/test |
Add the ability to check whether kata is running rootlessly or not. Add the setup of the rootless directory located in the dir /run/user/<UID> directory. Fixes: kata-containers#1874 Signed-off-by: Gabi Beyer <[email protected]> Co-developed-by: Marco Vedovati <[email protected]> Signed-off-by: Marco Vedovati <[email protected]>
Before using the default ctrsMapTrePath, check whether the runtime is being ran rootlessly, and if so set the ctrsMapTreePath to the rootlessRuntimeDir configured by the libpod rootless library. Fixes: kata-containers#1827 Signed-off-by: Gabi Beyer <[email protected]>
Modify some path variables to be functions that return the path with the rootless directory prefix if running rootlessly. Fixes: kata-containers#1827 Signed-off-by: Gabi Beyer <[email protected]>
rootless execution does not yet support cgroups, so if running rootlessly skip the cgroup creation and deletion. Fixes: 1877 Signed-off-by: Gabi Beyer <[email protected]>
The netlink dep needs to be updated to get logic for the tuntap link. It is fixing a bug that uses a generic link instead. This also requires the golang/x/sys package to be updated for the IFLA_* constraints. Commits for github.com/vishvananda/netlink c8c507c fix: fix ip rule goto bug db99c04 tuntap: Return TunTapLink instead of GenericLink e993616 Fix unit test failure: TestNeighAddDelLLIPAddr fb5fbae Mirred and connmark clobber their ActionAttrs 1187dc9 Fix tests 00009fb Add support for TC_ACT_CONNMARK fafc1e7 support vlan protocol fd97bf4 Add command to set devlink device switchdev mode bcb80b2 Add devlink command by to get specific device name f504738 Fix function comments based on best practices from Effective Go e281812 Fix typos adb577d Add support for IFLA_GSO_* aa950f2 travis: run tests with Go 1.12.x b64d7bc travis: specify go_import_path b9cafe4 remove redundant type assertions in type switch 1e2e7ab Add Support for Virtual XFRM Interfaces 48a75e0 Fix Race Condition in TestXfrmMonitorExpire e37f4b4 Avoid 64K allocation on the heap with each Receive 332a698 Add devlink commands for devlink device information cb78b18 neigh_linux: Fix failure on deleted link neighs updates 2bc5004 Replace redundant copied u32 types with type aliases 093e80f Pass Ndmsg to NeighListExecute 78a3099 Make test suite more deterministic 2529893 genetlink: Add missing error check 91b013f code simplification 023a6da Make go vet happier aa5b058 Simplify code e137ed6 Replace nl.NewRtAttrChild with method on struct 3b1c596 Run TravisCI with Go 1.10 and 1.11 d741264 Reduce allocations b48eed5 Add an API to rename rdma device name 02a3831 Adjust conntrack filters d3a23fd Make AddChild more generic 1404979 Add support for hoplimit metric in routes 6d53654 Add support for neighbor subscription 531df7a Avoid serializing empty TCA_OPTIONS in qdisc messages 56b1bd2 fix: BRIDGE_FLAGS_* constants off-by-one 8aa85bf Add support for action and ifindex in XFRM policy 9eab419 Netlink: Fix Darwin build 2cbcf73 Add a test for Vlan filtering support for bridges. 0bbc55b Initial support for vlan aware bridges. 3ac69fd Add network namespace ID management. d68dce4 Ingress qdisc add/del Test case 1006cf4 Implementation of HFSC d85e18e Allow Tuntap non-persist, allow empty tuntap name d77c86a protinfo: Check if object is nil a06dabf Increase size of receive buffer 3e48e44 Revert "RTEXT_FILTER_VF doesn't always work with dump request, fixes kata-containers#354" 028453c RTEXT_FILTER_VF doesn't always work with dump request, fixes kata-containers#354 ee06b1d add vti6 support b1cc70d fix prefixlen/local IP, incl. PtP addresses 7c0b594 Implemented String() for netem, fq and fq_codel in qdisc 769bb84 Adjust flags values 5f662e0 Add info about VFs on link 985ab95 Add support for link flag allmulticast 16769db Support LWTUNNEL_ENCAP_SEG6_LOCAL (including tests) b7f0669 Add test to Add/Del IPv6 route. 55d3a80 Added tests for Gretap/Gretun devices f07d9d5 Run both Inline/Encap mode in TestSEG6RouteAddDel 1970aef Add RDMA netlink socket for RDMA device information dc00cf9 Add Hash to U32 23a36f2 Add Divisor to U32 85aa3b7 Add statistics to class attributes aa0edbe Add support for setting InfininBand Node and Port GUID of a VF 41009d5 Read conntrack flow statistics a2ad57a Add changelog file, initial release tagging 5236321 Use IFLA_* constants from x/sys/unix 25d2c79 Use IFF_MULTI_QUEUE from x/sys/unix to define TUNTAP_MULTI_QUEUE d35d6b5 Clarify ESN bitmap length construction logic a2af46a Add FQ Codel 465b5fe Add Fq Qdisc support c27b7f7 Run gofmt -s -w on the project 5f5d5cd Add a 'ListExisting' option to get the existing entries in the route/addr/link tables as part of RouteSubscribeWithOptions, AddrSubscribeWithOptions, and LinkSubscribeWithOptions. 5a988e8 Support IPv6 GRE Tun and Tap 7291c36 addr_linux: Implement CacheInfo installation 422ffe6 addr_linux: Skip BROADCAST and LABEL for non-ipv4 1882fa9 Add Matchall filter 7b4c063 Update bpf_linux.go ad19ca1 netlink: allow non linux builds to pass. 3ff4c21 Don't overwrite the XDP file descriptor with flags d4235bf Eliminate cgo from netlink. 54ad9e3 Two new functions: LinkSetBondSlave and VethPeerIndex f67b75e Properly tear down netns at the end of test 016ba6f Add support for managing source MACVLANs 6e7bb56 Run TestSocketGet in dedicated netns a5d066d Fix LinkAdd for sit tunnel on 3.10 kernel 8bead6f Add requirements to conntrack tests 9ce265f Retrieve VLAN and VNI when listing neighbour fad79cb Fix go build issue for fou code Commits for golang/x/sys 88d2dcc unix: add IFLA_* constants for Linux 4.15 c1138c8 unix: update to Linux 4.15, glibc 2.27 and Go 1.10 37707fd unix: move gccgo redeclared *SyscallNoError functions to a separate file 8f27ce8 unix: fix cpuset size argument in sched_affinity syscall 3dbebcf unix: use SyscallNoError and RawSyscallNoError on Linux only ff2a66f unix: fix godoc comment for clen 0346725 unix: add godoc for Sockaddr* types 90f0fdc plan9: add arm support ef80224 unix: add sockaddr_l2 definitions af9a212 unix: don't export padding fields on all platforms af50095 unix: use ParseDirent from syscall 2c42eef unix: adjust replacement regex for removed struct fields for linux/s390x fff93fa unix: add Statx on Linux 52ba35d unix: check error return of os.Symlink in tests on Linux 810d700 unix: match seek argument size to signature on linux/arm b9cf5f9 unix: add cgroupstats type and constants d38bf78 unix: restore gccgo support 2493af8 plan9: move Unsetenv into env_plan9.go 3ca7571 windows: move Unsetenv into env_windows.go 1792d66 unix: move Unsetenv into env_unix.go dd9ec17 unix: fix build on Go 1.8 12d9d5b unix: add SchedGetaffinity and SchedSetaffinity on Linux a3f2cbd unix: fix typo in unix/asm_linux_arm64.s made in 28a7276 28a7276 unix: add SyscallNoError and RawSyscallNoError on Linux 8380141 unix: simplify error handling in *listxattr on FreeBSD df29b91 unix: add TestSelect for *BSD 801364e unix: add Select on Solaris d818ba1 unix: remove syscall constants on Solaris 236baca unix: add timeout tests for Select and Pselect on Linux 571f7bb unix: simplify TestGetwd d5840ad unix: add GetsockoptString for Darwin, *BSD and Solaris Signed-off-by: Gabi Beyer <[email protected]>
The tuntap network device is for tuntap interfaces to connect to the container. A specific use case is the slirp4netns tap interface for rootless kata-runtime. Fixes: kata-containers#1878 Signed-off-by: Gabi Beyer <[email protected]>
ab29a4b
to
e93bf96
Compare
Added a few more tests since coverage went too low /test |
@egernst , @jodh-intel : can you please take another look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the additional tests (and framework around it) @marcov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
This updates grpc-go vendor package to v1.11.3 release, to fix server.Stop() handling so that server.Serve() does not wait blindly. Full commit list: d11072e (tag: v1.11.3) Change version to 1.11.3 d06e756 clientconn: add support for unix network in DialContext. (kata-containers#1883) 452c2a7 Change version to 1.11.3-dev d89cded (tag: v1.11.2) Change version to 1.11.2 98ac976 server: add grpc.Method function for extracting method from context (kata-containers#1961) 0f5fa28 Change version to 1.11.2-dev 1e2570b (tag: v1.11.1) Change version to 1.11.1 d28faca client: Fix race when using both client-side default CallOptions and per-call CallOptions (kata-containers#1948) 48b7669 Change version to 1.11.1-dev afc05b9 (tag: v1.11.0) Change version to 1.11.0 f2620c3 resolver: keep full unparsed target string if scheme in parsed target is not registered (kata-containers#1943) 9d2250f status: rename Status to GRPCStatus to avoid name conflicts (kata-containers#1944) 2756956 status: Allow external packages to produce status-compatible errors (kata-containers#1927) 0ff1b76 routeguide: reimplement distance calculation dfbefc6 service reflection can lookup enum, enum val, oneof, and field symbols (kata-containers#1910) 32d9ffa Documentation: Fix broken link in rpc-errors.md (kata-containers#1935) d5126f9 Correct Go 1.6 support policy (kata-containers#1934) 5415d18 Add documentation and example of adding details to errors (kata-containers#1915) 57640c0 Allow storing alternate transport.ServerStream implementations in context (kata-containers#1904) 031ee13 Fix Test: Update the deadline since small deadlines are prone to flakes on Travis. (kata-containers#1932) 2249df6 gzip: Add ability to set compression level (kata-containers#1891) 8124abf credentials/alts: Remove the enable_untrusted_alts flag (kata-containers#1931) b96718f metadata: Fix bug where AppendToOutgoingContext could modify another context's metadata (kata-containers#1930) 738eb6b fix minor typos and remove grpc.Codec related code in TestInterceptorCanAccessCallOptions (kata-containers#1929) 211a7b7 credentials/alts: Update ALTS "New" APIs (kata-containers#1921) fa28bef client: export types implementing CallOptions for access by interceptors (kata-containers#1902) ec9275b travis: add Go 1.10 and run vet there instead of 1.9 (kata-containers#1913) 13975c0 stream: split per-attempt data from clientStream (kata-containers#1900) 2c2d834 stats: add BeginTime to stats.End (kata-containers#1907) 3a9e1ba Reset ping strike counter right before sending out data. (kata-containers#1905) 90dca43 resolver: always fall back to default resolver when target does not follow URI scheme (kata-containers#1889) 9aba044 server: Convert all non-status errors to codes.Unknown (kata-containers#1881) efcc755 credentials/alts: change ALTS protos to match the golden version (kata-containers#1908) 0843fd0 credentials/alts: fix infinite recursion bug [in custom error type] (kata-containers#1906) 207e276 Fix test race: Atomically access minConnecTimout in testing environment. (kata-containers#1897) 3ae2a61 interop: Add use_alts flag to client and server binaries (kata-containers#1896) 5190b06 ALTS: Simplify "New" APIs (kata-containers#1895) 7c5299d Fix flaky test: TestCloseConnectionWhenServerPrefaceNotReceived (kata-containers#1870) f0a1202 examples: Replace context.Background with context.WithTimeout (kata-containers#1877) a1de3b2 alts: Change ALTS proto package name (kata-containers#1886) 2e7e633 Add ALTS code (kata-containers#1865) 583a630 Expunge error codes that shouldn't be returned from library (kata-containers#1875) 2759199 Small spelling fixes (unknow -> unknown) (kata-containers#1868) 12da026 clientconn: fix a typo in GetMethodConfig documentation (kata-containers#1867) dfa1834 Change version to 1.11.0-dev (kata-containers#1863) 46fd263 benchmarks: add flag to benchmain to use bufconn instead of network (kata-containers#1837) 3926816 addrConn: Report underlying connection error in RPC error (kata-containers#1855) 445b728 Fix data race in TestServerGoAwayPendingRPC (kata-containers#1862) e014063 addrConn: keep retrying even on non-temporary errors (kata-containers#1856) 484b3eb transport: fix race causing flow control discrepancy when sending messages over server limit (kata-containers#1859) 6c48c7f interop test: Expect io.EOF from stream.Send() (kata-containers#1858) 08d6261 metadata: provide AppendToOutgoingContext interface (kata-containers#1794) d50734d Add status.Convert convenience function (kata-containers#1848) 365770f streams: Stop cleaning up after orphaned streams (kata-containers#1854) 7646b53 transport: support stats.Handler in serverHandlerTransport (kata-containers#1840) 104054a Fix connection drain error message (kata-containers#1844) d09ec43 Implement unary functionality using streams (kata-containers#1835) 37346e3 Revert "Add WithResolverUserOptions for custom resolver build options" (kata-containers#1839) 424e3e9 Stream: do not cancel ctx created with service config timeout (kata-containers#1838) f9628db Fix lint error and typo (kata-containers#1843) 0bd008f stats: Fix bug causing trailers-only responses to be reported as headers (kata-containers#1817) 5769e02 transport: remove unnecessary rstReceived (kata-containers#1834) 0848a09 transport: remove redundant check of stream state in Write (kata-containers#1833) c22018a client: send RST_STREAM on client-side errors to prevent server from blocking (kata-containers#1823) 82e9f61 Use keyed fields for struct initializers (kata-containers#1829) 5ba054b encoding: Introduce new method for registering and choosing codecs (kata-containers#1813) 4f7a2c7 compare atomic and mutex performance in case of contention. (kata-containers#1788) b71aced transport: Fix a data race when headers are received while the stream is being closed (kata-containers#1814) 46bef23 Write should fail when the stream was done but context wasn't cancelled. (kata-containers#1792) 10598f3 Explain target format in DialContext's documentation (kata-containers#1785) 08b7bd3 gzip: add Name const to avoid typos in usage (kata-containers#1804) 8b02d69 remove .please-update (kata-containers#1800) 1cd2346 Documentation: update broken wire.html link in metadata package. (kata-containers#1791) 6913ad5 Document that all errors from RPCs are status errors (kata-containers#1782) 8a8ac82 update const order (kata-containers#1770) e975017 Don't set reconnect parameters when the server has already responded. (kata-containers#1779) 7aea499 credentials: return Unavailable instead of Internal for per-RPC creds errors (kata-containers#1776) c998149 Avoid copying headers/trailers in unary RPCs unless requested by CallOptions (kata-containers#1775) 8246210 Update version to 1.10.0-dev (kata-containers#1777) 17c6e90 compare atomic and mutex performance for incrementing/storing one variable (kata-containers#1757) 65c901e Fix flakey test. (kata-containers#1771) 7f2472b grpclb: Remove duplicate init() (kata-containers#1764) 09fc336 server: fix bug preventing Serve from exiting when Listener is closed (kata-containers#1765) 035eb47 Fix TestGracefulStop flakiness (kata-containers#1767) 2720857 server: fix race between GracefulStop and new incoming connections (kata-containers#1745) 0547980 Notify parent ClientConn to re-resolve in grpclb (kata-containers#1699) e6549e6 Add dial option to set balancer (kata-containers#1697) 6610f9a Fix test: Data race while resetting global var. (kata-containers#1748) f4b5237 status: add Code convenience function (kata-containers#1754) 47bddd7 vet: run golint on _string files (kata-containers#1749) 45088c2 examples: fix concurrent map accesses in route_guide server (kata-containers#1752) 4e393e0 grpc: fix deprecation comments to conform to standard (kata-containers#1691) 0b24825 Adjust keepalive paramenters in the test such that scheduling delays don't cause false failures too often. (kata-containers#1730) f9390a7 fix typo (kata-containers#1746) 6ef45d3 fix stats flaky test (kata-containers#1740) 98b17f2 relocate check for shutdown in ac.tearDown() (kata-containers#1723) 5ff10c3 fix flaky TestPickfirstOneAddressRemoval (kata-containers#1731) 2625f03 bufconn: allow readers to receive data after writers close (kata-containers#1739) b0e0950 After sending second goaway close conn if idle. (kata-containers#1736) b8cf13e Make sure all goroutines have ended before restoring global vars. (kata-containers#1732) 4742c42 client: fix race between server response and stream context cancellation (kata-containers#1729) 8fba5fc In gracefull stop close server transport only after flushing status of the last stream. (kata-containers#1734) d1fc8fa Deflake tests that rely on Stop() then Dial() not reconnecting (kata-containers#1728) dba60db Switch balancer to grpclb when at least one address is grpclb address (kata-containers#1692) ca1b23b Update CONTRIBUTING.md to CNCF CLA 2941ee1 codes: Add UnmarshalJSON support to Code type (kata-containers#1720) ec61302 naming: Fix build constraints for go1.6 and go1.7 (kata-containers#1718) b8191e5 remove stringer and go generate (kata-containers#1715) ff1be3f Add WithResolverUserOptions for custom resolver build options (kata-containers#1711) 580defa Fix grpc basics link in route_guide example (kata-containers#1713) b7dc71e Optimize codes.String() method using a switch instead of a slice of indexes (kata-containers#1712) 1fc873d Disable ccBalancerWrapper when it is closed (kata-containers#1698) bf35f1b Refactor roundrobin to support custom picker (kata-containers#1707) 4308342 Change parseTimeout to not handle non-second durations (kata-containers#1706) be07790 make load balancing policy name string case-insensitive (kata-containers#1708) cd563b8 protoCodec: avoid buffer allocations if proto.Marshaler/Unmarshaler (kata-containers#1689) 61c6740 Add comments to ClientConn/SubConn interfaces to indicate new methods may be added (kata-containers#1680) ddbb27e client: backoff before reconnecting if an HTTP2 server preface was not received (kata-containers#1648) a4bf341 use the request context with net/http handler (kata-containers#1696) c6b4608 transport: fix race sending RPC status that could lead to a panic (kata-containers#1687) 00383af Fix misleading default resolver scheme comments (kata-containers#1703) a62701e Eliminate data race in ccBalancerWrapper (kata-containers#1688) 1e1a47f Re-resolve target when one connection becomes TransientFailure (kata-containers#1679) 2ef021f New grpclb implementation (kata-containers#1558) 10873b3 Fix panics on balancer and resolver updates (kata-containers#1684) 646f701 Change version to 1.9.0-dev (kata-containers#1682) Fixes: kata-containers#307 Signed-off-by: Peng Tao <[email protected]>
Run kata without sudo privileges. To do this various steps were taken:
Includes README.md on configurations user must make in order to run seamlessly
Fixes: #1358