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

Egress Gateway: Remove deprecated CENP CRD #21874

Merged
merged 2 commits into from
Dec 6, 2022
Merged

Egress Gateway: Remove deprecated CENP CRD #21874

merged 2 commits into from
Dec 6, 2022

Conversation

julianwiedmann
Copy link
Member

@julianwiedmann julianwiedmann commented Oct 25, 2022 •
edited
Loading

For v1.13 we want to drop the support for configuring EgressGW policies via CENP, and rely on CiliumEgressGatewayPolicy instead (introduced in v1.12).

We've been warning about this in the release notes and via deprecationWarning in the CENP CRD itself, so it shouldn't come as surprise to anyone.

egressgw: drop support for CiliumEgressNATPolicy

@julianwiedmann julianwiedmann added release-note/minor This PR changes functionality that users may find relevant to operating Cilium. feature/egress-gateway Impacts the egress IP gateway feature. labels Oct 25, 2022
@julianwiedmann
Copy link
Member Author

/test

@michaelasp michaelasp mentioned this pull request Oct 25, 2022
Merged
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann
Copy link
Member Author

/test-1.16-4.9

@julianwiedmann
Copy link
Member Author

[1.16-4.9 flaked with Unable to download helm chart v1.11 from GitHub]

@julianwiedmann julianwiedmann force-pushed the egressgw-crd branch 2 times, most recently from 32f1fb9 to e890e57 Compare November 21, 2022 14:45
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann
Copy link
Member Author

ci-aks seems to be hitting #22162.

@julianwiedmann julianwiedmann marked this pull request as ready for review November 22, 2022 10:56
@julianwiedmann julianwiedmann requested review from a team as code owners November 22, 2022 10:56
jibi
jibi approved these changes Nov 22, 2022
View reviewed changes
Copy link
Member

@jibi jibi left a comment

Choose a reason for hiding this comment

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

Looks good, thanks! 🚢 🚢

