kubernetes / ingress-nginx

Ingress-NGINX Controller for Kubernetes
https://kubernetes.github.io/ingress-nginx/
Apache License 2.0
17.22k stars 8.2k forks source link

New e2e test for static deployment for kind #9467

Open strongjz opened 1 year ago

strongjz commented 1 year ago

All of our e2e tests are done with helm or kind, then

This is the static deployment that needs a test harness set up like helm and the images https://github.com/kubernetes/ingress-nginx/blob/main/deploy/static/provider/kind/deploy.yaml

kind e2e test https://github.com/kubernetes/ingress-nginx/blob/main/.github/workflows/ci.yaml#L320

/triage accepted /kind feature /priority backlog /help-wanted /good-first-issue

k8s-ci-robot commented 1 year ago

@strongjz: This request has been marked as suitable for new contributors.

Guidelines

Please ensure that the issue body includes answers to the following questions:

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-good-first-issue command.

In response to [this](https://github.com/kubernetes/ingress-nginx/issues/9467): >All of our e2e tests are done with helm or kind, then > >This is the static deployment that needs a test harness set up like helm and the images https://github.com/kubernetes/ingress-nginx/blob/main/deploy/static/provider/kind/deploy.yaml > > >kind e2e test https://github.com/kubernetes/ingress-nginx/blob/main/.github/workflows/ci.yaml#L320 > > >/triage accepted >/kind feature >/priority backlog >/help-wanted >/good-first-issue Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
tan-i-ham commented 1 year ago

Hi @strongjz,

I searched this issue by the label "good first issue", so I might needs some help.

I would try my best to contribute and I have some questions about this issue.

Do you mean using https://github.com/kubernetes/ingress-nginx/blob/main/deploy/static/provider/kind/deploy.yaml in the kind-e2e-test? My approach might be update the command argument, but not sure if my understanding is correct.

nat-leo commented 1 year ago

After a bit of digging, assumptions, and jumping to conclusions:

We want to build a test-harness so that we can run tests with kind, using the following command in our ci workflow. However, it breaks:

make kind-e2e-test
Use 'docker scan' to run Snyk tests against images to find vulnerabilities and learn how to fix them
[dev-env] .. done building controller images
[dev-env] now building e2e-image..
..entered Makefile in /test/e2e-image
..calling Make target <<e2e-test-binary>> in /Makefile from inside /test/e2e-image/Makefile
Reached DIND check ELSE block, inside run-in-docker.sh
Failed to compile e2e:

go: downloading github.com/onsi/ginkgo/v2 v2.6.1
go: downloading github.com/stretchr/testify v1.8.1
go: downloading k8s.io/apiextensions-apiserver v0.25.0
go: downloading github.com/moul/pb v0.0.0-20220425114252-bca18df4138c
go: downloading google.golang.org/grpc v1.51.0
go: downloading golang.org/x/crypto v0.5.0
go: downloading github.com/Azure/go-autorest/autorest v0.11.27
go: downloading github.com/Azure/go-autorest/autorest/adal v0.9.20
go: downloading github.com/Azure/go-autorest v14.2.0+incompatible
go: downloading github.com/yudai/gojsondiff v1.0.0
go: downloading github.com/pmezard/go-difflib v1.0.0
go: downloading cloud.google.com/go/compute/metadata v0.2.0
go: downloading github.com/Azure/go-autorest/logger v0.2.1
go: downloading github.com/Azure/go-autorest/tracing v0.6.0
go: downloading github.com/Azure/go-autorest/autorest/date v0.3.0
go: downloading cloud.google.com/go/compute v1.9.0
go: downloading github.com/golang-jwt/jwt/v4 v4.2.0
go: downloading google.golang.org/genproto v0.0.0-20221118155620-16455021b5e6
go: downloading github.com/sergi/go-diff v1.1.0
go: downloading github.com/yudai/golcs v0.0.0-20170316035057-ecda9a501e82
../../.modcache/golang.org/x/oauth2@v0.3.0/google/default.go:17:2: ambiguous import: found package cloud.google.com/go/compute/metadata in multiple modules:
    cloud.google.com/go/compute v1.9.0 (/go/src/k8s.io/ingress-nginx/.modcache/cloud.google.com/go/compute@v1.9.0/metadata)
    cloud.google.com/go/compute/metadata v0.2.0 (/go/src/k8s.io/ingress-nginx/.modcache/cloud.google.com/go/compute/metadata@v0.2.0)

ginkgo build failed
  Failed to compile all tests
make[2]: *** [e2e-test-binary] Error 1
make[1]: *** [image] Error 2
Deleting cluster "ingress-nginx-dev" ...
make: *** [kind-e2e-test] Error 2

The failure occurs while making /test/e2e-image/Makefile at this point

make -C $(DIR)/../../ e2e-test-binary

and within the make e2e-test-binary above we compile the tests with

ginkgo build ./test/e2e

Resulting in the ginkgo build failed. The test harness, wherever it will go, should allow us to run

make kind-e2e-test

without error.

longwuyuan commented 1 year ago

Yes, this is a lesser known thing currently existing and the details are murky. The hope was the the impact will be contained. Have you rebased already and still see the error. And please come talk about it on kubernetes slack in the ingress-nginx-dev channel. Less resources here. Have you looked at the recent CI tests on the project;s repo on github, in ht actions tab.

The known details are as follows. We have dependabot creating PR that are updating packages in the project's go.mod/go.sum. The impact of these changes is that the indirect dependencies are showing up as part of git diff, when tests are run. There was one insance where a PR chcked in update go.mod & update go.sum. But it seems there is a need to do a complicated housekeeping sequence of steps like ;

diff --git a/go.mod b/go.mod
index 2daf8e667..156799ffb 100644
--- a/go.mod
+++ b/go.mod
@@ -27,7 +27,6 @@ require (
        github.com/zakjan/cert-chain-resolver v0.0.0-20211122211144-c6b0b792af9a
        golang.org/x/crypto v0.5.0
        google.golang.org/grpc v1.51.0
-       google.golang.org/grpc/examples v0.0.0-20221220003428-4f16fbe410f7
        gopkg.in/go-playground/pool.v3 v3.1.1
        gopkg.in/mcuadros/go-syslog.v2 v2.3.0
        k8s.io/api v0.25.4
@@ -45,8 +44,8 @@ require (
 )

 require (
-       cloud.google.com/go/compute v1.12.1 // indirect
-       cloud.google.com/go/compute/metadata v0.2.1 // indirect
+       cloud.google.com/go/compute v1.9.0 // indirect
+       cloud.google.com/go/compute/metadata v0.2.0 // indirect
        github.com/Azure/go-autorest v14.2.0+incompatible // indirect
        github.com/Azure/go-autorest/autorest v0.11.27 // indirect
        github.com/Azure/go-autorest/autorest/adal v0.9.20 // indirect
@@ -72,6 +71,7 @@ require (
        github.com/go-openapi/jsonpointer v0.19.5 // indirect
        github.com/go-openapi/jsonreference v0.19.5 // indirect
        github.com/go-openapi/swag v0.19.14 // indirect
+       github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0 // indirect
        github.com/godbus/dbus/v5 v5.0.6 // indirect
        github.com/gogo/protobuf v1.3.2 // indirect
        github.com/golang-jwt/jwt/v4 v4.2.0 // indirect
@@ -82,6 +82,7 @@ require (
        github.com/google/gnostic v0.5.7-v3refs // indirect
        github.com/google/go-cmp v0.5.9 // indirect
        github.com/google/gofuzz v1.1.0 // indirect
+       github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38 // indirect
        github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect
        github.com/google/uuid v1.3.0 // indirect
        github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7 // indirect
diff --git a/go.sum b/go.sum
index 84a8bb106..a1f090e34 100644
--- a/go.sum
+++ b/go.sum
@@ -19,8 +19,12 @@ cloud.google.com/go/bigquery v1.4.0/go.mod h1:S8dzgnTigyfTmLBfrtrhyYhwRxG72rYxvf
 cloud.google.com/go/bigquery v1.5.0/go.mod h1:snEHRnqQbz117VIFhE8bmtwIDY80NLUZUMb4Nv6dBIg=
 cloud.google.com/go/bigquery v1.7.0/go.mod h1://okPTzCYNXSlb24MZs83e2Do+h+VXtc4gLoIoXIAPc=
 cloud.google.com/go/bigquery v1.8.0/go.mod h1:J5hqkt3O0uAFnINi6JXValWIb1v0goeZM77hZzJN/fQ=
+cloud.google.com/go/compute v1.9.0 h1:ED/FP4xv8GJw63v556/ASNc1CeeLUO2Bs8nzaHchkHg=
+cloud.google.com/go/compute v1.9.0/go.mod h1:lWv1h/zUWTm/LozzfTJhBSkd6ShQq8la8VeeuOEGxfY=
 cloud.google.com/go/compute v1.12.1 h1:gKVJMEyqV5c/UnpzjjQbo3Rjvvqpr9B1DFSbJC4OXr0=
 cloud.google.com/go/compute v1.12.1/go.mod h1:e8yNOBcBONZU1vJKCvCoDw/4JQsA0dpM4x/6PIIOocU=
+cloud.google.com/go/compute/metadata v0.2.0 h1:nBbNSZyDpkNlo3DepaaLKVuO7ClyifSAmNloSCZrHnQ=
+cloud.google.com/go/compute/metadata v0.2.0/go.mod h1:zFmK7XCadkQkj6TtorcaGlCW1hT1fIilQDwofLpJ20k=
 cloud.google.com/go/compute/metadata v0.2.1 h1:efOwf5ymceDhK6PKMnnrTHP4pppY5L22mle96M1yP48=
 cloud.google.com/go/compute/metadata v0.2.1/go.mod h1:jgHgmJd2RKBGzXqF5LR2EZMGxBkeanZ9wwa75XHJgOM=
 cloud.google.com/go/datastore v1.0.0/go.mod h1:LXYbyblFSglQ5pkeyhO+Qmw7ukd3C+pD7TKLgZqpHYE=
@@ -141,6 +145,8 @@ github.com/go-openapi/swag v0.19.5/go.mod h1:POnQmlKehdgb5mhVOsnJFsivZCEZ/vjK9gh
:
longwuyuan commented 1 year ago

@nat-leo , you can help by providing one update. What is needed is a very specific quirky effort from you. After your make fails, look for git status and then if you see changed "go.mod" and "go.sum", then check it in locally on your branch. After that run the make command again, this time with a "go.mod & go.sum", that have deviated from project and contain diff like I pasted above (or other diff).

If a file other than go.mod and go.sum change after your first run of make, then please update.

Please do this on a fresh clean environment that does not ginkgo installed or the .modcache dir to begin with etc

longwuyuan commented 1 year ago

cc @rikatz @tao12345666333 @strongjz

nat-leo commented 1 year ago

@longwuyuan I'll be sending this message to the the Slack ingress-nginx dev channel too.

TLDR

Downgrading from go1.19.5 -> go1.16.15 resulted in make kind-e2e-test running all the required tests without causing the other make commands in the Makefile to break, and adding the test harness feature #9467 may be unblocked.

The attempted fix

Tidying up didn't work too well, make kind-e2e-test resulting in: Ambiguous error

Failed to compile e2e:

../../.modcache/golang.org/x/oauth2@v0.3.0/google/default.go:17:2: ambiguous import: found package cloud.google.com/go/compute/metadata in multiple modules:
    cloud.google.com/go/compute v1.9.0 (/go/src/k8s.io/ingress-nginx/.modcache/cloud.google.com/go/compute@v1.9.0/metadata)
    cloud.google.com/go/compute/metadata v0.2.0 (/go/src/k8s.io/ingress-nginx/.modcache/cloud.google.com/go/compute/metadata@v0.2.0)

and changed go.mod and go.sum, which was the current expected behavior.

On branch main
Your branch is up to date with 'origin/main'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
    modified:   go.mod
    modified:   go.sum

After checking in the new go.mod and go.sum files and running make kind-e2e-test again resulted in the same ambiguous import as above.

Running go mod tidy to clean up the go.mod and go.sum was the next thing to try. The result was the compile asking to try tidy again:

>>> go mod tidy
...
>>> make kind-e2e-test
...
Failed to compile e2e:

go: updates to go.mod needed; to update it:
    go mod tidy
>>> go mod tidy
go: finding module for package google.golang.org/grpc/examples/features/proto/echo
go: finding module for package google.golang.org/grpc/examples/helloworld/helloworld
go: found google.golang.org/grpc/examples/features/proto/echo in google.golang.org/grpc/examples v0.0.0-20230113182548-78ddc05d9b33
go: found google.golang.org/grpc/examples/helloworld/helloworld in google.golang.org/grpc/examples v0.0.0-20230113182548-78ddc05d9b33
>>> make kind-e2e-test
...
Failed to compile e2e:

../../.modcache/golang.org/x/oauth2@v0.4.0/google/default.go:17:2: missing go.sum entry for module providing package cloud.google.com/go/compute/metadata (imported by golang.org/x/oauth2/google); to add:
    go get golang.org/x/oauth2/google@v0.4.0
../../.modcache/github.com/moul/pb@v0.0.0-20220425114252-bca18df4138c/grpcbin/go-grpc/grpcbin.pb.go:25:2: missing go.sum entry for module providing package google.golang.org/grpc (imported by k8s.io/ingress-nginx/test/e2e/annotations); to add:
    go get k8s.io/ingress-nginx/test/e2e/annotations
annotations/grpc.go:30:2: missing go.sum entry for module providing package google.golang.org/grpc/credentials (imported by k8s.io/ingress-nginx/test/e2e/annotations); to add:
    go get k8s.io/ingress-nginx/test/e2e/annotations
annotations/grpc.go:31:2: missing go.sum entry for module providing package google.golang.org/grpc/metadata (imported by k8s.io/ingress-nginx/test/e2e/annotations); to add:
    go get k8s.io/ingress-nginx/test/e2e/annotations

This final section here shows off what I believe to an issue in running the make command, running go get golang.org/x/oauth2/google@v0.4.0 manually as recommended by the compiler above freezes:

>>>go get golang.org/x/oauth2/google@v0.4.0
go: downloading cloud.google.com/go v0.108.0
go: downloading cloud.google.com/go/compute v1.14.0

And running go install golang.org/x/oauth2/google@v0.4.0 instead is not possible!:

>>>go install golang.org/x/oauth2/google@v0.4.0
package golang.org/x/oauth2/google is not a main package

Finally

I found that the go get command had been deprecated in go1.17 and onwards -- the command would only update entries in go.mod and go.sum files while running with the latest go version. This meant that running make kind-e2e-test with go1.19, resulted in a catch-22 of not being able to install part of a package because go get can no longer install part of a package like it used to, and go install can only install a main package.

The Fix:

Downgrading to go1.16, the last version before go get started being deprecated and losing responsibilities resulted in make kind-e2e-test running the tests perfectly fine, and also no breaking the other commands in the main Makefile.

note: I didn't check if go1.17 or go1.18 were able to compile and run make kind-e2e-test. The might be able to, given that the go get deprecation started at go1.17 and gradually lost responsibilities.

nat-leo commented 1 year ago

/assign

nat-leo commented 1 year ago

I think that setting up the new e2e test for static deployment for kind means:

  1. Adding a new job to our GitHub actions in .github/workflows/ci.yaml.
  2. More or less copying the format of the Kubernetes, Kubernetes-chroot, or helm jobs in the ci.yaml, but instead of running kind create cluster, run something like kubectl apply -f ./blob/main/deploy/static/provider/kind/deploy.yaml
strongjz commented 1 year ago

I think that setting up the new e2e test for static deployment for kind means:

  1. Adding a new job to our GitHub actions in .github/workflows/ci.yaml.
  2. More or less copying the format of the Kubernetes, Kubernetes-chroot, or helm jobs in the ci.yaml, but instead of running kind create cluster, run something like kubectl apply -f ./blob/main/deploy/static/provider/kind/deploy.yaml

Yes but also running the e2e test with that kind cluster, you use the skip cluster creation environment variable and it will use what's in the kubectl context.

nat-leo commented 1 year ago

The E2E testing suite is now set up for Kind Static Deployment. Some Kind Static Deployment tests don't pass yet, so I think that the pull request should stay on hold in order to not block other pull requests from passing their tests.

nat-leo commented 1 year ago

Update 1/27/23 11:57am

Currently the kind tests are failing at specifically framework/k8s.go on line 76:

 [FAILED] 
    Error Trace:    /k8s.go:76
                                /framework.go:394
                                /k8s.go:79
                                /proxy.go:186
                                /node.go:445
                                /suite.go:847
                                /asm_amd64.s:1594
    Error:          Expected nil, but got: &errors.errorString{s:"timed out waiting for the condition"}
    Test:           [Annotations] proxy-* should turn off proxy-request-buffering
    Messages:       creating ingress

  In [It] at: /go/src/k8s.io/ingress-nginx/test/e2e/framework/k8s.go:76 @ 01/27/23 19:44:47.572

  There were additional failures detected.  To view them in detail run ginkgo -vv

The error message in particular timed out waiting for the condition is found in kubernetes's apimachinery project. The error message comes from the wait.ExponentialBackoff function in pkg/util/wait.go. The function returns the error when the last retry for exponential backoff has occurred.

Changing exponential backoff or default timeout:

one test I tried was changing these two files: e2e/framework/k8s.go:376-382

const (
    // Parameters for retrying with exponential backoff.
    retryBackoffInitialDuration = 100 * time.Millisecond
    retryBackoffFactor          = 3
    retryBackoffJitter          = 0
    retryBackoffSteps           = 6
)

e2e/framework/util.go:40-46

const (
    // Poll how often to poll for conditions
    Poll = 2 * time.Second

    // DefaultTimeout time to wait for operations to complete
    DefaultTimeout = 90 * time.Second
)

Which would be odd if it worked, given that the other tests in the github workflow appear to be working completely fine -- especially odd since the kubernetes e2e test in ci.yaml is very similar to the actions for kind.

Not applying the kind deployment yaml file.

Tests run perfectly without running kubectl apply and then running the tests, which makes the test running exactly the same as the k8s workflow in ci.yaml. This combined with the timoeout errors when polling for a specific ingress leaves me suspicious of namespaces.

z1cheng commented 1 year ago

@strongjz It seems @nat-leo is no longer working on this issue, so please let me have a try.

Now I have some questions for the issue:

  1. I noticed E2E tests always try to initialize a new environment including a new IngressController using helm install before each test: https://github.com/kubernetes/ingress-nginx/blob/main/test/e2e/framework/framework.go#LL129C1-L145C2. So I think we can't just simply add kubectl apply as a step in ci.yaml

  2. The image tag in the static deployment yaml is "v1.8.0", but e2e testing tag is "1.0.0-dev", it seems we can't apply the yaml file directly

k8s-triage-robot commented 2 months ago

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted