Skip to content

Skip unnecessary netbox updates#538

Open
MaIT-HA wants to merge 16 commits intomainfrom
feature/skip-unnecessary-netbox-updates
Open

Skip unnecessary netbox updates#538
MaIT-HA wants to merge 16 commits intomainfrom
feature/skip-unnecessary-netbox-updates

Conversation

@MaIT-HA
Copy link
Copy Markdown
Collaborator

@MaIT-HA MaIT-HA commented Mar 18, 2026

(Summary from CoPilot)

This PR reduces unnecessary NetBox reconcile updates by tracking status.lastUpdated for IpAddress, IpRange, and Prefix, and by skipping update calls when the NetBox last_updated timestamp and the CR Ready condition’s ObservedGeneration indicate everything is already in sync.

Changes:

  • Add Status.LastUpdated to IpAddress, IpRange, and Prefix CRDs/types and propagate NetBox LastUpdated into status from controllers.
  • Introduce utils.IsUpToDate and use it in NetBox API clients to skip update operations when LastUpdated + Ready condition + generation match.
  • Refactor Prefix API v3/v4 request building and add/extend unit + controller tests for the new behavior.

@MaIT-HA MaIT-HA requested a review from bruelea March 18, 2026 08:04
@MaIT-HA MaIT-HA force-pushed the feature/skip-unnecessary-netbox-updates branch 2 times, most recently from e00fa29 to dc4cd63 Compare March 18, 2026 10:33
@MaIT-HA MaIT-HA force-pushed the feature/skip-unnecessary-netbox-updates branch from dc4cd63 to efbe9b2 Compare March 18, 2026 10:39
@MaIT-HA MaIT-HA changed the title [Feature] Skip unnecessary netbox updates Skip unnecessary netbox updates Mar 18, 2026
@MaIT-HA MaIT-HA marked this pull request as draft March 18, 2026 13:20
@MaIT-HA MaIT-HA force-pushed the feature/skip-unnecessary-netbox-updates branch from 0a01c07 to fac22fe Compare March 26, 2026 08:38
@MaIT-HA MaIT-HA marked this pull request as ready for review March 26, 2026 10:04
@MaIT-HA MaIT-HA self-assigned this Mar 26, 2026
Copy link
Copy Markdown
Collaborator

@bruelea bruelea left a comment

Choose a reason for hiding this comment

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

The review is focused on the ip address controller, since the structure of the controllers are aligned the comments apply to all 3 controllers.

Thanks for adding extensive testing!

MaIT-HA and others added 2 commits March 27, 2026 11:06
Co-authored-by: bruelea <166021996+bruelea@users.noreply.github.com>
@MaIT-HA MaIT-HA force-pushed the feature/skip-unnecessary-netbox-updates branch from e5e8100 to d6491e9 Compare March 30, 2026 22:05
@MaIT-HA MaIT-HA requested a review from bruelea March 31, 2026 07:01
@MaIT-HA MaIT-HA force-pushed the feature/skip-unnecessary-netbox-updates branch from d6491e9 to db6ff95 Compare March 31, 2026 07:22
MaIT-HA and others added 3 commits March 31, 2026 21:39
Co-authored-by: bruelea <166021996+bruelea@users.noreply.github.com>
Co-authored-by: bruelea <166021996+bruelea@users.noreply.github.com>
@MaIT-HA MaIT-HA requested a review from Copilot April 1, 2026 08:49
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR reduces unnecessary NetBox reconcile updates by tracking status.lastUpdated for IpAddress, IpRange, and Prefix, and by skipping update calls when the NetBox last_updated timestamp and the CR Ready condition’s ObservedGeneration indicate everything is already in sync.

Changes:

  • Add Status.LastUpdated to IpAddress, IpRange, and Prefix CRDs/types and propagate NetBox LastUpdated into status from controllers.
  • Introduce utils.IsUpToDate and use it in NetBox API clients to skip update operations when LastUpdated + Ready condition + generation match.
  • Refactor Prefix API v3/v4 request building and add/extend unit + controller tests for the new behavior.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pkg/netbox/utils/helpers.go Adds IsUpToDate helper used to decide whether to skip updates.
pkg/netbox/api/prefix.go Updates Prefix reserve/update logic to return (resp, isUpToDate, err) and skip updates when up to date; refactors v3/v4 request building usage.
pkg/netbox/api/prefix_v4.go Extracts v4 writable request builder + v4 create/update helpers.
pkg/netbox/api/prefix_v4_test.go Adds tests for writablePrefixRequestV4.
pkg/netbox/api/prefix_v3.go Adjusts v3 prefix client return signature and adds v3 writable request builder.
pkg/netbox/api/prefix_v3_test.go Adds tests for v3 writable prefix request builder and updated signature.
pkg/netbox/api/prefix_test.go Extends Prefix composite-client tests for up-to-date skipping behavior and LastUpdated.
pkg/netbox/api/ip_range.go Updates IP range reserve/update logic to return (resp, isUpToDate, err) and skip updates when up to date.
pkg/netbox/api/ip_range_test.go Extends IP range tests for up-to-date skipping and LastUpdated.
pkg/netbox/api/ip_address.go Updates IP address reserve/update logic to return (resp, isUpToDate, err) and skip updates when up to date; uses NetBox LastUpdated.
pkg/netbox/api/ip_address_test.go Extends IP address tests for up-to-date skipping and LastUpdated.
internal/controller/prefix_controller.go Skips status updates when NetBox object is already up to date; persists status.lastUpdated.
internal/controller/iprange_controller.go Skips status updates when up to date; persists status.lastUpdated.
internal/controller/ipaddress_controller.go Skips status updates when up to date; persists status.lastUpdated.
internal/controller/ipaddress_controller_test.go Adds a controller test case for “skip update when already up to date”.
internal/controller/netbox_testdata_test.go Adds stable NetBox LastUpdated test data and an “up to date” CR helper.
config/crd/bases/netbox.dev_prefixes.yaml Adds status.lastUpdated field to Prefix CRD schema.
config/crd/bases/netbox.dev_ipranges.yaml Adds status.lastUpdated field to IpRange CRD schema.
config/crd/bases/netbox.dev_ipaddresses.yaml Adds status.lastUpdated field to IpAddress CRD schema.
api/v1/prefix_types.go Adds LastUpdated *metav1.Time to PrefixStatus.
api/v1/iprange_types.go Adds LastUpdated *metav1.Time to IpRangeStatus.
api/v1/ipaddress_types.go Adds LastUpdated *metav1.Time to IpAddressStatus.
api/v1/zz_generated.deepcopy.go Updates deep-copy logic for new LastUpdated fields.
Makefile Uses $(GOBIN)/ginkgo for integration tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

3 participants