test/k8s/manifests.go
@@ -13,7 +13,7 @@ var (
DemoDaemonSet = helpers.Manifest{
Filename: "demo_ds.yaml",
Alternate: "demo_ds_local.yaml",
DaemonSetNames: []string{"testds", "testclient", "testclient-2"},
Copy link
Member

Choose a reason for hiding this comment

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

😢

@julianwiedmann
Copy link
Member Author

[also needed to drop the privileged tests for the old-style configuration without node selector]

kaworu
kaworu approved these changes Nov 24, 2022
View reviewed changes
Copy link
Member

@kaworu kaworu left a comment

Choose a reason for hiding this comment

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

Thanks @julianwiedmann, Helm changes LGTM.

joestringer
joestringer approved these changes Nov 29, 2022
View reviewed changes
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

LGTM for my codeowners.

Documentation/network/egress-gateway.rst Show resolved Hide resolved
pkg/egressgateway/manager_privileged_test.go Show resolved Hide resolved
@joestringer joestringer added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Nov 29, 2022
@joestringer joestringer mentioned this pull request Dec 1, 2022
Closed
@julianwiedmann
egressgw: remove deprecated CENP CRD
3295e74 
CiliumEgressNATPolicy was deprecated in v1.12, being replaced with
CiliumEgressGatewayPolicy. Remove all the related code & tests.

The cenp-sample policy for one of the e2e tests temporarily gets converted
to CEGP. Fully removing it requires touching some other e2e parts, so
splitting that off into a separate patch.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann
egressgw: test: remove duplicated test
a28e928 
With CENP gone, there's no need to run the same effective pod-to-external
connectivity test twice. Slim down the testConnectivity() routine again,
and remove the unused DaemonSet from the Demo deployment.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann julianwiedmann removed the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Dec 6, 2022
@julianwiedmann
Copy link
Member Author

julianwiedmann commented Dec 6, 2022 •
edited by maintainer-s-little-helper bot
Loading

/test

Job 'Cilium-PR-K8s-1.16-kernel-4.9' failed:

Click to show.

Test Name

K8sDatapathConfig Host firewall With VXLAN

Failure Output

FAIL: Found 1 io.cilium/app=operator logs matching list of errors that must be investigated:

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

Job 'Cilium-PR-K8s-1.26-kernel-net-next' failed:

Click to show.

Test Name

K8sDatapathServicesTest Checks N/S loadbalancing Tests with XDP, vxlan tunnel, SNAT and Random

Failure Output

FAIL: Can not connect to service "http://192.168.56.11:32718" from outside cluster (1/10)

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

@julianwiedmann
Copy link
Member Author

julianwiedmann commented Dec 6, 2022 •
edited by maintainer-s-little-helper bot
Loading

/mlh new-flake Cilium-PR-K8s-1.16-kernel-4.9

👍 created #22578

@julianwiedmann
Copy link
Member Author

julianwiedmann commented Dec 6, 2022 •
edited by maintainer-s-little-helper bot
Loading

/mlh new-flake Cilium-PR-K8s-1.26-kernel-net-next

👍 created #22579

@julianwiedmann
Copy link
Member Author

/test-1.16-4.9

@julianwiedmann
Copy link
Member Author

/test-1.26-net-next

@joestringer joestringer merged commit d95b0a0 into cilium:master Dec 6, 2022
@julianwiedmann julianwiedmann deleted the egressgw-crd branch December 7, 2022 07:14
@julianwiedmann julianwiedmann mentioned this pull request Dec 7, 2022
Closed
@joestringer joestringer mentioned this pull request Dec 22, 2022
Merged
sayboras added a commit to sayboras/cilium-cli that referenced this pull request Jan 5, 2023
@sayboras
vendor: Bump cilium to v1.13.x version
7476c47 
This is to include new CRDs introduced as part of v1.13. Also,
CiliumEgressNATPolicy is removed as part of the below PR.

Relates: cilium/cilium#21874
Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit to sayboras/cilium-cli that referenced this pull request Jan 5, 2023
@sayboras
vendor: Bump cilium to v1.13.x version
dc35e0c 
This is to include new CRDs introduced as part of v1.13. Also,
CiliumEgressNATPolicy is removed as part of the below PR.

Relates: cilium/cilium#21874
Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit to sayboras/cilium-cli that referenced this pull request Jan 24, 2023
@sayboras
vendor: Bump cilium to v1.13.x version
3c2d615 
This is to include new CRDs introduced as part of v1.13. Also,
CiliumEgressNATPolicy is no longer available in 1.13 as part of the below PR, so dynamic client is used instead.

Relates: cilium/cilium#21874
Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit to sayboras/cilium-cli that referenced this pull request Jan 24, 2023
@sayboras
vendor: Bump cilium to v1.13.x version
510d8fd 
This is to include new CRDs introduced as part of v1.13. Also,
CiliumEgressNATPolicy is no longer available in 1.13 as part of the below PR, so dynamic client is used instead.

Relates: cilium/cilium#21874
Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit to sayboras/cilium-cli that referenced this pull request Jan 24, 2023
@sayboras
vendor: Bump cilium to v1.13.x version
ba5ad86 
This is to include new CRDs introduced as part of v1.13. Also,
CiliumEgressNATPolicy is no longer available in 1.13 as part of the below PR, so dynamic client is used instead.

Relates: cilium/cilium#21874
Signed-off-by: Tam Mach <tam.mach@cilium.io>
tklauser pushed a commit to cilium/cilium-cli that referenced this pull request Jan 24, 2023
@sayboras @tklauser
vendor: Bump cilium to v1.13.x version
4a9e06d 
This is to include new CRDs introduced as part of v1.13. Also,
CiliumEgressNATPolicy is no longer available in 1.13 as part of the below PR, so dynamic client is used instead.

Relates: cilium/cilium#21874
Signed-off-by: Tam Mach <tam.mach@cilium.io>
michi-covalent pushed a commit to michi-covalent/cilium that referenced this pull request May 30, 2023
@sayboras @michi-covalent
vendor: Bump cilium to v1.13.x version
06340de 
This is to include new CRDs introduced as part of v1.13. Also,
CiliumEgressNATPolicy is no longer available in 1.13 as part of the below PR, so dynamic client is used instead.

Relates: cilium#21874
Signed-off-by: Tam Mach <tam.mach@cilium.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jibi jibi jibi approved these changes

@kaworu kaworu kaworu approved these changes

@joestringer joestringer joestringer approved these changes

@christarazi christarazi christarazi approved these changes

@ldelossa ldelossa ldelossa approved these changes

@nathanjsweet nathanjsweet Awaiting requested review from nathanjsweet nathanjsweet was automatically assigned from cilium/api

Assignees
No one assigned
Labels
feature/egress-gateway Impacts the egress IP gateway feature. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@julianwiedmann @jibi @kaworu @joestringer @christarazi @ldelossa