-
Notifications
You must be signed in to change notification settings - Fork 40k
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
Node Conformance Test: Add system verification #32427
Node Conformance Test: Add system verification #32427
Conversation
Please remove the dependency of #31093 from this one. I think this one should be in first before the packaging solution. |
fcbede9
to
5d8e157
Compare
@dchen1107 Removed the dependency. |
106128f
to
ee5aa9e
Compare
@k8s-bot test this issue #IGNORE. |
Cgroups: []string{"cpu", "cpuacct", "cpuset", "devices", "freezer", "memory"}, | ||
RuntimeSpec: RuntimeSpec{ | ||
DockerSpec: &DockerSpec{ | ||
GraphDriver: []string{"aufs", "overlay", "overlay2", "devicemapper"}, |
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 don't think we validate overlay2 in any format. It might work, but we need to kick out the qualification process before we claim we support 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.
Ok, I just realized that when overlayfs moved to kernel tree, the upstream called it overlay2. What you are referring to graphdrivers specifically named by docker.
From https://docs.docker.com/engine/userguide/storagedriver/overlayfs-driver/ it says "For the overlay2 driver, the version of your kernel must be 4.0 or newer" which should be fine on GCI & CoreOS cases (cc/ @euank @justinsb @philips). But should we tight our validation here against kernel version too?
Also I just realized that we don't know how the docker upgrade will work from 'overlay' to 'overlay2'. We didn't validate that part when we validate docker 1.12.
ee5aa9e
to
f2393fb
Compare
@dchen1107 Addressed comments.
|
for _, v := range validators { | ||
glog.Infof("Validating %s...", v.Name()) | ||
err := v.Validate(spec) | ||
if err != nil { |
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.
nit: errors.NewAggregate()
actually checks to see if the errors in the list are nil or not, so you don't have to check here.
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.
ACK.
f2393fb
to
6d6cd8d
Compare
As we'd like to use this in |
Here are couple of things missing in current validation tests as the minimal requirements for kernel:
There are more required based on the feature set. For example, if the user uses Gluster volume, Kernel has to configured with CONFIG_FUSE; if the user uses Calico networking, CONFIG_IP_SET_HASH_IP has to be set. We can extend this in next version for those as a warning with detail information, not as the minimal requirement here. |
@errordeveloper @luxas Sorry for the delay. We'll finish this this week. @errordeveloper Feel free to send PR to make this prettier! That's really appreciated! :) |
It would be great if we could get this PR in before the code freeze. Thanks! |
eb83300
to
0ac3e2a
Compare
@dchen1107 Addressed comments. Here is the new output: |
Jenkins verification failed for commit 0ac3e2a5d3e86fcdd19f0aac544e67d3931c4fe1. Full PR test history. The magic incantation to run this job again is |
756bda0
to
f9b50f0
Compare
@dchen1107 I've included unit test from #32454 into this PR so as to get the change merged soon. |
Jenkins GCI GCE e2e failed for commit f9b50f0. Full PR test history. The magic incantation to run this job again is |
@k8s-bot gci gce e2e test this |
LGTM |
Apply LGTM based on #32427 (comment). |
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
Automatic merge from submit-queue |
Automatic merge from submit-queue Node Conformance Test: Containerize the node e2e test For #30122, #30174. Based on #32427, #32454. **Please only review the last 3 commits.** This PR packages the node e2e test into a docker image: - 1st commit: Add `NodeConformance` flag in the node e2e framework to avoid starting kubelet and collecting system logs. We do this because: - There are all kinds of ways to manage kubelet and system logs, for different situation we need to mount different things into the container, run different commands. It is hard and unnecessary to handle the complexity inside the test suite. - 2nd commit: Remove all `sudo` in the test container. We do this because: - In most container, there is no `sudo` command, and there is no need to use `sudo` inside the container. - It introduces some complexity to use `sudo` inside the test. (#29211, #26748) In fact we just need to run the test suite with `sudo`. - 3rd commit: Package the test into a docker container with corresponding `Makefile` and `Dockerfile`. We also added a `run_test.sh` script to start kubelet and run the test container. The script is only for demonstration purpose and we'll also use the script in our node e2e framework. In the future, we should update the script to start kubelet in production way (maybe with `systemd` or `supervisord`). @dchen1107 @vishh /cc @kubernetes/sig-node @kubernetes/sig-testing **Release note**: <!-- Steps to write your release note: 1. Use the release-note-* labels to set the release note state (if you have access) 2. Enter your extended release note in the below block; leaving it blank means using the PR title as the release note. If no release note is required, just write `NONE`. --> ``` release-note Release alpha version node test container gcr.io/google_containers/node-test-ARCH:0.1 for users to verify their node setup. ```
For #30122 and #29081.
This PR introduces system verification test in node e2e and conformance test. It will run before the real test. Once the system verification fails, the test will just fail. The output of the system verification is like this:
It verifies the system following a predefined
SysSpec
:Currently, it only supports:
The validating framework is ready. The specific validation items could be added over time.
@dchen1107
/cc @kubernetes/sig-node
This change is