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

Make Discovery Handlers check channel health each discovery loop #385

Merged

Conversation

kate-goldenring
Copy link
Contributor

What this PR does / why we need it:
Akri's Discovery Handlers were designed to only send updates to the Agent if new devices are discovered. This means that if the Agent restarts, the Discovery Handler may not know to register with it again, since it is only aware of Agent restarts when a send errors. This creates scenarios where an operator may do a helm upgrade to modify a configuration, say to filter for certain ONVIF cameras and the change may not be communicated to the Discovery Handler due to the following: The Agent will see the Configuration has changed and close its connection with the discovery handler. The Discovery Handler will not know the connection is closed since devices have not gone offline, so it will not re-register and therefore receive the new filtering information.

To fix this, the Discovery Handlers should check if the channel with the Agent is open at the beginning of each discovery loop. If not, it should re-register with the Agent.

Special notes for your reviewer:

If applicable:

  • this PR has an associated PR with documentation in akri-docs
  • this PR contains unit tests
  • added code adheres to standard Rust formatting (cargo fmt)
  • code builds properly (cargo build)
  • code is free of common mistakes (cargo clippy)
  • all Akri tests succeed (cargo test)
  • inline documentation builds (cargo doc)
  • version has been updated appropriately (./version.sh)

@kate-goldenring kate-goldenring merged commit 8fa52c3 into project-akri:main Sep 28, 2021
@kate-goldenring kate-goldenring deleted the check-channel-liveness branch September 28, 2021 20:41
vincepnguyen pushed a commit that referenced this pull request Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Helm upgrade for ONVIF (going from Exclude to Include) doesn’t always lead to expected results.
2 participants