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

Update policy status on CRD creation/removal #1304

Merged
merged 3 commits into from
Dec 5, 2023

Conversation

c-kruse
Copy link
Contributor

@c-kruse c-kruse commented Dec 2, 2023

Issue #1291

Updates the flow controller to periodically poll for changes to the
skupper policy CRD, and to send a flow record to update the site.

Changes the collector to update network status when an update to a site
is detected.

@c-kruse
Copy link
Contributor Author

c-kruse commented Dec 2, 2023

Need to do some testing first, but hope this fixes #1291.

@c-kruse c-kruse force-pushed the fix/update-policy-status branch from 45abe2e to dc72039 Compare December 3, 2023 22:06
Issue skupperproject#1291

Updates the flow controller to periodically poll for changes to the
skupper policy CRD, and to send a flow record to update the site.

Changes the collector to update network status when an update to a site
is detected.

Signed-off-by: c-kruse <[email protected]>
@c-kruse c-kruse force-pushed the fix/update-policy-status branch from dc72039 to 9e1dc84 Compare December 4, 2023 15:57
@c-kruse c-kruse marked this pull request as ready for review December 4, 2023 17:31
@c-kruse c-kruse changed the title Update policy status on CRD creation/removal (#1291) Update policy status on CRD creation/removal Dec 4, 2023
@c-kruse
Copy link
Contributor Author

c-kruse commented Dec 4, 2023

Would appreciate someone else doing a manual end to end test on this, or maybe better yet pairing with me on that.

It appears to work as I would expect when isolated, but when it was hooked up to multiple sites and services there was a bit of drama after adding the CRDs. I think it may just be the way policies behave, but want to be sure this isn't introducing any bad behavior.

@c-kruse c-kruse requested a review from nluaces December 4, 2023 18:12
Copy link
Member

@fgiorgetti fgiorgetti left a comment

Choose a reason for hiding this comment

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

A couple of minor comments, but overall it seems to work fine.
Nice job @c-kruse !

pkg/flow/controller.go Outdated Show resolved Hide resolved
pkg/flow/controller.go Outdated Show resolved Hide resolved
Signed-off-by: Christian Kruse <[email protected]>
Copy link
Member

@ajssmith ajssmith left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

@nluaces nluaces linked an issue Dec 5, 2023 that may be closed by this pull request
@c-kruse c-kruse merged commit c0e57d3 into skupperproject:main Dec 5, 2023
1 check passed
nluaces pushed a commit that referenced this pull request Dec 6, 2023
Updates the flow controller to periodically poll for changes to the
skupper policy CRD, and to send a flow record to update the site.

Changes the collector to update network status when an update to a site
is detected.

Closes issue 1291
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Policy status is not being updated
4 participants