OSAC-779, OSAC-780: fix PublicIP feedback controller and detach annotation

[akshaynadkarni]

Summary

Two fixes for the PublicIP delete and detach flows:

1. OSAC-779: Feedback controller uses DELETING (not RELEASING) when a PublicIP CR
   is being deleted. Bumps private-api BSR module to v0.0.61.

2. OSAC-780: Target-namespace annotation is only cleared when state is ALLOCATED
   (detach completed), not when state is ATTACHED (about to detach). The previous check
   (state != Releasing) failed because the annotation sync runs before the state
   transitions to RELEASING in the same reconcile loop.

Why

OSAC-779: The feedback controller set RELEASING for all deletions, but RELEASING is the
detach flow (ATTACHED -> RELEASING -> ALLOCATED). The fulfillment-service state machine
rejected ALLOCATED -> RELEASING, leaving the CR stuck in Deleting phase.

OSAC-780: The syncComputeInstanceTargetNamespaceAnnotation() function runs before the
state transition in handleUpdate(). When spec.computeInstance is cleared (detach trigger),
the annotation was deleted while state was still ATTACHED (not yet RELEASING), so the
OSAC-776 preservation check never triggered. The AAP detach job then failed because the
annotation was missing.

Testing

go build ./...                                                           # pass
ginkgo run --focus="PublicIP" internal/controller                        # 89 passed, 0 failed

E2E verified on edge22: full lifecycle (create pool, create IP, attach, detach, delete)
completed successfully with both fixes applied.

Related PRs

• fulfillment-service PR #515 (merged): added PUBLIC_IP_STATE_DELETING to proto enum
• osac-aap PR (pending): relaxed detach playbook validation for spec.computeInstance

Tickets

• OSAC-779
• OSAC-780

Signed-off-by: akshaynadkarni 25892229+akshaynadkarni@users.noreply.github.com
Assisted-by: Cursor/Claude

[coderabbitai]

📝 Walkthrough

Walkthrough

Bumps private-api proto input to v0.0.61, changes feedback delete mapping to set public IP status to PUBLIC_IP_STATE_DELETING for non-Failed deletions, narrows when the publicip-target-namespace annotation is cleared (only when status == Allocated), and updates tests accordingly.

Changes

Public IP Deletion & Annotation Changes

Layer / File(s)	Summary
API Contract Update buf.gen.yaml	Private-api proto input bumped from v0.0.60 to v0.0.61 (buf.build/osac-project/private-api).
Deletion State Mapping internal/controller/publicip_feedback_controller.go	handleDelete now sets PUBLIC_IP_STATE_DELETING instead of PUBLIC_IP_STATE_RELEASING for non-Failed deletion paths.
Annotation Clear Condition internal/controller/publicip_controller.go	publicip-target-namespace annotation is cleared when spec.computeInstance is emptied only if PublicIP status == Allocated; preserved in Releasing/Attached detach phases.
Tests / Assertions internal/controller/publicip_feedback_controller_test.go, internal/controller/publicip_controller_test.go	Feedback deletion tests updated to expect PUBLIC_IP_STATE_DELETING; controller tests updated to require status==Allocated to clear the annotation and to assert preservation during Releasing/Attached detach cases.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• osac-project/osac-operator#200: Modifies the same PublicIP feedback reconciler deletion-state mapping logic and related tests.
• osac-project/osac-operator#234: Changes syncComputeInstanceTargetNamespaceAnnotation behavior and related tests for annotation preservation.
• osac-project/osac-operator#221: Touches state-to-private-API mapping and related PublicIP controller/feedback logic.

Suggested labels

lgtm, jira/valid-reference

Suggested reviewers

• eranco74
• adriengentil

Poem

🐰 I hopped through code with nimble paws,
From RELEASING to DELETING — new applause!
Tests aligned and proto bumped too,
Annotations stay when states aren't through.
A tiny hop, a stable cause.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title accurately reflects the main changes: fixing the PublicIP feedback controller to use the DELETING state (OSAC-779) and fixing the detach annotation handling (OSAC-780).
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[akshaynadkarni]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[openshift-ci-robot]

@akshaynadkarni: This pull request references OSAC-779 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

