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.

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

Record an event when Egress IP assignment changes #5765

Merged
merged 1 commit into from
Jan 2, 2024

Conversation

jainpulkit22
Copy link
Contributor

@jainpulkit22 jainpulkit22 commented Dec 1, 2023

Fixes #4614.

Record event when EgressIP is assigned to the Node interface.

@jainpulkit22 jainpulkit22 force-pushed the egress-api-visibility branch from d23db4b to ee8a7e4 Compare December 1, 2023 12:54
@rajnkamr rajnkamr added the area/transit/egress Issues or PRs related to Egress (SNAT for traffic egressing the cluster). label Dec 6, 2023
@rajnkamr rajnkamr added this to the Antrea v1.15 release milestone Dec 6, 2023
@jainpulkit22 jainpulkit22 force-pushed the egress-api-visibility branch 2 times, most recently from 408448f to f2cb840 Compare December 11, 2023 06:36
@jainpulkit22 jainpulkit22 marked this pull request as ready for review December 11, 2023 08:40
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

Can you validate the event is received in e2e testEgressCRUD?

pkg/agent/controller/egress/egress_controller.go Outdated Show resolved Hide resolved
pkg/agent/controller/egress/egress_controller.go Outdated Show resolved Hide resolved
@jainpulkit22 jainpulkit22 requested a review from tnqn December 13, 2023 09:55
pkg/agent/controller/egress/egress_controller.go Outdated Show resolved Hide resolved
pkg/agent/controller/egress/egress_controller.go Outdated Show resolved Hide resolved
pkg/agent/controller/egress/egress_controller.go Outdated Show resolved Hide resolved
pkg/agent/controller/egress/egress_controller.go Outdated Show resolved Hide resolved
@jainpulkit22 jainpulkit22 force-pushed the egress-api-visibility branch 5 times, most recently from f20c9b2 to 169f508 Compare December 20, 2023 08:55
pkg/agent/controller/egress/egress_controller.go Outdated Show resolved Hide resolved
pkg/agent/controller/egress/egress_controller.go Outdated Show resolved Hide resolved
pkg/agent/controller/egress/egress_controller.go Outdated Show resolved Hide resolved
pkg/agent/controller/egress/egress_controller.go Outdated Show resolved Hide resolved
pkg/agent/controller/egress/egress_controller.go Outdated Show resolved Hide resolved
pkg/agent/controller/egress/egress_controller.go Outdated Show resolved Hide resolved
pkg/agent/controller/egress/egress_controller.go Outdated Show resolved Hide resolved
pkg/agent/ipassigner/ip_assigner.go Show resolved Hide resolved
test/e2e/egress_test.go Outdated Show resolved Hide resolved
test/e2e/egress_test.go Outdated Show resolved Hide resolved
@jainpulkit22 jainpulkit22 force-pushed the egress-api-visibility branch 3 times, most recently from 8320b84 to d28a2d1 Compare December 23, 2023 03:33
@jainpulkit22 jainpulkit22 marked this pull request as draft December 23, 2023 03:34
@jainpulkit22 jainpulkit22 marked this pull request as ready for review December 26, 2023 07:15
@jainpulkit22 jainpulkit22 requested a review from tnqn December 26, 2023 07:15
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

The title could be more specific, like "Record an event when Egress IP assignment changes"

pkg/agent/controller/egress/egress_controller.go Outdated Show resolved Hide resolved
pkg/agent/controller/egress/egress_controller.go Outdated Show resolved Hide resolved
pkg/agent/controller/egress/egress_controller.go Outdated Show resolved Hide resolved
pkg/agent/controller/egress/egress_controller.go Outdated Show resolved Hide resolved
pkg/agent/ipassigner/ip_assigner.go Outdated Show resolved Hide resolved
pkg/agent/ipassigner/ip_assigner.go Outdated Show resolved Hide resolved
pkg/agent/ipassigner/ip_assigner.go Outdated Show resolved Hide resolved
pkg/agent/ipassigner/ip_assigner.go Outdated Show resolved Hide resolved
pkg/agent/ipassigner/ip_assigner.go Outdated Show resolved Hide resolved
@jainpulkit22 jainpulkit22 force-pushed the egress-api-visibility branch from b11d5c4 to fe760bc Compare January 2, 2024 05:19
@jainpulkit22 jainpulkit22 changed the title Improve Egress API visibility Record an event when Egress IP assignment changes Jan 2, 2024
@jainpulkit22 jainpulkit22 force-pushed the egress-api-visibility branch 2 times, most recently from 6720c1a to 1a7e460 Compare January 2, 2024 05:22
@jainpulkit22 jainpulkit22 requested review from tnqn and luolanzone and removed request for luolanzone January 2, 2024 06:31
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

