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

Rework status Conditions for Globalnet resources #2487

Merged
merged 1 commit into from
Jul 31, 2023
Merged

Rework status Conditions for Globalnet resources #2487

merged 1 commit into from
Jul 31, 2023

Conversation

tpantelis
Copy link
Contributor

The status Conditions are appended to preserve the history however this is contrary to the intended design where each condition type should be unique and only the last status is preserved. Instead of appending, use meta.SetStatusCondition. Also trim existing conditions on upgrade to contain only the last one.

Fixes #2463

@tpantelis tpantelis self-assigned this May 18, 2023
@submariner-bot
Copy link
Contributor

🤖 Created branch: z_pr2487/tpantelis/gn_status_conditions
🚀 Full E2E won't run until the "ready-to-test" label is applied. I will add it automatically once the PR has 2 approvals, or you can add it manually.

@stale
Copy link

stale bot commented Jun 10, 2023

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Jun 10, 2023
@tpantelis tpantelis removed the wontfix This will not be worked on label Jun 10, 2023
sridhargaddam
sridhargaddam reviewed Jul 28, 2023
View reviewed changes
Copy link
Member

@sridhargaddam sridhargaddam left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @tpantelis . Some minor comments, otherwise LGTM.

pkg/globalnet/controllers/base_controllers.go Show resolved Hide resolved
pkg/globalnet/controllers/cluster_egressip_controller_test.go Show resolved Hide resolved
pkg/globalnet/controllers/global_egressip_controller_test.go Outdated
It("should trim the Status conditions", func() {
Eventually(func() []metav1.Condition {
return getGlobalEgressIPStatus(t.globalEgressIPs, existing.Name).Conditions
}, time.Second).Should(HaveLen(1))
Copy link
Member

Choose a reason for hiding this comment

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

same here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it to call awaitStatusConditions that verifies the expected condition fields in addition that there's only one.

pkg/globalnet/controllers/global_ingressip_controller_test.go Outdated
It("should trim the Status conditions", func() {
Eventually(func() []metav1.Condition {
return t.getGlobalIngressIPStatus(existing.Name).Conditions
}, time.Second).Should(HaveLen(1))
Copy link
Member

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here

tpantelis added a commit to tpantelis/submariner-website that referenced this pull request Jul 31, 2023
@tpantelis
Release note for Globalnet status condition change
963a02c 
by submariner-io/submariner#2487.

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
@tpantelis tpantelis mentioned this pull request Jul 31, 2023
Merged
@tpantelis
Rework status Conditions for Globalnet resources
55c9c7d 
The status Conditions are appended to preserve the history however
this is contrary to the intended design where each condition type
should be unique and only the last status is preserved. Instead of
appending, use meta.SetStatusCondition. Also trim existing conditions
on upgrade to contain only the last one.

Fixes submariner-io#2463

Signed-off-by: Automated Release <release@submariner.io>
@tpantelis tpantelis mentioned this pull request Jul 31, 2023
Merged
@submariner-bot submariner-bot added the ready-to-test When a PR is ready for full E2E testing label Jul 31, 2023
tpantelis added a commit to tpantelis/submariner-website that referenced this pull request Jul 31, 2023
@tpantelis
Release note for Globalnet status condition change
391cfd7 
by submariner-io/submariner#2487.

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
@tpantelis tpantelis merged commit 3b162b5 into submariner-io:devel Jul 31, 2023
@submariner-bot
Copy link
Contributor

🤖 Closed branches: [z_pr2487/tpantelis/gn_status_conditions]

dfarrell07 pushed a commit to submariner-io/submariner-website that referenced this pull request Aug 2, 2023
@tpantelis @dfarrell07
Release note for Globalnet status condition change
dbbde20 
by submariner-io/submariner#2487.

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
dfarrell07 pushed a commit to dfarrell07/submariner-website that referenced this pull request Oct 18, 2023
@tpantelis @dfarrell07
Release note for Globalnet status condition change
f2d98bc 
by submariner-io/submariner#2487.

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
dfarrell07 pushed a commit to dfarrell07/submariner-website that referenced this pull request Oct 18, 2023
@tpantelis @dfarrell07
Release note for Globalnet status condition change
4247219 
by submariner-io/submariner#2487.

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
dfarrell07 pushed a commit to dfarrell07/submariner-website that referenced this pull request Oct 19, 2023
@tpantelis @dfarrell07
Release note for Globalnet status condition change
0cd07d2 
by submariner-io/submariner#2487.

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
tpantelis added a commit to submariner-io/submariner-website that referenced this pull request Oct 22, 2023
@tpantelis
Release note for Globalnet status condition change
1b809ab 
by submariner-io/submariner#2487.

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
@tpantelis tpantelis deleted the gn_status_conditions branch October 24, 2023 12:24
tpantelis added a commit to tpantelis/submariner-website that referenced this pull request Nov 7, 2023
@tpantelis
Release note for Globalnet status condition change
992e673 
by submariner-io/submariner#2487.

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aswinsuryan aswinsuryan aswinsuryan approved these changes

@sridhargaddam sridhargaddam sridhargaddam approved these changes

@Oats87 Oats87 Awaiting requested review from Oats87 Oats87 is a code owner

@skitt skitt Awaiting requested review from skitt skitt is a code owner

Assignees

@tpantelis tpantelis

Labels
ready-to-test When a PR is ready for full E2E testing release-note-handled
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rework status Conditions for Globalnet resources
5 participants
@tpantelis @submariner-bot @sridhargaddam @aswinsuryan @dfarrell07