Add conformance CI test with all alpha features enabled #1428

weiqiangt · 2020-10-22T05:50:23Z

No description provided.

antrea-bot · 2020-10-22T05:50:39Z

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

/test-e2e: to trigger e2e tests.
/skip-e2e: to skip e2e tests.
/test-conformance: to trigger conformance tests.
/skip-conformance: to skip conformance tests.
/test-whole-conformance: to trigger all conformance tests on linux.
/skip-whole-conformance: to skip all conformance tests on linux.
/test-networkpolicy: to trigger networkpolicy tests.
/skip-networkpolicy: to skip networkpolicy tests.
/test-windows-conformance: to trigger windows conformance tests.
/skip-windows-conformance: to skip windows conformance tests.
/test-windows-networkpolicy: to trigger windows networkpolicy tests.
/skip-windows-networkpolicy: to skip windows networkpolicy tests.
/test-hw-offload: to trigger ovs hardware offload test.
/skip-hw-offload: to skip ovs hardware offload test.
/test-all: to trigger all tests (except whole conformance).
/skip-all: to skip all tests (except whole conformance).

codecov-io · 2020-10-22T06:18:41Z

Codecov Report

Merging #1428 into master will increase coverage by 0.10%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1428      +/-   ##
==========================================
+ Coverage   67.80%   67.90%   +0.10%     
==========================================
  Files         159      159              
  Lines       12829    12829              
==========================================
+ Hits         8699     8712      +13     
+ Misses       3208     3194      -14     
- Partials      922      923       +1

