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

Suffix usage slot to annotation key name #560

Conversation

johnsonshih
Copy link
Contributor

@johnsonshih johnsonshih commented Feb 28, 2023

What this PR does / why we need it:

When allocate a device plugin, Agent adds the usage slot id to annotations to keep track of slot usage. Currently, when a container uses multiple device plugins, all device plugins will set their values to the same annotation key and only information of one slot is recorded. This PR suffix slot id to the annotation key name to keep all slot information.

The new format of annotation is:
akri.agent.slot-"slot id": "slot id"

In Agent, we parse the annotation keys use the format to avoid accidentally pulling in annotations with similar key name but are not set by Agent.

This PR is part of the fix for issue #492

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)
  • all commits pass the DCO bot check by being signed off -- see the failing DCO check for instructions on how to retroactively sign commits

Signed-off-by: Johnson Shih <jshih@microsoft.com>
@github-actions github-actions bot added the version/patch Patch version change is needed label Feb 28, 2023
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@adithyaj
Copy link
Collaborator

Johnson, I am unable to see any of the tests running for this PR. Were you able to see the tests run when you committed the changed? Can you also mark that cargo clippy/cargo fmt/cargo build etc from up top?

Signed-off-by: Johnson Shih <jshih@microsoft.com>

This reverts commit d4b5338.
@johnsonshih
Copy link
Contributor Author

/add-same-version-label

@github-actions
Copy link
Contributor

github-actions bot commented Apr 4, 2023

👋 Added [same version] label :)!

@github-actions
Copy link
Contributor

github-actions bot commented Apr 4, 2023

👋 Added [same version] label :)!

Signed-off-by: Johnson Shih <jshih@microsoft.com>
@adithyaj
Copy link
Collaborator

adithyaj commented Apr 4, 2023

tests ran successfully 🎉 and this looks good to me; this should likely have a docs change as well when #560 & #561 resolves #492

@johnsonshih
Copy link
Contributor Author

tests ran successfully 🎉 and this looks good to me; this should likely have a docs change as well when #560 & #561 resolves #492

PR#560 is transparent to user.
PR#561 requires document change.

@yujinkim-msft yujinkim-msft merged commit b999938 into project-akri:main Apr 5, 2023
33 checks passed
@johnsonshih johnsonshih deleted the user/jshih/device-plugin-annotation branch April 5, 2023 05:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
same version version/patch Patch version change is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable mounting connectivity information for multiple devices/instances in a Pod
3 participants