Skip to content

OCPBUGS-132756: Delete the subscriber ConfigMap entry on connection loss - #769

Open
ferikov wants to merge 2 commits into
redhat-cne:mainfrom
ferikov:ocpbugs38284-additional-fix
Open

ferikov wants to merge 2 commits into
redhat-cne:mainfrom
ferikov:ocpbugs38284-additional-fix

Conversation

@ferikov

@ferikov ferikov commented Oct 18, 2026 •

Copy link
Copy Markdown

Connection-loss cleanup deletes the subscriber with the subscriber client ID from GetClientIDAddressByResource, then called cleanupConfigMap(d.ClientID). d.ClientID is the publisher ID set by PublishEventViaAPI from scConfig.ClientID(), and that value is never assigned, so the ConfigMap delete targets 00000000-0000-0000-0000-000000000000 and the consumer entry remains. A restart reloads it from the ConfigMap.

The explicit DELETE /api/ocloudNotifications/v2/subscriptions path already sends the subscriber client ID and is unchanged. This pull request passes that same subscriber ID into cleanupConfigMap on the connection-loss path.

Conformance test coverage for this path is proposed in k8snetworkplumbingwg/ptp-operator#329. That pull request is still open.

Fixes #768
Signed-off-by: Ferenc Kovacs fkovacs@redhat.com

Fixes redhat-cne#768
Signed-off-by: Ferenc Kovacs <fkovacs@redhat.com>
@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 995d9987-6ab4-401f-aaec-f4eafad87d3a

Comment @coderabbitai help to get the list of available commands.

@openshift-ci

openshift-ci Bot commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Hi @ferikov. Thanks for your PR.

I'm waiting for a redhat-cne member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@jzding jzding left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good find!
/lgtm

@openshift-ci

openshift-ci Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ferikov, jzding

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:

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

@jzding jzding changed the title Delete the subscriber ConfigMap entry on connection loss OCPBUGS-132756: Delete the subscriber ConfigMap entry on connection loss Oct 21, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@ferikov: This pull request references Jira Issue OCPBUGS-132756, which is invalid:

  • expected the bug to target the "5.1.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Connection-loss cleanup deletes the subscriber with the subscriber client ID from GetClientIDAddressByResource, then called cleanupConfigMap(d.ClientID). d.ClientID is the publisher ID set by PublishEventViaAPI from scConfig.ClientID(), and that value is never assigned, so the ConfigMap delete targets 00000000-0000-0000-0000-000000000000 and the consumer entry remains. A restart reloads it from the ConfigMap.

The explicit DELETE /api/ocloudNotifications/v2/subscriptions path already sends the subscriber client ID and is unchanged. This pull request passes that same subscriber ID into cleanupConfigMap on the connection-loss path.

Conformance test coverage for this path is proposed in k8snetworkplumbingwg/ptp-operator#329. That pull request is still open.

Fixes #768
Signed-off-by: Ferenc Kovacs fkovacs@redhat.com

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.

@jzding

jzding commented Oct 21, 2026

Copy link
Copy Markdown
Member

/jira refresh

@openshift-ci-robot

Copy link
Copy Markdown

@jzding: This pull request references Jira Issue OCPBUGS-132756, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (5.1.0) matches configured target version for branch (5.1.0)
  • bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @dpopsuev

Details

In response to this:

/jira refresh

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

openshift-ci Bot commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

@openshift-ci-robot: GitHub didn't allow me to request PR reviews from the following users: dpopsuev.

Note that only redhat-cne members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

@jzding: This pull request references Jira Issue OCPBUGS-132756, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (5.1.0) matches configured target version for branch (5.1.0)
  • bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @dpopsuev

In response to this:

/jira refresh

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.

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.

@jzding

jzding commented Oct 21, 2026

Copy link
Copy Markdown
Member

/verified later @dpopsuev

@openshift-ci-robot

Copy link
Copy Markdown

@jzding: This PR has been marked to be verified later by @dpopsuev.

Details

In response to this:

/verified later @dpopsuev

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

openshift-ci Bot commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

New changes are detected. LGTM label has been removed.

@sebsoto

sebsoto commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

/hold

@sebsoto

sebsoto commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Where is this requirement coming from? I checked the ORAN spec and I don't see this behavior documented.
Specifically I looked at O-RAN.WG6.O-Cloud Notification API-v04.00

@ferikov

ferikov commented Oct 29, 2026

Copy link
Copy Markdown
Author

@sebsoto I think this is a non-functional requirement. Once we have persistency in the form of a configmap there should be a mechanism on the cep side to delete stale subcriptions. AFAIK this is the only mechanism for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Connection-loss cleanup deletes the wrong ConfigMap key

4 participants