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.

Sign up for GitHub

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

Jump to bottom

Deprecate vsock_id #2763

Merged
merged 3 commits into from
Nov 3, 2021
Merged

Deprecate vsock_id #2763

merged 3 commits into from
Nov 3, 2021

Conversation

georgepisaltu
Copy link
Contributor

@georgepisaltu georgepisaltu commented Oct 28, 2021 •
edited
Loading

Reason for This PR

vsock_id serves no purpose in our API, as the vsock device always has the same internal ID when registering it.

Description of Changes

Deprecated the vsock_id field in PUTs on /vsock.

  • This functionality can be added in rust-vmm.

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license.

PR Checklist

[Author TODO: Meet these criteria.]
[Reviewer TODO: Verify that these criteria are met. Request changes if not]

  • All commits in this PR are signed (git commit -s).
  • The issue which led to this PR has a clear conclusion.
  • This PR follows the solution outlined in the related issue.
  • The description of changes is clear and encompassing.
  • Any required documentation changes (code and docs) are included in this PR.
  • Any newly added unsafe code is properly documented.
  • Any API changes are reflected in firecracker/swagger.yaml.
  • Any user-facing changes are mentioned in CHANGELOG.md.
  • All added/changed functionality is tested.

@georgepisaltu georgepisaltu self-assigned this Oct 28, 2021
@georgepisaltu georgepisaltu reopened this Oct 28, 2021
@georgepisaltu georgepisaltu marked this pull request as ready for review October 28, 2021 15:09
@georgepisaltu georgepisaltu force-pushed the vsock_id_deprecation branch 4 times, most recently from 2644c44 to 3a85ab9 Compare October 28, 2021 16:32
AlexandruCihodaru
AlexandruCihodaru previously approved these changes Oct 29, 2021
View reviewed changes
Copy link
Contributor

@AlexandruCihodaru AlexandruCihodaru left a comment

Choose a reason for hiding this comment

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

Top notch job! 🚢

tests/integration_tests/functional/test_api.py Outdated Show resolved Hide resolved
@serban300 serban300 self-requested a review October 29, 2021 08:11
serban300
serban300 reviewed Oct 29, 2021
View reviewed changes
src/api_server/src/parsed_request.rs Outdated Show resolved Hide resolved
src/api_server/src/request/vsock.rs Outdated Show resolved Hide resolved
src/api_server/src/parsed_request.rs Outdated Show resolved Hide resolved
tests/integration_tests/functional/test_api.py Outdated Show resolved Hide resolved
tests/integration_tests/functional/test_api.py Outdated Show resolved Hide resolved
tests/integration_tests/functional/test_api.py Show resolved Hide resolved
@georgepisaltu georgepisaltu force-pushed the vsock_id_deprecation branch 3 times, most recently from bd443d4 to b7765ca Compare November 1, 2021 13:38
serban300
serban300 reviewed Nov 1, 2021
View reviewed changes
src/api_server/src/lib.rs Outdated Show resolved Hide resolved
tests/integration_tests/functional/test_api.py Show resolved Hide resolved
tests/integration_tests/functional/test_api.py Show resolved Hide resolved
tests/integration_tests/functional/test_api.py Outdated Show resolved Hide resolved
@georgepisaltu
refactor api_server for endpoint deprecation
9162da8 
Signed-off-by: George Pisaltu <gpl@amazon.com>
@georgepisaltu
make vsock_id field in PUT /vsock optional
ea5bdbb 
In this commit we deprecate the `vsock_id` field in PUTs on /vsock by
making it optional.

Signed-off-by: George Pisaltu <gpl@amazon.com>
@georgepisaltu
add deprecation case for vsock_id in integ tests
859885d 
Signed-off-by: George Pisaltu <gpl@amazon.com>
@AlexandruCihodaru AlexandruCihodaru merged commit 472a81d into firecracker-microvm:main Nov 3, 2021
@georgepisaltu georgepisaltu deleted the vsock_id_deprecation branch November 12, 2021 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@serban300 serban300 serban300 approved these changes

@AlexandruCihodaru AlexandruCihodaru AlexandruCihodaru approved these changes

Assignees

@georgepisaltu georgepisaltu

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@georgepisaltu @serban300 @AlexandruCihodaru