Skip to content

mmds: deprecate V1 #2973

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
Merged
merged 1 commit into from
Apr 26, 2022
Merged

mmds: deprecate V1 #2973

merged 1 commit into from
Apr 26, 2022

Conversation

alindima
Copy link
Contributor

@alindima alindima commented Apr 26, 2022 •
edited
Loading

Description of Changes

Deprecates MmdsV1

  • 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

[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 follow the Runbook for Firecracker API changes.
  • Any user-facing changes are mentioned in CHANGELOG.md.
  • All added/changed functionality is tested.

@alindima
mmds: deprecate V1
558ff5c 
Signed-off-by: alindima <[email protected]>
@alindima alindima self-assigned this Apr 26, 2022
@alindima alindima requested review from luminitavoicu and a team April 26, 2022 12:52
luminitavoicu
luminitavoicu approved these changes Apr 26, 2022
View reviewed changes
Copy link
Contributor

@luminitavoicu luminitavoicu left a comment

Choose a reason for hiding this comment

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

Looks good!

tests/integration_tests/functional/test_mmds.py
Comment on lines +1042 to +1046
assert sum(list(map(
lambda line:
json.loads(line)['deprecated_api']['deprecated_http_api_calls'],
lines
))) == 2
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

@alindima alindima requested a review from a team April 26, 2022 13:27
@dianpopa dianpopa merged commit e1127dc into firecracker-microvm:main Apr 26, 2022
@raduiliescu
Copy link
Contributor

Do we want to release this in Firecracker v1.1?

georgepisaltu
georgepisaltu reviewed Apr 27, 2022
View reviewed changes
Copy link
Contributor

@georgepisaltu georgepisaltu left a comment

Choose a reason for hiding this comment

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

Maybe it's worth considering changing the default MMDS version to V2 so that you don't default to a deprecated endpoint.

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

@georgepisaltu georgepisaltu georgepisaltu left review comments

@dianpopa dianpopa dianpopa approved these changes

@luminitavoicu luminitavoicu luminitavoicu approved these changes

Assignees

@alindima alindima

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@alindima @raduiliescu @dianpopa @luminitavoicu @georgepisaltu