gha: Run kubernetes Conformance and SIG-network tests #24209

aojea · 2023-03-07T08:56:38Z

For first time contributors, read Submitting a pull request
All code is covered by unit and/or runtime tests where feasible.
All commits contain a well written commit description including a title,
description and a Fixes: #XXX line if the commit addresses a particular
GitHub issue.
If your commit description contains a Fixes: <commit-id> tag, then
please add the commit author[s] as reviewer[s] to this issue.
All commits are signed off. See the section Developer’s Certificate of Origin
Provide a title or release-note blurb suitable for the release notes.
Thanks for contributing!

The kubernetes e2e sig-network tests are created to standardize the expected behaviors for the different implementations.

Running them ensure that all the ecosystem provide the same experience for users and contribute to reduce toil to implementations, knowing that the behavior is conformance with the defined by Kubernetes.

aojea · 2023-03-07T08:57:04Z

/assign @squeed @sayboras

squeed

one very minor change, then this looks good to me.

squeed · 2023-03-07T09:39:04Z

.github/workflows/conformance-k8s-kind.yaml

+
+      - name: Install Cilium
+        run: |
+          cilium install ${{ steps.vars.outputs.cilium_install_defaults }}


This should have the --wait flag (although I bet kubetest also waits for all the nodes to be Ready). Still good practice.

check line 148, it sets it to false, for some reasons?

squeed · 2023-03-07T09:42:38Z

This has a lot of skips. Should we be creating issues to un-Skip any of them?

It seems like, at least, NetworkPolicy should be enabled.

aojea · 2023-03-07T09:51:18Z

--skip="Feature|Federation|PerformanceDNS|DualStack|Disruptive|Serial|KubeProxy|LoadBalancer|GCE|Netpol|NetworkPolicy|rejected|externalTrafficPolicy|HostPort|ExternalIP|kube-proxy"

This has a lot of skips. Should we be creating issues to un-Skip any of them?

It seems like, at least, NetworkPolicy should be enabled.

remember how did we have it in ovn-kubernetes?

I wanted to do something similiar, to read them from a file adding the issues , so we can work towards remove the list of skips to zero ... can I add this file in the repo? where would be the best place?

squeed · 2023-03-07T09:57:51Z

I wanted to do something similiar, to read them from a file adding the issues , so we can work towards remove the list of skips to zero ... can I add this file in the repo? where would be the best place?

I think this is fine for now; as we knock the skips down, we can consider moving it. I suspect that the skips will vary with certain configurations (e.g. dualstack, kpr, aws-cni chaining, etc).

aojea · 2023-03-07T10:03:47Z

I think this is fine for now; as we knock the skips down, we can consider moving it. I suspect that the skips will vary with certain configurations (e.g. dualstack, kpr, aws-cni chaining, etc).

do you want to add a matrix ipv4, ipv6 and dual-stack?
that is very simple to add here

sayboras

Thanks and LGTM 💯 🎖️

.github/workflows/conformance-k8s-kind.yaml

aojea · 2023-03-07T10:38:38Z

Last push removes the external dependency on the kind config and add a matrix to test ipv4, ipv6 and dual-stack.

I have this action setup running for years in several projects with success, we can always remove permutation from the matrix slice if we don't want to proceed with all of them

.github/workflows/conformance-k8s-kind.yaml

aojea · 2023-03-07T11:14:50Z

/test

sayboras · 2023-03-07T11:22:43Z

/test

test command only works for cilium committer sadly, so if you want to run full CI tests, feel free to ping us in development channel. I don't think this PR requires full CI tests (Jenkins + GHA) though as the changes are only for GHA.

squeed · 2023-03-07T15:04:41Z

3 failures, I assume #24174 and #24202 will take care of this

 Summarizing 3 Failures:
  [FAIL] [sig-network] Services [It] should be able to update service type to NodePort listening on same port number but different protocols
  test/e2e/network/service.go:1421
  [FAIL] [sig-network] Services [It] should be able to connect to terminating and unready endpoints if PublishNotReadyAddresses is true
  test/e2e/network/util.go:177
  [FAIL] [sig-network] Services [It] should create endpoints for unready pods

aditighag

Looks like some of the tests overlap with the existing k8s conformance test suite in cilium? Probably good to bring this up in the community meeting. /cc @aanm

joestringer · 2023-03-07T19:54:35Z

Looks like some of the tests overlap with the existing k8s conformance test suite in cilium? Probably good to bring this up in the community meeting. /cc @aanm

that seems to use vagrant, kind is much better suited for running k8s e2e tests since is one of the kubernetes default tool for testing the project itself and is tuned for performance, see the results

I agree - the current mechanism was built some years ago based on the infra we used at the time. It currently requires a special trigger phrase and relies on some additional custom Jenkins configuration. I like the idea of moving this to GHA instead.

The only thing I would suggest is to check the end of that script for which tests are being skipped + comments and maybe copy any relevant explanations+links here.

At a glance it also looks like that job tests Cilium with kube-proxy, whereas this workflow tests without kube-proxy. cf. #24199

joestringer · 2023-03-07T19:57:47Z

Do we need to run sig-storage etc.? Some of the reason behind the limited set of tests in the current script is that there's a bunch of stuff that is unrelated to Cilium's functionality.

aojea · 2023-03-07T20:57:46Z

Do we need to run sig-storage etc.? Some of the reason behind the limited set of tests in the current script is that there's a bunch of stuff that is unrelated to Cilium's functionality.

this regex that I added is specific for networking plugins, is the same regex I have in kubernetes/kubernetes and added to other networking plugins that reached out to me for this kind of situation

sayboras · 2023-03-07T21:33:47Z

ipv6 fails to install cilium https://github.com/cilium/cilium/actions/runs/4353373283/jobs/7607811754

You can take a look at the existing ipv6 (e.g. test-smoke-ipv6) jobs and kind-config-ipv6.yaml for reference. The last time I worked on this, I need the bump CIDR range as the default values might be too small. IMO, we can add ipv6 test once in subsequent PR.

Relates:
https://github.com/cilium/cilium/blob/master/.github/workflows/tests-smoke-ipv6.yaml#L129-L133
0ac8c4c

The kubernetes e2e sig-network tests are created to standardize the expected behaviors for the different implementations. Running them ensure that all the ecosystem provide the same experience for users and contribute to reduce toil to implementations, knowing that the behavior is conformance with the defined by Kubernetes. Signed-off-by: Antonio Ojea <aojea@google.com>

aojea · 2023-03-07T22:31:46Z

Since my inmidiate priority is to have a Kubernetes job running I prefer to focus on having just ipv4 and monitor the job for some weeks so you can be confident is stable, I tested it locally right now and with my two PRs that got merged there are no failing tests


Ran 371 of 7084 Specs in 682.514 seconds
SUCCESS! -- 371 Passed | 0 Failed | 0 Pending | 6713 Skipped

aojea · 2023-03-07T23:30:44Z

and this is green now 👍

sayboras

One small comment, the rest looks good to me. Thanks.

.github/workflows/conformance-k8s-kind.yaml

sayboras · 2023-03-08T11:23:55Z

Full CI is not required as the changes in this PR are to add a new GHA, which is successfully run.

christarazi · 2023-03-08T19:47:12Z

Removing myself in favor of @nbusseneau

aojea · 2023-03-14T17:20:54Z

just for clarification, both the kubernetes and e2e tests are pinned to a specific version, there is no possibility a test that was passing start to fail if is not related to a change in the cilium code or an environment problem (but this runners seems very powerful )

squeed · 2023-03-15T14:24:32Z

@aditighag @brlbil @nbusseneau would you mind reviewing this PR?

aditighag · 2023-03-17T19:35:19Z

I think this is good to go, but let's wait for @nbusseneau's review.

aditighag · 2023-03-20T15:54:43Z

It's been a few days since the pending reviews were requested, so let's get the PR merged. @nbusseneau Please follow up with @aojea if you have any follow-ups in mind.

nbusseneau · 2023-03-20T17:10:13Z

Sorry, I missed the pings last week. Had a review and LGTM, great job, thanks!

aojea · 2023-03-20T21:30:27Z

Thanks, I'll be available for any issue related with this job

Per the contributor ladder document https://github.com/cilium/community/blob/main/CONTRIBUTOR-LADDER.md#organization-member I'd like to request membership in the org for myself, since I plan to contribute regularly, mainly in the areas related to Kubernetes where I'm sig-network and sig-testing member and Kind maintainer. I've already contributed adding more test coverage for the Kubernetes integration to the project - cilium/cilium#25258 - cilium/cilium#24209 fixing some bugs: - cilium/cilium#24202 - cilium/cilium#24174 and helping to triage issues Signed-off-by: Antonio Ojea <antonio.ojea.garcia@gmail.com>

aojea requested review from a team as code owners March 7, 2023 08:56

aojea requested review from brlbil and christarazi March 7, 2023 08:56

maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Mar 7, 2023

aojea requested a review from aditighag March 7, 2023 08:56

github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Mar 7, 2023

aojea mentioned this pull request Mar 7, 2023

EndpointSlice Conditions semantics: Ready, Serving and Terminating #24174

Merged

7 tasks

squeed added the release-note/ci This PR makes changes to the CI. label Mar 7, 2023

maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Mar 7, 2023

squeed requested changes Mar 7, 2023

View reviewed changes

sayboras approved these changes Mar 7, 2023

View reviewed changes

.github/workflows/conformance-k8s-kind.yaml Outdated Show resolved Hide resolved

aojea force-pushed the ci branch from 4d7e6cb to 1a0adf7 Compare March 7, 2023 10:37

aojea force-pushed the ci branch from 1a0adf7 to ac3e822 Compare March 7, 2023 10:39

aojea commented Mar 7, 2023

View reviewed changes

.github/workflows/conformance-k8s-kind.yaml Outdated Show resolved Hide resolved

aojea force-pushed the ci branch 2 times, most recently from 8b8f73b to 167f6b0 Compare March 7, 2023 10:57

maintainer-s-little-helper bot requested a review from squeed March 7, 2023 14:03

squeed approved these changes Mar 7, 2023

View reviewed changes

aditighag reviewed Mar 7, 2023

View reviewed changes

aojea force-pushed the ci branch 2 times, most recently from 87bb8bb to feeafd9 Compare March 7, 2023 21:50

aojea force-pushed the ci branch from feeafd9 to f906018 Compare March 7, 2023 21:52

sayboras approved these changes Mar 8, 2023

View reviewed changes

.github/workflows/conformance-k8s-kind.yaml Show resolved Hide resolved

sayboras changed the title ~~Run kubernetes Conformance and SIG-network tests~~ gha: Run kubernetes Conformance and SIG-network tests Mar 8, 2023

sayboras added the area/CI Continuous Integration testing issue or flake label Mar 8, 2023

nbusseneau self-requested a review March 8, 2023 16:10

christarazi removed their request for review March 8, 2023 19:46

sayboras requested a review from aditighag March 15, 2023 14:26

brlbil approved these changes Mar 17, 2023

View reviewed changes

aditighag approved these changes Mar 17, 2023

View reviewed changes

aditighag merged commit caf0da8 into cilium:master Mar 20, 2023

aojea mentioned this pull request May 17, 2023

Request org membership for aojea cilium/community#26

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gha: Run kubernetes Conformance and SIG-network tests #24209

gha: Run kubernetes Conformance and SIG-network tests #24209

aojea commented Mar 7, 2023 •

edited by sayboras

aojea commented Mar 7, 2023

squeed left a comment

squeed Mar 7, 2023

aojea Mar 7, 2023

squeed commented Mar 7, 2023

aojea commented Mar 7, 2023

squeed commented Mar 7, 2023

aojea commented Mar 7, 2023

sayboras left a comment

aojea commented Mar 7, 2023

aojea commented Mar 7, 2023

sayboras commented Mar 7, 2023 •

edited

squeed commented Mar 7, 2023

aditighag left a comment

joestringer commented Mar 7, 2023

joestringer commented Mar 7, 2023

aojea commented Mar 7, 2023 •

edited

sayboras commented Mar 7, 2023

aojea commented Mar 7, 2023

aojea commented Mar 7, 2023

sayboras left a comment

sayboras commented Mar 8, 2023

christarazi commented Mar 8, 2023

aojea commented Mar 14, 2023

squeed commented Mar 15, 2023

aditighag commented Mar 17, 2023

aditighag commented Mar 20, 2023

nbusseneau commented Mar 20, 2023

aojea commented Mar 20, 2023

gha: Run kubernetes Conformance and SIG-network tests #24209

gha: Run kubernetes Conformance and SIG-network tests #24209

Conversation

aojea commented Mar 7, 2023 • edited by sayboras

aojea commented Mar 7, 2023

squeed left a comment

Choose a reason for hiding this comment

squeed Mar 7, 2023

Choose a reason for hiding this comment

aojea Mar 7, 2023

Choose a reason for hiding this comment

squeed commented Mar 7, 2023

aojea commented Mar 7, 2023

squeed commented Mar 7, 2023

aojea commented Mar 7, 2023

sayboras left a comment

Choose a reason for hiding this comment

aojea commented Mar 7, 2023

aojea commented Mar 7, 2023

sayboras commented Mar 7, 2023 • edited

squeed commented Mar 7, 2023

aditighag left a comment

Choose a reason for hiding this comment

joestringer commented Mar 7, 2023

joestringer commented Mar 7, 2023

aojea commented Mar 7, 2023 • edited

sayboras commented Mar 7, 2023

aojea commented Mar 7, 2023

aojea commented Mar 7, 2023

sayboras left a comment

Choose a reason for hiding this comment

sayboras commented Mar 8, 2023

christarazi commented Mar 8, 2023

aojea commented Mar 14, 2023

squeed commented Mar 15, 2023

aditighag commented Mar 17, 2023

aditighag commented Mar 20, 2023

nbusseneau commented Mar 20, 2023

aojea commented Mar 20, 2023

aojea commented Mar 7, 2023 •

edited by sayboras

sayboras commented Mar 7, 2023 •

edited

aojea commented Mar 7, 2023 •

edited