OSAC-779: Updates the PublicIP feedback controller to report DELETING (not RELEASING)
when a PublicIP CR is being deleted. Bumps private-api BSR module to v0.0.61.

Why

The feedback controller's handleDelete() was setting RELEASING for all deletions,
but RELEASING is the detach flow (ATTACHED -> RELEASING -> ALLOCATED). When deleting
an ALLOCATED PublicIP, the fulfillment-service rejected ALLOCATED -> RELEASING as an
invalid state transition, leaving the K8s CR stuck in Deleting phase.

DELETING is now a distinct state in the proto (added in fulfillment-service PR #515),
and this PR uses it in the operator.

Testing

go build ./...                                                           # pass
ginkgo run --focus="PublicIP" internal/controller                        # 88 passed, 0 failed
ginkgo run --focus="feedback.*PublicIP" internal/controller              # 4 passed, 0 failed

Tests updated:

• Two deletion tests now expect DELETING instead of RELEASING
• Detach test (syncState with Releasing) still expects RELEASING (unchanged)

Related PRs

• fulfillment-service PR #515 (merged): added PUBLIC_IP_STATE_DELETING to proto enum and state map

Ticket

OSAC-779

Signed-off-by: akshaynadkarni 25892229+akshaynadkarni@users.noreply.github.com
Assisted-by: Cursor/Claude

Summary by CodeRabbit

• Chores

• Updated managed API module version.

• Bug Fixes

• Adjusted public IP deletion state transition to use a distinct "deleting" state in relevant flows.

• Preserve the public IP target-namespace annotation in additional status phases; only remove it when the IP is Allocated.

• Tests

• Updated tests to match the revised deletion state and annotation preservation behavior.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci-robot]

@akshaynadkarni: This pull request references OSAC-779 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set.

This pull request references OSAC-780 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

Two fixes for the PublicIP delete and detach flows:

1. OSAC-779: Feedback controller uses DELETING (not RELEASING) when a PublicIP CR
   is being deleted. Bumps private-api BSR module to v0.0.61.

2. OSAC-780: Target-namespace annotation is only cleared when state is ALLOCATED
   (detach completed), not when state is ATTACHED (about to detach). The previous check
   (state != Releasing) failed because the annotation sync runs before the state
   transitions to RELEASING in the same reconcile loop.

Why

OSAC-779: The feedback controller set RELEASING for all deletions, but RELEASING is the
detach flow (ATTACHED -> RELEASING -> ALLOCATED). The fulfillment-service state machine
rejected ALLOCATED -> RELEASING, leaving the CR stuck in Deleting phase.

OSAC-780: The syncComputeInstanceTargetNamespaceAnnotation() function runs before the
state transition in handleUpdate(). When spec.computeInstance is cleared (detach trigger),
the annotation was deleted while state was still ATTACHED (not yet RELEASING), so the
OSAC-776 preservation check never triggered. The AAP detach job then failed because the
annotation was missing.

Testing

go build ./...                                                           # pass
ginkgo run --focus="PublicIP" internal/controller                        # 89 passed, 0 failed

E2E verified on edge22: full lifecycle (create pool, create IP, attach, detach, delete)
completed successfully with both fixes applied.

Related PRs

• fulfillment-service PR #515 (merged): added PUBLIC_IP_STATE_DELETING to proto enum
• osac-aap PR (pending): relaxed detach playbook validation for spec.computeInstance

Tickets

• OSAC-779
• OSAC-780

Signed-off-by: akshaynadkarni 25892229+akshaynadkarni@users.noreply.github.com
Assisted-by: Cursor/Claude

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[akshaynadkarni]

/retest-required

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akshaynadkarni, DakCrowder, SiddarthR56

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [akshaynadkarni]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[akshaynadkarni]

/override ci/prow/e2e-metal-vmaas-compute-instance-api-fields

[openshift-ci]

@akshaynadkarni: Overrode contexts on behalf of akshaynadkarni: ci/prow/e2e-metal-vmaas-compute-instance-api-fields

Details

In response to this:

/override ci/prow/e2e-metal-vmaas-compute-instance-api-fields

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[openshift-ci]

@akshaynadkarni: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-metal-vmaas-compute-instance-api-fields	2cb9ac6	link	unknown	/test e2e-metal-vmaas-compute-instance-api-fields

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.