Flag	Coverage Δ
integration-tests	`47.63% <ø> (ø)`
kind-e2e-tests	`55.82% <ø> (+0.72%)`	⬆️
unit-tests	`42.29% <ø> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/controller/networkpolicy/tier.go	`90.00% <0.00%> (-10.00%)`	⬇️
...ntroller/networkpolicy/networkpolicy_controller.go	`80.69% <0.00%> (ø)`
pkg/agent/openflow/client.go	`67.85% <0.00%> (+0.35%)`	⬆️
...ntroller/networkpolicy/networkpolicy_controller.go	`69.25% <0.00%> (+3.33%)`	⬆️
pkg/apis/controlplane/v1beta1/sets.go	`44.11% <0.00%> (+7.35%)`	⬆️

ksamoray · 2020-10-22T07:25:01Z

LGTM. Yet @lzhecheng was reluctant to add additional tests for Antrea Proxy, so let's wait for his feedback

lzhecheng · 2020-10-22T12:11:01Z

LGTM. Yet @lzhecheng was reluctant to add additional tests for Antrea Proxy, so let's wait for his feedback

Thanks Weiqiang for your effort! As I explained to Kobi yesterday, I think it might be unnecessary to add one job for each feature like Antrea Proxy. In the near future, we will turn on Antrea Proxy by default and then this job become useless since it will be tested on the major jobs... Also, now Antrea Proxy PR can turn on Antrea Proxy feature gate for testing and turn it off before getting merged. Let alone the extra cloud computing resource for this new job.
This is just my personal idea. What do you think? @antoninbas Since Weiqiang has finished the CI code, we can have it here and delete it when it is turned on by default. I just want to say it is not a must-have job.

antoninbas · 2020-10-22T20:03:36Z

@lzhecheng it may make sense to have one job with the default manifest and one job with all the "alpha" features enabled, especially since these "alpha" features tend to have dependencies on each other. This way we keep the number of jobs under control and we don't need to keep updating the job list as feature stability levels evolve. But yes, I would recommend against defining one job for each individual feature.

weiqiangt · 2020-10-23T03:00:15Z

@lzhecheng @antoninbas, thanks for your feedback and I understand your opinions.
But since we may want to enable Antrea Proxy by default in future releases, we need to have enough confidence to believe that proxy is stable. Let alone manually toggle the feature gate to verify a direct PR, we need to ensure the feature is not broken by other PRs, thus we need not only e2e but conformance to guard the feature.
For resource-consuming concerns, I think checks like this do not need to be mandatory and we can only run the check when we feel a PR may have influence.
I also feel good if there is a conformance test for the deluxe edition (which also covers my ) and I will update this PR.

lzhecheng · 2020-10-23T05:47:34Z

Yes, I think we can have one job for all "alpha" features including Antrea Proxy. Maybe let's rename the job in this PR to "alpha-feature-conformance" and make it optional.

lzhecheng · 2020-10-27T10:44:26Z

ci/jenkins/test-vmc.sh

                  [--garbage-collection] [--setup-only] [--cleanup-only] [--coverage] [--test-only]

-Setup a VMC cluster to run K8s e2e community tests (E2e, Conformance, whole Conformance & Network Policy).
+Setup a VMC cluster to run K8s e2e community tests (E2e, Conformance, Antrea Proxy Conformance, whole Conformance & Network Policy).


Suggested change

Setup a VMC cluster to run K8s e2e community tests (E2e, Conformance, Antrea Proxy Conformance, whole Conformance & Network Policy).

Setup a VMC cluster to run K8s e2e community tests (E2e, Conformance, full features Conformance, whole Conformance & Network Policy).

I don't think this was addressed?

lzhecheng · 2020-10-27T10:44:43Z

ci/jenkins/test-vmc.sh


        --cluster-name           The cluster name to be used for the generated VMC cluster.
        --kubeconfig             Path to save kubeconfig of generated VMC cluster.
        --workdir                Home path for Go, vSphere information and antrea_logs during cluster setup. Default is $WORKDIR.
        --log-mode               Use the flag to set either 'report', 'detail', or 'dump' level data for sonobouy results.
-        --testcase               The testcase to run: e2e, conformance, whole-conformance or networkpolicy.
+        --testcase               The testcase to run: e2e, conformance, proxy-conformance, whole-conformance or networkpolicy.


Suggested change

--testcase The testcase to run: e2e, conformance, proxy-conformance, whole-conformance or networkpolicy.

--testcase The testcase to run: e2e, conformance, full-features-conformance, whole-conformance or networkpolicy.

ci/jenkins/test-vmc.sh

lzhecheng

LGTM except in some places, "proxy-conformance" should be "full-features-conformance"

weiqiangt · 2020-10-28T02:26:15Z

LGTM except in some places, "proxy-conformance" should be "full-features-conformance"

Thanks, @lzhecheng.

weiqiangt · 2020-10-28T02:26:52Z

/skip-all

lzhecheng · 2020-10-28T10:23:22Z

jenkins-jobs-validator failed. I checked logs:
https://jenkins.antrea-ci.rocks/job/antrea-jenkins-job-validator-job-validator/4381/console

It seems indention here is wrong:
https://github.com/vmware-tanzu/antrea/blob/feffa6dd1b62dd4110eb1518c07b52ab70fd1a50/ci/jenkins/jobs/projects.yaml#L413

antoninbas · 2020-10-28T20:40:19Z

Makefile

@@ -247,6 +247,7 @@ build-ubuntu-coverage:
 manifest:
 	@echo "===> Generating dev manifest for Antrea <==="
 	$(CURDIR)/hack/generate-manifest.sh --mode dev > build/yamls/antrea.yml
+	$(CURDIR)/hack/generate-manifest.sh --mode dev --full > build/yamls/antrea-full.yml


Could we avoid checking this manifest in? Having more manifest files checked-in means that every PR that makes a small change to an Antrea resource leads to a big diff.

I feel like this manifest is only useful for testing and therefore we can generate in on the fly as part of the test script. We do the same thing for the "coverage" manifests in test-vmc.sh. For those we have a dedicated Makefile target (make manifest-coverage) but I think it's not necessary here: we can just call generate-manifest.sh directly from the test-vmc.sh script.

Make sense. I have removed the generation in Makefile and moved it to test-vmc.sh.

antoninbas · 2020-10-28T20:40:54Z

ci/jenkins/jobs/projects.yaml

@@ -410,6 +410,92 @@
          started_status: null
          wrappers: []
          publishers: []
+        - '{name}-{test_name}-for-pull-request':
+          test_name: full-features-conformance-pending-label


sorry for the late comment, but I feel like all-features-* is a better name

antoninbas · 2020-10-28T20:41:22Z

hack/generate-manifest.sh

@@ -27,6 +27,7 @@ Generate a YAML manifest for Antrea using Kustomize and print it to stdout.
        --kind                        Generate a manifest appropriate for running Antrea in a Kind cluster
        --cloud                       Generate a manifest appropriate for running Antrea in Public Cloud
        --ipsec                       Generate a manifest with IPSec encryption of tunnel traffic enabled
+        --full                        Generate a manifest with all alpha features enabled


Please rename to --all-features

antoninbas

I see several references to "proxy-conformance" in test-vmc.sh?

You should also remove build/yamls/antrea-full.yml, it no longer needs to be checked-in

antoninbas · 2020-10-30T18:09:09Z

ci/jenkins/test-vmc.sh

                  [--garbage-collection] [--setup-only] [--cleanup-only] [--coverage] [--test-only]

-Setup a VMC cluster to run K8s e2e community tests (E2e, Conformance, whole Conformance & Network Policy).
+Setup a VMC cluster to run K8s e2e community tests (E2e, Conformance, Antrea Proxy Conformance, whole Conformance & Network Policy).


I don't think this was addressed?

antoninbas · 2020-10-30T18:09:17Z

ci/jenkins/test-vmc.sh


        --cluster-name           The cluster name to be used for the generated VMC cluster.
        --kubeconfig             Path to save kubeconfig of generated VMC cluster.
        --workdir                Home path for Go, vSphere information and antrea_logs during cluster setup. Default is $WORKDIR.
        --log-mode               Use the flag to set either 'report', 'detail', or 'dump' level data for sonobouy results.
-        --testcase               The testcase to run: e2e, conformance, whole-conformance or networkpolicy.
+        --testcase               The testcase to run: e2e, conformance, proxy-conformance, whole-conformance or networkpolicy.


Signed-off-by: Weiqiang Tang <weiqiangt@vmware.com>

weiqiangt · 2020-11-01T15:16:28Z

Sorry, it looks like the review commit was flushed by my local force-push.

lzhecheng · 2020-11-03T01:33:48Z

/test-e2e
/test-networkpolicy
/test-conformance

vmwclabot added the cla-not-required label Oct 22, 2020

weiqiangt requested a review from lzhecheng October 22, 2020 05:51

weiqiangt force-pushed the jenkins-proxy-conformance branch from 9ff9d39 to 291d17f Compare October 22, 2020 05:56

weiqiangt requested review from antoninbas and ksamoray October 22, 2020 06:04

weiqiangt force-pushed the jenkins-proxy-conformance branch from 291d17f to e030bc7 Compare October 22, 2020 06:15

weiqiangt force-pushed the jenkins-proxy-conformance branch from e030bc7 to 3e083d4 Compare October 23, 2020 06:14

weiqiangt changed the title ~~Add conformance CI test with Antrea Proxy enabled~~ Add conformance CI test with all alpha features enabled Oct 23, 2020

lzhecheng reviewed Oct 27, 2020

View reviewed changes

ci/jenkins/test-vmc.sh Outdated Show resolved Hide resolved

lzhecheng reviewed Oct 27, 2020

View reviewed changes

ci/jenkins/test-vmc.sh Outdated Show resolved Hide resolved

lzhecheng self-requested a review October 27, 2020 10:45

lzhecheng previously approved these changes Oct 27, 2020

View reviewed changes

weiqiangt dismissed lzhecheng’s stale review via feffa6d October 28, 2020 02:25

antoninbas reviewed Oct 28, 2020

View reviewed changes

weiqiangt force-pushed the jenkins-proxy-conformance branch from feffa6d to 9e2e7c0 Compare October 30, 2020 06:20

antoninbas reviewed Oct 30, 2020

View reviewed changes

Add conformance CI test with Antrea Proxy enabled

b29f691

Signed-off-by: Weiqiang Tang <weiqiangt@vmware.com>

weiqiangt force-pushed the jenkins-proxy-conformance branch from 9e2e7c0 to b29f691 Compare November 1, 2020 15:13

weiqiangt requested review from lzhecheng and antoninbas November 1, 2020 15:17

antoninbas approved these changes Nov 2, 2020

View reviewed changes

weiqiangt merged commit 3a0b9c5 into antrea-io:master Nov 3, 2020

weiqiangt deleted the jenkins-proxy-conformance branch November 3, 2020 03:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add conformance CI test with all alpha features enabled #1428

Add conformance CI test with all alpha features enabled #1428

weiqiangt commented Oct 22, 2020

antrea-bot commented Oct 22, 2020

codecov-io commented Oct 22, 2020 •

edited

Loading

ksamoray commented Oct 22, 2020

lzhecheng commented Oct 22, 2020

antoninbas commented Oct 22, 2020

weiqiangt commented Oct 23, 2020

lzhecheng commented Oct 23, 2020

lzhecheng Oct 27, 2020

antoninbas Oct 30, 2020

lzhecheng Oct 27, 2020

antoninbas Oct 30, 2020

lzhecheng left a comment

weiqiangt commented Oct 28, 2020

weiqiangt commented Oct 28, 2020

lzhecheng commented Oct 28, 2020

antoninbas Oct 28, 2020

weiqiangt Oct 30, 2020

antoninbas Oct 28, 2020

weiqiangt Oct 30, 2020

antoninbas Oct 28, 2020

weiqiangt Oct 30, 2020

antoninbas left a comment

antoninbas Oct 30, 2020

antoninbas Oct 30, 2020

weiqiangt commented Nov 1, 2020

lzhecheng commented Nov 3, 2020

	Setup a VMC cluster to run K8s e2e community tests (E2e, Conformance, Antrea Proxy Conformance, whole Conformance & Network Policy).
	Setup a VMC cluster to run K8s e2e community tests (E2e, Conformance, full features Conformance, whole Conformance & Network Policy).

	--testcase The testcase to run: e2e, conformance, proxy-conformance, whole-conformance or networkpolicy.
	--testcase The testcase to run: e2e, conformance, full-features-conformance, whole-conformance or networkpolicy.

Add conformance CI test with all alpha features enabled #1428

Add conformance CI test with all alpha features enabled #1428

Conversation

weiqiangt commented Oct 22, 2020

antrea-bot commented Oct 22, 2020

codecov-io commented Oct 22, 2020 • edited Loading

Codecov Report

ksamoray commented Oct 22, 2020

lzhecheng commented Oct 22, 2020

antoninbas commented Oct 22, 2020

weiqiangt commented Oct 23, 2020

lzhecheng commented Oct 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lzhecheng left a comment

Choose a reason for hiding this comment

weiqiangt commented Oct 28, 2020

weiqiangt commented Oct 28, 2020

lzhecheng commented Oct 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antoninbas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weiqiangt commented Nov 1, 2020

lzhecheng commented Nov 3, 2020

codecov-io commented Oct 22, 2020 •

edited

Loading