It should add patch and update permission of events, otherwise it can't correctly record events in some case:

I0102 05:49:41.364352       1 event.go:294] "Event occurred" object="egress-pirate-pods" fieldPath="" kind="Egress" apiVersion="crd.antrea.io/v1beta1" type="Normal" reason="IPAssigned" message="Assigned Egress egress-pirate-pods with IP 172.18.0.10 on Node kind-worker"
E0102 05:49:41.366451       1 event.go:267] Server rejected event '&v1.Event{TypeMeta:v1.TypeMeta{Kind:"", APIVersion:""}, ObjectMeta:v1.ObjectMeta{Name:"egress-pirate-pods.17a671a773e1d992", GenerateName:"", Namespace:"default", SelfLink:"", UID:"", ResourceVersion:"1122", Generation:0, CreationTimestamp:time.Date(1, time.January, 1, 0, 0, 0, 0, time.UTC), DeletionTimestamp:<nil>, DeletionGracePeriodSeconds:(*int64)(nil), Labels:map[string]string(nil), Annotations:map[string]string(nil), OwnerReferences:[]v1.OwnerReference(nil), Finalizers:[]string(nil), ManagedFields:[]v1.ManagedFieldsEntry(nil)}, InvolvedObject:v1.ObjectReference{Kind:"Egress", Namespace:"", Name:"egress-pirate-pods", UID:"9a3312c6-c90f-446d-b112-d6af3b43d330", APIVersion:"crd.antrea.io/v1beta1", ResourceVersion:"1266", FieldPath:""}, Reason:"IPAssigned", Message:"Assigned Egress egress-pirate-pods with IP 172.18.0.10 on Node kind-worker", Source:v1.EventSource{Component:"AntreaAgentEgressController", Host:""}, FirstTimestamp:time.Date(2024, time.January, 2, 5, 47, 53, 0, time.Local), LastTimestamp:time.Date(2024, time.January, 2, 5, 49, 41, 364053810, time.Local), Count:2, Type:"Normal", EventTime:time.Date(1, time.January, 1, 0, 0, 0, 0, time.UTC), Series:(*v1.EventSeries)(nil), Action:"", Related:(*v1.ObjectReference)(nil), ReportingController:"", ReportingInstance:""}': 'events "egress-pirate-pods.17a671a773e1d992" is forbidden: User "system:serviceaccount:kube-system:antrea-agent" cannot patch resource "events" in API group "" in the namespace "default"' (will not retry!)

@jainpulkit22 jainpulkit22 force-pushed the egress-api-visibility branch from 1a7e460 to c2d24ad Compare January 2, 2024 08:56
Record an event when EgressIP is assigned to the Node interface or when
it is unassigned from the Node interface.

Signed-off-by: Pulkit Jain <jainpu@vmware.com>
@jainpulkit22 jainpulkit22 force-pushed the egress-api-visibility branch from c2d24ad to c07d213 Compare January 2, 2024 10:02
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM

@tnqn
Copy link
Member

tnqn commented Jan 2, 2024

/test-all

@jainpulkit22 jainpulkit22 removed the request for review from Atish-iaf January 2, 2024 11:56
@tnqn
Copy link
Member

tnqn commented Jan 2, 2024

/skip-conformance which failed due to a service IP overlapping with server IP

@tnqn
Copy link
Member

tnqn commented Jan 2, 2024

/skip-conformance

@tnqn tnqn added the action/release-note Indicates a PR that should be included in release notes. label Jan 2, 2024
@tnqn tnqn merged commit 327cb03 into antrea-io:main Jan 2, 2024
48 of 53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/release-note Indicates a PR that should be included in release notes. area/transit/egress Issues or PRs related to Egress (SNAT for traffic egressing the cluster).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve Egress API visibility
4 participants