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

Add support for the MMDS configuration. #290

Merged
merged 3 commits into from
Dec 14, 2020

Conversation

yjuba
Copy link
Contributor

@yjuba yjuba commented Nov 22, 2020

The goal of this PR is to support the configuration of MMDS in the SDK.

Issue #, if available:

Description of changes:

  • Add a handler to call mmds/config in handlers.go.
  • Add a wrapper to call mmds/config in firecracker.go.
  • Add a wrapped function call to machine.go.

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

@kzys
Copy link
Contributor

kzys commented Nov 24, 2020

The handler looks good. How about adding the IP address to Config struct?

@yjuba
Copy link
Contributor Author

yjuba commented Nov 24, 2020

Thank you for your comment.
I was wondering if I should add the IP address to the Config struct, so I would like to add it.

@kzys
Copy link
Contributor

kzys commented Dec 11, 2020

Sorry for the late reply. Looks good to me. Can you rebase the branch against master? You don't have to change go.mod and go.sum if they conflict. This PR doesn't add any new dependencies.

Signed-off-by: Yutaka Juba <yutaka.j+github@gmail.com>
…he handler.

Signed-off-by: Yutaka Juba <yutaka.j+github@gmail.com>
Signed-off-by: Yutaka Juba <yutaka.j+github@gmail.com>
@yjuba
Copy link
Contributor Author

yjuba commented Dec 14, 2020

Thank you for your comment.
Please check it because it has been rebased.

I haven't made any changes to the code, but the test timed out.
Do you have any idea of ​​the cause?

@kzys
Copy link
Contributor

kzys commented Dec 14, 2020

That's odd. Let me restart the BuildKite test.

@kzys
Copy link
Contributor

kzys commented Dec 14, 2020

The error we have right now is find: ‘./testdata/TestCNI/cni.cache/results’: Permission denied which is not related to this PR.

@kzys kzys mentioned this pull request Dec 14, 2020
@kzys kzys merged commit f88301e into firecracker-microvm:master Dec 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants