Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[v1.11] test/k8sT: remove l7_demos test #23348

Merged
merged 1 commit into from
Jan 27, 2023
Merged

[v1.11] test/k8sT: remove l7_demos test #23348

merged 1 commit into from
Jan 27, 2023

Conversation

tklauser
Copy link
Member

Manual backport of #20619 to v1.11 branch. See #20619 (comment) and #23313 (comment) for context.

[ upstream commit 1db1156 ]

[ Backporter's note: manual backport because the containing directory got
renamed in commit ffd7e57 ("test: Standardize file naming for
tests") on master. ]

With cilium/cilium-cli#962 in place in cilium-cli v0.12.0 and the CI update to use that version in #20617, the connectivity tests cover all functionality tested by the tests in l7_demos.go. Moreover, the cilium-cli connectivity tests can be run against arbitrary clusters with Cilium deployed, while this test is specific for the test setup based on vagrant VMs. Thus, drop this test.

@tklauser tklauser added kind/backports This PR provides functionality previously merged into master. release-note/ci This PR makes changes to the CI. backport/1.11 This PR represents a backport for Cilium 1.11.x of a PR that was merged to main. labels Jan 25, 2023
@tklauser tklauser requested a review from a team as a code owner January 25, 2023 15:23
@tklauser
Copy link
Member Author

tklauser commented Jan 25, 2023 •
edited by maintainer-s-little-helper bot

/test-backport-1.11

Job 'Cilium-PR-K8s-1.22-kernel-4.19' hit: #22019 (90.65% similarity)

@tklauser tklauser mentioned this pull request Jan 25, 2023
Merged
18 tasks
michi-covalent
michi-covalent approved these changes Jan 25, 2023
View reviewed changes
Copy link
Contributor

@michi-covalent michi-covalent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@maintainer-s-little-helper maintainer-s-little-helper bot mentioned this pull request Jan 25, 2023
Closed
@maintainer-s-little-helper
Copy link

Commit 789e11eda0eed3c4dc6fd210e61714c2f324409c does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jan 25, 2023
@tklauser @joestringer
test/k8sT: remove l7_demos test
cebc0f7 
[ upstream commit 1db1156 ]

[ Backporter's note: manual backport because the containing directory got
  renamed in commit ffd7e57 ("test: Standardize file naming for
  tests") on master. ]

With cilium/cilium-cli#962 in place in cilium-cli v0.12.0 and the CI
update to use that version in #20617, the connectivity tests
cover all functionality tested by the tests in l7_demos.go. Moreover,
the cilium-cli connectivity tests can be run against arbitrary clusters
with Cilium deployed, while this test is specific for the test setup
based on vagrant VMs. Thus, drop this test.

Signed-off-by: Tobias Klauser <tobias@cilium.io>
@joestringer joestringer force-pushed the pr/tklauser/v1.11-test-k8s-remove-l7-demos branch from 789e11e to cebc0f7 Compare January 25, 2023 23:39
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jan 25, 2023
@joestringer
Copy link
Member

joestringer commented Jan 25, 2023 •
edited by maintainer-s-little-helper bot

/test-backport-1.11

Job 'Cilium-PR-K8s-GKE' failed:

Click to show.

Test Name

K8sServicesTest Checks E/W loadbalancing (ClusterIP, NodePort from inside cluster, etc) Checks service on same node

Failure Output

FAIL: Timed out after 240.001s.

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-GKE so I can create one.

@tklauser
Copy link
Member Author

tklauser commented Jan 26, 2023 •
edited

/test-gke

Previous run imed out due to Cilium pods still in ContainerCreating or Init state, presumably due to delays in image build: https://jenkins.cilium.io/job/Cilium-PR-K8s-GKE/9256/ Re-triggered to confirm.

@tklauser
Copy link
Member Author

/test-gke

Previous run imed out due to Cilium pods still in ContainerCreating or Init state, presumably due to delays in image build: https://jenkins.cilium.io/job/Cilium-PR-K8s-GKE/9256/ Re-triggered to confirm.

From https://jenkins.cilium.io/view/Cilium-v1.11/job/cilium-v1.11-gke/ it looks like that job is broken on v1.11 in general. I assume it's thus also not marked as required. Given the failure is unrelated to this PR, all other tests passed and approvals are in, I'll mark this as ready to merge.

@tklauser tklauser added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 26, 2023
@joestringer
Copy link
Member

I assume it's thus also not marked as required.

No need to assume, the UI does not label it as "Required":

image

@aanm aanm merged commit afdfec9 into v1.11 Jan 27, 2023
@aanm aanm deleted the pr/tklauser/v1.11-test-k8s-remove-l7-demos branch January 27, 2023 10:26
This was referenced Feb 13, 2023
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joestringer joestringer joestringer approved these changes

@michi-covalent michi-covalent michi-covalent approved these changes

Assignees
No one assigned
Labels
backport/1.11 This PR represents a backport for Cilium 1.11.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@tklauser @joestringer @michi-covalent @aanm